commit 0dda8c84938d6bb4ce5a1707e5e109ea187fc33d
author Vladimir Marko <vmarko@google.com> Thu May 16 12:47:40 2019 +0000
committer Vladimir Marko <vmarko@google.com> Thu May 16 12:47:40 2019 +0000
tree 3d0b4f35ef4d00aa18eba0e417655d43bc44bf5a
parent 0ece86491008837a9814f7a2e0d7961c74ef4195

Revert "Improve ArraySet codegen."

This reverts commit 0ece86491008837a9814f7a2e0d7961c74ef4195.

Reason for revert: Breaks heap poisoning tests.

Bug: 32489401
Change-Id: Ied4150829eea848d0f967866d87c6aa7dafd39a1

compiler/optimizing/code_generator_x86_64.cc

diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index e816450..7c293b8 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -5143,11 +5143,13 @@
 
   bool needs_write_barrier =
       CodeGenerator::StoreNeedsWriteBarrier(value_type, instruction->GetValue());
-  bool needs_type_check = instruction->NeedsTypeCheck();
+  bool may_need_runtime_call_for_type_check = instruction->NeedsTypeCheck();
 
   LocationSummary* locations = new (GetGraph()->GetAllocator()) LocationSummary(
       instruction,
-      needs_type_check ? LocationSummary::kCallOnSlowPath : LocationSummary::kNoCall);
+      may_need_runtime_call_for_type_check ?
+          LocationSummary::kCallOnSlowPath :
+          LocationSummary::kNoCall);
 
   locations->SetInAt(0, Location::RequiresRegister());
   locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1)));
@@ -5171,9 +5173,12 @@
   Location index = locations->InAt(1);
   Location value = locations->InAt(2);
   DataType::Type value_type = instruction->GetComponentType();
-  bool needs_type_check = instruction->NeedsTypeCheck();
+  bool may_need_runtime_call_for_type_check = instruction->NeedsTypeCheck();
   bool needs_write_barrier =
       CodeGenerator::StoreNeedsWriteBarrier(value_type, instruction->GetValue());
+  uint32_t class_offset = mirror::Object::ClassOffset().Int32Value();
+  uint32_t super_offset = mirror::Class::SuperClassOffset().Int32Value();
+  uint32_t component_offset = mirror::Class::ComponentTypeOffset().Int32Value();
 
   switch (value_type) {
     case DataType::Type::kBool:
@@ -5215,30 +5220,30 @@
         __ movl(address, Immediate(0));
         codegen_->MaybeRecordImplicitNullCheck(instruction);
         DCHECK(!needs_write_barrier);
-        DCHECK(!needs_type_check);
+        DCHECK(!may_need_runtime_call_for_type_check);
         break;
       }
 
       DCHECK(needs_write_barrier);
       CpuRegister register_value = value.AsRegister<CpuRegister>();
+      // We cannot use a NearLabel for `done`, as its range may be too
+      // short when Baker read barriers are enabled.
+      Label done;
+      NearLabel not_null, do_put;
+      SlowPathCode* slow_path = nullptr;
       Location temp_loc = locations->GetTemp(0);
       CpuRegister temp = temp_loc.AsRegister<CpuRegister>();
-
-      bool can_value_be_null = instruction->GetValueCanBeNull();
-      NearLabel do_store;
-      if (can_value_be_null) {
-        __ testl(register_value, register_value);
-        __ j(kEqual, &do_store);
-      }
-
-      if (needs_type_check) {
-        SlowPathCode* slow_path =
-            new (codegen_->GetScopedAllocator()) ArraySetSlowPathX86_64(instruction);
+      if (may_need_runtime_call_for_type_check) {
+        slow_path = new (codegen_->GetScopedAllocator()) ArraySetSlowPathX86_64(instruction);
         codegen_->AddSlowPath(slow_path);
-
-        const uint32_t class_offset = mirror::Object::ClassOffset().Int32Value();
-        const uint32_t super_offset = mirror::Class::SuperClassOffset().Int32Value();
-        const uint32_t component_offset = mirror::Class::ComponentTypeOffset().Int32Value();
+        if (instruction->GetValueCanBeNull()) {
+          __ testl(register_value, register_value);
+          __ j(kNotEqual, &not_null);
+          __ movl(address, Immediate(0));
+          codegen_->MaybeRecordImplicitNullCheck(instruction);
+          __ jmp(&done);
+          __ Bind(&not_null);
+        }
 
         // Note that when Baker read barriers are enabled, the type
         // checks are performed without read barriers.  This is fine,
@@ -5261,7 +5266,6 @@
         __ cmpl(temp, Address(register_value, class_offset));
 
         if (instruction->StaticTypeOfArrayIsObjectArray()) {
-          NearLabel do_put;  // Use a dedicated NearLabel instead of the slow_path->GetExitLabel().
           __ j(kEqual, &do_put);
           // If heap poisoning is enabled, the `temp` reference has
           // not been unpoisoned yet; unpoison it now.
@@ -5276,29 +5280,26 @@
         } else {
           __ j(kNotEqual, slow_path->GetEntryLabel());
         }
-        __ Bind(slow_path->GetExitLabel());
+      }
+
+      if (kPoisonHeapReferences) {
+        __ movl(temp, register_value);
+        __ PoisonHeapReference(temp);
+        __ movl(address, temp);
+      } else {
+        __ movl(address, register_value);
+      }
+      if (!may_need_runtime_call_for_type_check) {
+        codegen_->MaybeRecordImplicitNullCheck(instruction);
       }
 
       CpuRegister card = locations->GetTemp(1).AsRegister<CpuRegister>();
       codegen_->MarkGCCard(
-          temp, card, array, value.AsRegister<CpuRegister>(), /* value_can_be_null= */ false);
+          temp, card, array, value.AsRegister<CpuRegister>(), instruction->GetValueCanBeNull());
+      __ Bind(&done);
 
-      Location source = value;
-      if (kPoisonHeapReferences) {
-        __ movl(temp, register_value);
-        __ PoisonHeapReference(temp);
-        source = temp_loc;
-      }
-
-      if (can_value_be_null) {
-        DCHECK(do_store.IsLinked());
-        __ Bind(&do_store);
-      }
-
-      __ movl(address, source.AsRegister<CpuRegister>());
-
-      if (can_value_be_null || !needs_type_check) {
-        codegen_->MaybeRecordImplicitNullCheck(instruction);
+      if (slow_path != nullptr) {
+        __ Bind(slow_path->GetExitLabel());
       }
 
       break;