commit f0acfe7a812a332122011832074142718c278dae
author Nicolas Geoffray <ngeoffray@google.com> Mon Jan 09 20:54:52 2017 +0000
committer Nicolas Geoffray <ngeoffray@google.com> Tue Jan 10 21:26:23 2017 +0000
tree 49c4fc481cebd03323aaf0109066859165508303
parent 91db41f315f6c2366b7098c531224bee01170364

Keep resolved String in HLoadString.

For the following reasons:
- Avoids needing to do a lookup again in CodeGenerator::EmitJitRoots.
- Fixes races where we the string was GC'ed before CodeGenerator::EmitJitRoots.
- Makes it possible to do GVN on the same string but defined in different
  dex files.

Test: test-art-host, test-art-target
Change-Id: If2b5d3079f7555427b1b96ab04546b3373fcf921

compiler/optimizing/code_generator_x86.cc

diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 0abe855..f13b60a 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -6232,15 +6232,19 @@
 }
 
 Label* CodeGeneratorX86::NewJitRootStringPatch(const DexFile& dex_file,
-                                               dex::StringIndex dex_index) {
-  jit_string_roots_.Overwrite(StringReference(&dex_file, dex_index), /* placeholder */ 0u);
+                                               dex::StringIndex dex_index,
+                                               Handle<mirror::String> handle) {
+  jit_string_roots_.Overwrite(
+      StringReference(&dex_file, dex_index), reinterpret_cast64<uint64_t>(handle.GetReference()));
   // Add a patch entry and return the label.
   jit_string_patches_.emplace_back(dex_file, dex_index.index_);
   PatchInfo<Label>* info = &jit_string_patches_.back();
   return &info->label;
 }
 
-void InstructionCodeGeneratorX86::VisitLoadString(HLoadString* load) {
+// NO_THREAD_SAFETY_ANALYSIS as we manipulate handles whose internal object we know does not
+// move.
+void InstructionCodeGeneratorX86::VisitLoadString(HLoadString* load) NO_THREAD_SAFETY_ANALYSIS {
   LocationSummary* locations = load->GetLocations();
   Location out_loc = locations->Out();
   Register out = out_loc.AsRegister<Register>();
@@ -6258,8 +6262,9 @@
       return;  // No dex cache slow path.
     }
     case HLoadString::LoadKind::kBootImageAddress: {
-      DCHECK_NE(load->GetAddress(), 0u);
-      uint32_t address = dchecked_integral_cast<uint32_t>(load->GetAddress());
+      uint32_t address = dchecked_integral_cast<uint32_t>(
+          reinterpret_cast<uintptr_t>(load->GetString().Get()));
+      DCHECK_NE(address, 0u);
       __ movl(out, Immediate(address));
       codegen_->RecordSimplePatch();
       return;  // No dex cache slow path.
@@ -6280,7 +6285,7 @@
     case HLoadString::LoadKind::kJitTableAddress: {
       Address address = Address::Absolute(CodeGeneratorX86::kDummy32BitOffset);
       Label* fixup_label = codegen_->NewJitRootStringPatch(
-          load->GetDexFile(), load->GetStringIndex());
+          load->GetDexFile(), load->GetStringIndex(), load->GetString());
       // /* GcRoot<mirror::String> */ out = *address
       GenerateGcRootFieldLoad(load, out_loc, address, fixup_label, kCompilerReadBarrierOption);
       return;