OSDN Git Service

Fix code generation of materialized conditions.
authorNicolas Geoffray <ngeoffray@google.com>
Wed, 8 Oct 2014 20:07:48 +0000 (21:07 +0100)
committerNicolas Geoffray <ngeoffray@google.com>
Thu, 9 Oct 2014 14:21:57 +0000 (15:21 +0100)
Move the logic for knowing if a condition needs to be materialized
in an optimization pass (so that the information does not change
as a side effect of another optimization).

Also clean-up arm and x86_64 codegen:
- arm: ldr and str are for power-users when a constant is
  in play. We should use LoadFromOffset and StoreToOffset.
- x86_64: fix misuses of movq instead of movl.

Change-Id: I01a03b91803624be2281a344a13ad5efbf4f3ef3

14 files changed:
compiler/optimizing/code_generator_arm.cc
compiler/optimizing/code_generator_arm.h
compiler/optimizing/code_generator_x86.cc
compiler/optimizing/code_generator_x86.h
compiler/optimizing/code_generator_x86_64.cc
compiler/optimizing/code_generator_x86_64.h
compiler/optimizing/codegen_test.cc
compiler/optimizing/live_ranges_test.cc
compiler/optimizing/liveness_test.cc
compiler/optimizing/nodes.cc
compiler/optimizing/nodes.h
compiler/optimizing/prepare_for_register_allocation.cc
compiler/optimizing/prepare_for_register_allocation.h
compiler/optimizing/register_allocator.cc

index d555a0d..1297381 100644 (file)
@@ -67,7 +67,7 @@ class NullCheckSlowPathARM : public SlowPathCode {
   virtual void EmitNativeCode(CodeGenerator* codegen) OVERRIDE {
     __ Bind(GetEntryLabel());
     int32_t offset = QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pThrowNullPointer).Int32Value();
-    __ ldr(LR, Address(TR, offset));
+    __ LoadFromOffset(kLoadWord, LR, TR, offset);
     __ blx(LR);
     codegen->RecordPcInfo(instruction_, instruction_->GetDexPc());
   }
@@ -100,7 +100,7 @@ class SuspendCheckSlowPathARM : public SlowPathCode {
     __ Bind(GetEntryLabel());
     codegen->SaveLiveRegisters(instruction_->GetLocations());
     int32_t offset = QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pTestSuspend).Int32Value();
-    __ ldr(LR, Address(TR, offset));
+    __ LoadFromOffset(kLoadWord, LR, TR, offset);
     __ blx(LR);
     codegen->RecordPcInfo(instruction_, instruction_->GetDexPc());
     codegen->RestoreLiveRegisters(instruction_->GetLocations());
@@ -143,7 +143,7 @@ class BoundsCheckSlowPathARM : public SlowPathCode {
     arm_codegen->Move32(Location::RegisterLocation(calling_convention.GetRegisterAt(0)), index_location_);
     arm_codegen->Move32(Location::RegisterLocation(calling_convention.GetRegisterAt(1)), length_location_);
     int32_t offset = QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pThrowArrayBounds).Int32Value();
-    __ ldr(LR, Address(TR, offset));
+    __ LoadFromOffset(kLoadWord, LR, TR, offset);
     __ blx(LR);
     codegen->RecordPcInfo(instruction_, instruction_->GetDexPc());
   }
@@ -196,11 +196,11 @@ void CodeGeneratorARM::DumpFloatingPointRegister(std::ostream& stream, int reg)
 }
 
 void CodeGeneratorARM::SaveCoreRegister(Location stack_location, uint32_t reg_id) {
-  __ str(static_cast<Register>(reg_id), Address(SP, stack_location.GetStackIndex()));
+  __ StoreToOffset(kStoreWord, static_cast<Register>(reg_id), SP, stack_location.GetStackIndex());
 }
 
 void CodeGeneratorARM::RestoreCoreRegister(Location stack_location, uint32_t reg_id) {
-  __ ldr(static_cast<Register>(reg_id), Address(SP, stack_location.GetStackIndex()));
+  __ LoadFromOffset(kLoadWord, static_cast<Register>(reg_id), SP, stack_location.GetStackIndex());
 }
 
 CodeGeneratorARM::CodeGeneratorARM(HGraph* graph)
@@ -339,7 +339,7 @@ void CodeGeneratorARM::GenerateFrameEntry() {
       __ b(slow_path->GetEntryLabel(), CC);
     } else {
       __ AddConstant(IP, SP, -static_cast<int32_t>(GetStackOverflowReservedBytes(kArm)));
-      __ ldr(IP, Address(IP, 0));
+      __ LoadFromOffset(kLoadWord, IP, IP, 0);
       RecordPcInfo(nullptr, 0);
     }
   }
@@ -349,7 +349,7 @@ void CodeGeneratorARM::GenerateFrameEntry() {
 
   // The return PC has already been pushed on the stack.
   __ AddConstant(SP, -(GetFrameSize() - kNumberOfPushedRegistersAtEntry * kArmWordSize));
-  __ str(R0, Address(SP, 0));
+  __ StoreToOffset(kStoreWord, R0, SP, 0);
 }
 
 void CodeGeneratorARM::GenerateFrameExit() {
@@ -434,7 +434,7 @@ void CodeGeneratorARM::Move32(Location destination, Location source) {
     } else if (source.IsFpuRegister()) {
       __ vmovrs(destination.As<Register>(), FromDToLowS(source.As<DRegister>()));
     } else {
-      __ ldr(destination.As<Register>(), Address(SP, source.GetStackIndex()));
+      __ LoadFromOffset(kLoadWord, destination.As<Register>(), SP, source.GetStackIndex());
     }
   } else if (destination.IsFpuRegister()) {
     if (source.IsRegister()) {
@@ -447,13 +447,13 @@ void CodeGeneratorARM::Move32(Location destination, Location source) {
   } else {
     DCHECK(destination.IsStackSlot());
     if (source.IsRegister()) {
-      __ str(source.As<Register>(), Address(SP, destination.GetStackIndex()));
+      __ StoreToOffset(kStoreWord, source.As<Register>(), SP, destination.GetStackIndex());
     } else if (source.IsFpuRegister()) {
       __ vstrs(FromDToLowS(source.As<DRegister>()), Address(SP, destination.GetStackIndex()));
     } else {
       DCHECK(source.IsStackSlot());
-      __ ldr(IP, Address(SP, source.GetStackIndex()));
-      __ str(IP, Address(SP, destination.GetStackIndex()));
+      __ LoadFromOffset(kLoadWord, IP, SP, source.GetStackIndex());
+      __ StoreToOffset(kStoreWord, IP, SP, destination.GetStackIndex());
     }
   }
 }
@@ -473,14 +473,14 @@ void CodeGeneratorARM::Move64(Location destination, Location source) {
       InvokeDexCallingConvention calling_convention;
       __ Mov(destination.AsRegisterPairLow<Register>(),
              calling_convention.GetRegisterAt(argument_index));
-      __ ldr(destination.AsRegisterPairHigh<Register>(),
-             Address(SP, calling_convention.GetStackOffsetOf(argument_index + 1) + GetFrameSize()));
+      __ LoadFromOffset(kLoadWord, destination.AsRegisterPairHigh<Register>(),
+             SP, calling_convention.GetStackOffsetOf(argument_index + 1) + GetFrameSize());
     } else {
       DCHECK(source.IsDoubleStackSlot());
       if (destination.AsRegisterPairLow<Register>() == R1) {
         DCHECK_EQ(destination.AsRegisterPairHigh<Register>(), R2);
-        __ ldr(R1, Address(SP, source.GetStackIndex()));
-        __ ldr(R2, Address(SP, source.GetHighStackIndex(kArmWordSize)));
+        __ LoadFromOffset(kLoadWord, R1, SP, source.GetStackIndex());
+        __ LoadFromOffset(kLoadWord, R2, SP, source.GetHighStackIndex(kArmWordSize));
       } else {
         __ LoadFromOffset(kLoadWordPair, destination.AsRegisterPairLow<Register>(),
                           SP, source.GetStackIndex());
@@ -496,24 +496,25 @@ void CodeGeneratorARM::Move64(Location destination, Location source) {
     InvokeDexCallingConvention calling_convention;
     uint32_t argument_index = destination.GetQuickParameterIndex();
     if (source.IsRegisterPair()) {
-      __ Mov(calling_convention.GetRegisterAt(argument_index), source.AsRegisterPairLow<Register>());
-      __ str(source.AsRegisterPairHigh<Register>(),
-             Address(SP, calling_convention.GetStackOffsetOf(argument_index + 1)));
+      __ Mov(calling_convention.GetRegisterAt(argument_index),
+             source.AsRegisterPairLow<Register>());
+      __ StoreToOffset(kStoreWord, source.AsRegisterPairHigh<Register>(),
+             SP, calling_convention.GetStackOffsetOf(argument_index + 1));
     } else if (source.IsFpuRegister()) {
       LOG(FATAL) << "Unimplemented";
     } else {
       DCHECK(source.IsDoubleStackSlot());
-      __ ldr(calling_convention.GetRegisterAt(argument_index), Address(SP, source.GetStackIndex()));
-      __ ldr(R0, Address(SP, source.GetHighStackIndex(kArmWordSize)));
-      __ str(R0, Address(SP, calling_convention.GetStackOffsetOf(argument_index + 1)));
+      __ LoadFromOffset(kLoadWord, calling_convention.GetRegisterAt(argument_index), SP, source.GetStackIndex());
+      __ LoadFromOffset(kLoadWord, R0, SP, source.GetHighStackIndex(kArmWordSize));
+      __ StoreToOffset(kStoreWord, R0, SP, calling_convention.GetStackOffsetOf(argument_index + 1));
     }
   } else {
     DCHECK(destination.IsDoubleStackSlot());
     if (source.IsRegisterPair()) {
       if (source.AsRegisterPairLow<Register>() == R1) {
         DCHECK_EQ(source.AsRegisterPairHigh<Register>(), R2);
-        __ str(R1, Address(SP, destination.GetStackIndex()));
-        __ str(R2, Address(SP, destination.GetHighStackIndex(kArmWordSize)));
+        __ StoreToOffset(kStoreWord, R1, SP, destination.GetStackIndex());
+        __ StoreToOffset(kStoreWord, R2, SP, destination.GetHighStackIndex(kArmWordSize));
       } else {
         __ StoreToOffset(kStoreWordPair, source.AsRegisterPairLow<Register>(),
                          SP, destination.GetStackIndex());
@@ -521,19 +522,19 @@ void CodeGeneratorARM::Move64(Location destination, Location source) {
     } else if (source.IsQuickParameter()) {
       InvokeDexCallingConvention calling_convention;
       uint32_t argument_index = source.GetQuickParameterIndex();
-      __ str(calling_convention.GetRegisterAt(argument_index),
-             Address(SP, destination.GetStackIndex()));
-      __ ldr(R0,
-             Address(SP, calling_convention.GetStackOffsetOf(argument_index + 1) + GetFrameSize()));
-      __ str(R0, Address(SP, destination.GetHighStackIndex(kArmWordSize)));
+      __ StoreToOffset(kStoreWord, calling_convention.GetRegisterAt(argument_index),
+             SP, destination.GetStackIndex());
+      __ LoadFromOffset(kLoadWord, R0,
+             SP, calling_convention.GetStackOffsetOf(argument_index + 1) + GetFrameSize());
+      __ StoreToOffset(kStoreWord, R0, SP, destination.GetHighStackIndex(kArmWordSize));
     } else if (source.IsFpuRegister()) {
       __ vstrd(source.As<DRegister>(), Address(SP, destination.GetStackIndex()));
     } else {
       DCHECK(source.IsDoubleStackSlot());
-      __ ldr(IP, Address(SP, source.GetStackIndex()));
-      __ str(IP, Address(SP, destination.GetStackIndex()));
-      __ ldr(IP, Address(SP, source.GetHighStackIndex(kArmWordSize)));
-      __ str(IP, Address(SP, destination.GetHighStackIndex(kArmWordSize)));
+      __ LoadFromOffset(kLoadWord, IP, SP, source.GetStackIndex());
+      __ StoreToOffset(kStoreWord, IP, SP, destination.GetStackIndex());
+      __ LoadFromOffset(kLoadWord, IP, SP, source.GetHighStackIndex(kArmWordSize));
+      __ StoreToOffset(kStoreWord, IP, SP, destination.GetHighStackIndex(kArmWordSize));
     }
   }
 }
@@ -551,7 +552,7 @@ void CodeGeneratorARM::Move(HInstruction* instruction, Location location, HInstr
     } else {
       DCHECK(location.IsStackSlot());
       __ LoadImmediate(IP, value);
-      __ str(IP, Address(SP, location.GetStackIndex()));
+      __ StoreToOffset(kStoreWord, IP, SP, location.GetStackIndex());
     }
   } else if (instruction->AsLongConstant() != nullptr) {
     int64_t value = instruction->AsLongConstant()->GetValue();
@@ -561,9 +562,9 @@ void CodeGeneratorARM::Move(HInstruction* instruction, Location location, HInstr
     } else {
       DCHECK(location.IsDoubleStackSlot());
       __ LoadImmediate(IP, Low32Bits(value));
-      __ str(IP, Address(SP, location.GetStackIndex()));
+      __ StoreToOffset(kStoreWord, IP, SP, location.GetStackIndex());
       __ LoadImmediate(IP, High32Bits(value));
-      __ str(IP, Address(SP, location.GetHighStackIndex(kArmWordSize)));
+      __ StoreToOffset(kStoreWord, IP, SP, location.GetHighStackIndex(kArmWordSize));
     }
   } else if (instruction->AsLoadLocal() != nullptr) {
     uint32_t stack_slot = GetStackSlot(instruction->AsLoadLocal()->GetLocal());
@@ -902,7 +903,7 @@ void LocationsBuilderARM::VisitInvokeStatic(HInvokeStatic* invoke) {
 }
 
 void InstructionCodeGeneratorARM::LoadCurrentMethod(Register reg) {
-  __ ldr(reg, Address(SP, kCurrentMethodStackOffset));
+  __ LoadFromOffset(kLoadWord, reg, SP, kCurrentMethodStackOffset);
 }
 
 void InstructionCodeGeneratorARM::VisitInvokeStatic(HInvokeStatic* invoke) {
@@ -921,12 +922,12 @@ void InstructionCodeGeneratorARM::VisitInvokeStatic(HInvokeStatic* invoke) {
   // temp = method;
   LoadCurrentMethod(temp);
   // temp = temp->dex_cache_resolved_methods_;
-  __ ldr(temp, Address(temp, mirror::ArtMethod::DexCacheResolvedMethodsOffset().Int32Value()));
+  __ LoadFromOffset(kLoadWord, temp, temp, mirror::ArtMethod::DexCacheResolvedMethodsOffset().Int32Value());
   // temp = temp[index_in_cache]
-  __ ldr(temp, Address(temp, index_in_cache));
+  __ LoadFromOffset(kLoadWord, temp, temp, index_in_cache);
   // LR = temp[offset_of_quick_compiled_code]
-  __ ldr(LR, Address(temp,
-                     mirror::ArtMethod::EntryPointFromQuickCompiledCodeOffset().Int32Value()));
+  __ LoadFromOffset(kLoadWord, LR, temp,
+                     mirror::ArtMethod::EntryPointFromQuickCompiledCodeOffset().Int32Value());
   // LR()
   __ blx(LR);
 
@@ -980,16 +981,16 @@ void InstructionCodeGeneratorARM::VisitInvokeVirtual(HInvokeVirtual* invoke) {
   uint32_t class_offset = mirror::Object::ClassOffset().Int32Value();
   // temp = object->GetClass();
   if (receiver.IsStackSlot()) {
-    __ ldr(temp, Address(SP, receiver.GetStackIndex()));
-    __ ldr(temp, Address(temp, class_offset));
+    __ LoadFromOffset(kLoadWord, temp, SP, receiver.GetStackIndex());
+    __ LoadFromOffset(kLoadWord, temp, temp, class_offset);
   } else {
-    __ ldr(temp, Address(receiver.As<Register>(), class_offset));
+    __ LoadFromOffset(kLoadWord, temp, receiver.As<Register>(), class_offset);
   }
   // temp = temp->GetMethodAt(method_offset);
   uint32_t entry_point = mirror::ArtMethod::EntryPointFromQuickCompiledCodeOffset().Int32Value();
-  __ ldr(temp, Address(temp, method_offset));
+  __ LoadFromOffset(kLoadWord, temp, temp, method_offset);
   // LR = temp->GetEntryPoint();
-  __ ldr(LR, Address(temp, entry_point));
+  __ LoadFromOffset(kLoadWord, LR, temp, entry_point);
   // LR();
   __ blx(LR);
   DCHECK(!codegen_->IsLeafMethod());
@@ -1139,7 +1140,7 @@ void InstructionCodeGeneratorARM::VisitNewInstance(HNewInstance* instruction) {
   __ LoadImmediate(calling_convention.GetRegisterAt(0), instruction->GetTypeIndex());
 
   int32_t offset = QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pAllocObjectWithAccessCheck).Int32Value();
-  __ ldr(LR, Address(TR, offset));
+  __ LoadFromOffset(kLoadWord, LR, TR, offset);
   __ blx(LR);
 
   codegen_->RecordPcInfo(instruction, instruction->GetDexPc());
@@ -1552,7 +1553,7 @@ void InstructionCodeGeneratorARM::VisitArraySet(HArraySet* instruction) {
 
     case Primitive::kPrimNot: {
       int32_t offset = QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pAputObject).Int32Value();
-      __ ldr(LR, Address(TR, offset));
+      __ LoadFromOffset(kLoadWord, LR, TR, offset);
       __ blx(LR);
       codegen_->RecordPcInfo(instruction, instruction->GetDexPc());
       DCHECK(!codegen_->IsLeafMethod());
@@ -1713,7 +1714,7 @@ void ParallelMoveResolverARM::EmitMove(size_t index) {
     } else {
       DCHECK(destination.IsStackSlot());
       __ LoadImmediate(IP, value);
-      __ str(IP, Address(SP, destination.GetStackIndex()));
+      __ StoreToOffset(kStoreWord, IP, SP, destination.GetStackIndex());
     }
   }
 }
index 9da26e8..c5a8e55 100644 (file)
@@ -91,7 +91,7 @@ class LocationsBuilderARM : public HGraphVisitor {
   LocationsBuilderARM(HGraph* graph, CodeGeneratorARM* codegen)
       : HGraphVisitor(graph), codegen_(codegen) {}
 
-#define DECLARE_VISIT_INSTRUCTION(name)     \
+#define DECLARE_VISIT_INSTRUCTION(name, super)     \
   virtual void Visit##name(H##name* instr);
 
   FOR_EACH_CONCRETE_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
@@ -111,7 +111,7 @@ class InstructionCodeGeneratorARM : public HGraphVisitor {
  public:
   InstructionCodeGeneratorARM(HGraph* graph, CodeGeneratorARM* codegen);
 
-#define DECLARE_VISIT_INSTRUCTION(name)     \
+#define DECLARE_VISIT_INSTRUCTION(name, super)     \
   virtual void Visit##name(H##name* instr);
 
   FOR_EACH_CONCRETE_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
index 5f6d458..114a574 100644 (file)
@@ -593,10 +593,13 @@ void LocationsBuilderX86::VisitIf(HIf* if_instr) {
 
 void InstructionCodeGeneratorX86::VisitIf(HIf* if_instr) {
   HInstruction* cond = if_instr->InputAt(0);
-  if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) {
-    // Moves do not affect the eflags register, so if the condition is evaluated
-    // just before the if, we don't need to evaluate it again.
-    if (!cond->IsCondition() || !cond->AsCondition()->IsBeforeWhenDisregardMoves(if_instr)) {
+  bool materialized = !cond->IsCondition() || cond->AsCondition()->NeedsMaterialization();
+  // Moves do not affect the eflags register, so if the condition is evaluated
+  // just before the if, we don't need to evaluate it again.
+  bool eflags_set = cond->IsCondition()
+      && cond->AsCondition()->IsBeforeWhenDisregardMoves(if_instr);
+  if (materialized) {
+    if (!eflags_set) {
       // Materialized condition, compare against 0.
       Location lhs = if_instr->GetLocations()->InAt(0);
       if (lhs.IsRegister()) {
@@ -604,8 +607,11 @@ void InstructionCodeGeneratorX86::VisitIf(HIf* if_instr) {
       } else {
         __ cmpl(Address(ESP, lhs.GetStackIndex()), Immediate(0));
       }
+      __ j(kNotEqual,  codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
+    } else {
+      __ j(X86Condition(cond->AsCondition()->GetCondition()),
+           codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
     }
-    __ j(kNotEqual,  codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
   } else {
     Location lhs = cond->GetLocations()->InAt(0);
     Location rhs = cond->GetLocations()->InAt(1);
index c520164..2524725 100644 (file)
@@ -92,7 +92,7 @@ class LocationsBuilderX86 : public HGraphVisitor {
   LocationsBuilderX86(HGraph* graph, CodeGeneratorX86* codegen)
       : HGraphVisitor(graph), codegen_(codegen) {}
 
-#define DECLARE_VISIT_INSTRUCTION(name)     \
+#define DECLARE_VISIT_INSTRUCTION(name, super)     \
   virtual void Visit##name(H##name* instr);
 
   FOR_EACH_CONCRETE_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
@@ -112,7 +112,7 @@ class InstructionCodeGeneratorX86 : public HGraphVisitor {
  public:
   InstructionCodeGeneratorX86(HGraph* graph, CodeGeneratorX86* codegen);
 
-#define DECLARE_VISIT_INSTRUCTION(name)     \
+#define DECLARE_VISIT_INSTRUCTION(name, super)     \
   virtual void Visit##name(H##name* instr);
 
   FOR_EACH_CONCRETE_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
index 393eb1a..d37ef06 100644 (file)
@@ -482,10 +482,13 @@ void LocationsBuilderX86_64::VisitIf(HIf* if_instr) {
 
 void InstructionCodeGeneratorX86_64::VisitIf(HIf* if_instr) {
   HInstruction* cond = if_instr->InputAt(0);
-  if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) {
-    // Moves do not affect the eflags register, so if the condition is evaluated
-    // just before the if, we don't need to evaluate it again.
-    if (!cond->IsCondition() || !cond->AsCondition()->IsBeforeWhenDisregardMoves(if_instr)) {
+  bool materialized = !cond->IsCondition() || cond->AsCondition()->NeedsMaterialization();
+  // Moves do not affect the eflags register, so if the condition is evaluated
+  // just before the if, we don't need to evaluate it again.
+  bool eflags_set = cond->IsCondition()
+      && cond->AsCondition()->IsBeforeWhenDisregardMoves(if_instr);
+  if (materialized) {
+    if (!eflags_set) {
       // Materialized condition, compare against 0.
       Location lhs = if_instr->GetLocations()->InAt(0);
       if (lhs.IsRegister()) {
@@ -493,8 +496,11 @@ void InstructionCodeGeneratorX86_64::VisitIf(HIf* if_instr) {
       } else {
         __ cmpl(Address(CpuRegister(RSP), lhs.GetStackIndex()), Immediate(0));
       }
+      __ j(kNotEqual, codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
+    } else {
+      __ j(X86_64Condition(cond->AsCondition()->GetCondition()),
+           codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
     }
-    __ j(kNotEqual, codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
   } else {
     Location lhs = cond->GetLocations()->InAt(0);
     Location rhs = cond->GetLocations()->InAt(1);
@@ -574,13 +580,13 @@ void InstructionCodeGeneratorX86_64::VisitCondition(HCondition* comp) {
     // Clear register: setcc only sets the low byte.
     __ xorq(reg, reg);
     if (locations->InAt(1).IsRegister()) {
-      __ cmpq(locations->InAt(0).As<CpuRegister>(),
+      __ cmpl(locations->InAt(0).As<CpuRegister>(),
               locations->InAt(1).As<CpuRegister>());
     } else if (locations->InAt(1).IsConstant()) {
-      __ cmpq(locations->InAt(0).As<CpuRegister>(),
+      __ cmpl(locations->InAt(0).As<CpuRegister>(),
               Immediate(locations->InAt(1).GetConstant()->AsIntConstant()->GetValue()));
     } else {
-      __ cmpq(locations->InAt(0).As<CpuRegister>(),
+      __ cmpl(locations->InAt(0).As<CpuRegister>(),
               Address(CpuRegister(RSP), locations->InAt(1).GetStackIndex()));
     }
     __ setcc(X86_64Condition(comp->GetCondition()), reg);
@@ -879,10 +885,10 @@ void InstructionCodeGeneratorX86_64::VisitInvokeVirtual(HInvokeVirtual* invoke)
   size_t class_offset = mirror::Object::ClassOffset().SizeValue();
   // temp = object->GetClass();
   if (receiver.IsStackSlot()) {
-    __ movq(temp, Address(CpuRegister(RSP), receiver.GetStackIndex()));
-    __ movq(temp, Address(temp, class_offset));
+    __ movl(temp, Address(CpuRegister(RSP), receiver.GetStackIndex()));
+    __ movl(temp, Address(temp, class_offset));
   } else {
-    __ movq(temp, Address(receiver.As<CpuRegister>(), class_offset));
+    __ movl(temp, Address(receiver.As<CpuRegister>(), class_offset));
   }
   // temp = temp->GetMethodAt(method_offset);
   __ movl(temp, Address(temp, method_offset));
index bdaf15f..cba3a54 100644 (file)
@@ -94,7 +94,7 @@ class LocationsBuilderX86_64 : public HGraphVisitor {
   LocationsBuilderX86_64(HGraph* graph, CodeGeneratorX86_64* codegen)
       : HGraphVisitor(graph), codegen_(codegen) {}
 
-#define DECLARE_VISIT_INSTRUCTION(name)     \
+#define DECLARE_VISIT_INSTRUCTION(name, super)     \
   virtual void Visit##name(H##name* instr);
 
   FOR_EACH_CONCRETE_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
@@ -114,7 +114,7 @@ class InstructionCodeGeneratorX86_64 : public HGraphVisitor {
  public:
   InstructionCodeGeneratorX86_64(HGraph* graph, CodeGeneratorX86_64* codegen);
 
-#define DECLARE_VISIT_INSTRUCTION(name)     \
+#define DECLARE_VISIT_INSTRUCTION(name, super)     \
   virtual void Visit##name(H##name* instr);
 
   FOR_EACH_CONCRETE_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
index 7161eed..3037f1c 100644 (file)
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <functional>
+
 #include "builder.h"
 #include "code_generator_arm.h"
 #include "code_generator_x86.h"
@@ -24,6 +26,9 @@
 #include "instruction_set.h"
 #include "nodes.h"
 #include "optimizing_unit_test.h"
+#include "prepare_for_register_allocation.h"
+#include "register_allocator.h"
+#include "ssa_liveness_analysis.h"
 
 #include "gtest/gtest.h"
 
@@ -62,19 +67,11 @@ static void Run(const InternalCodeAllocator& allocator,
   }
   int32_t result = f();
   if (has_result) {
-    CHECK_EQ(result, expected);
+    ASSERT_EQ(result, expected);
   }
 }
 
-static void TestCode(const uint16_t* data, bool has_result = false, int32_t expected = 0) {
-  ArenaPool pool;
-  ArenaAllocator arena(&pool);
-  HGraphBuilder builder(&arena);
-  const DexFile::CodeItem* item = reinterpret_cast<const DexFile::CodeItem*>(data);
-  HGraph* graph = builder.BuildGraph(*item);
-  // Remove suspend checks, they cannot be executed in this context.
-  RemoveSuspendChecks(graph);
-  ASSERT_NE(graph, nullptr);
+static void RunCodeBaseline(HGraph* graph, bool has_result, int32_t expected) {
   InternalCodeAllocator allocator;
 
   x86::CodeGeneratorX86 codegenX86(graph);
@@ -98,6 +95,51 @@ static void TestCode(const uint16_t* data, bool has_result = false, int32_t expe
   }
 }
 
+static void RunCodeOptimized(CodeGenerator* codegen,
+                             HGraph* graph,
+                             std::function<void(HGraph*)> hook_before_codegen,
+                             bool has_result,
+                             int32_t expected) {
+  SsaLivenessAnalysis liveness(*graph, codegen);
+  liveness.Analyze();
+
+  RegisterAllocator register_allocator(graph->GetArena(), codegen, liveness);
+  register_allocator.AllocateRegisters();
+  hook_before_codegen(graph);
+
+  InternalCodeAllocator allocator;
+  codegen->CompileOptimized(&allocator);
+  Run(allocator, *codegen, has_result, expected);
+}
+
+static void RunCodeOptimized(HGraph* graph,
+                             std::function<void(HGraph*)> hook_before_codegen,
+                             bool has_result,
+                             int32_t expected) {
+  if (kRuntimeISA == kX86) {
+    x86::CodeGeneratorX86 codegenX86(graph);
+    RunCodeOptimized(&codegenX86, graph, hook_before_codegen, has_result, expected);
+  } else if (kRuntimeISA == kArm || kRuntimeISA == kThumb2) {
+    arm::CodeGeneratorARM codegenARM(graph);
+    RunCodeOptimized(&codegenARM, graph, hook_before_codegen, has_result, expected);
+  } else if (kRuntimeISA == kX86_64) {
+    x86_64::CodeGeneratorX86_64 codegenX86_64(graph);
+    RunCodeOptimized(&codegenX86_64, graph, hook_before_codegen, has_result, expected);
+  }
+}
+
+static void TestCode(const uint16_t* data, bool has_result = false, int32_t expected = 0) {
+  ArenaPool pool;
+  ArenaAllocator arena(&pool);
+  HGraphBuilder builder(&arena);
+  const DexFile::CodeItem* item = reinterpret_cast<const DexFile::CodeItem*>(data);
+  HGraph* graph = builder.BuildGraph(*item);
+  // Remove suspend checks, they cannot be executed in this context.
+  RemoveSuspendChecks(graph);
+  ASSERT_NE(graph, nullptr);
+  RunCodeBaseline(graph, has_result, expected);
+}
+
 TEST(CodegenTest, ReturnVoid) {
   const uint16_t data[] = ZERO_REGISTER_CODE_ITEM(Instruction::RETURN_VOID);
   TestCode(data);
@@ -256,4 +298,55 @@ TEST(CodegenTest, ReturnAdd4) {
   TestCode(data, true, 7);
 }
 
+TEST(CodegenTest, NonMaterializedCondition) {
+  ArenaPool pool;
+  ArenaAllocator allocator(&pool);
+
+  HGraph* graph = new (&allocator) HGraph(&allocator);
+  HBasicBlock* entry = new (&allocator) HBasicBlock(graph);
+  graph->AddBlock(entry);
+  graph->SetEntryBlock(entry);
+  entry->AddInstruction(new (&allocator) HGoto());
+
+  HBasicBlock* first_block = new (&allocator) HBasicBlock(graph);
+  graph->AddBlock(first_block);
+  entry->AddSuccessor(first_block);
+  HIntConstant* constant0 = new (&allocator) HIntConstant(0);
+  entry->AddInstruction(constant0);
+  HIntConstant* constant1 = new (&allocator) HIntConstant(1);
+  entry->AddInstruction(constant1);
+  HEqual* equal = new (&allocator) HEqual(constant0, constant0);
+  first_block->AddInstruction(equal);
+  first_block->AddInstruction(new (&allocator) HIf(equal));
+
+  HBasicBlock* then = new (&allocator) HBasicBlock(graph);
+  HBasicBlock* else_ = new (&allocator) HBasicBlock(graph);
+  HBasicBlock* exit = new (&allocator) HBasicBlock(graph);
+
+  graph->AddBlock(then);
+  graph->AddBlock(else_);
+  graph->AddBlock(exit);
+  first_block->AddSuccessor(then);
+  first_block->AddSuccessor(else_);
+  then->AddSuccessor(exit);
+  else_->AddSuccessor(exit);
+
+  exit->AddInstruction(new (&allocator) HExit());
+  then->AddInstruction(new (&allocator) HReturn(constant0));
+  else_->AddInstruction(new (&allocator) HReturn(constant1));
+
+  ASSERT_TRUE(equal->NeedsMaterialization());
+  graph->BuildDominatorTree();
+  PrepareForRegisterAllocation(graph).Run();
+  ASSERT_FALSE(equal->NeedsMaterialization());
+
+  auto hook_before_codegen = [](HGraph* graph) {
+    HBasicBlock* block = graph->GetEntryBlock()->GetSuccessors().Get(0);
+    HParallelMove* move = new (graph->GetArena()) HParallelMove(graph->GetArena());
+    block->InsertInstructionBefore(move, block->GetLastInstruction());
+  };
+
+  RunCodeOptimized(graph, hook_before_codegen, true, 0);
+}
+
 }  // namespace art
index 8be4746..eafc3e1 100644 (file)
@@ -21,6 +21,7 @@
 #include "dex_instruction.h"
 #include "nodes.h"
 #include "optimizing_unit_test.h"
+#include "prepare_for_register_allocation.h"
 #include "ssa_liveness_analysis.h"
 #include "utils/arena_allocator.h"
 
@@ -38,6 +39,8 @@ static HGraph* BuildGraph(const uint16_t* data, ArenaAllocator* allocator) {
   graph->BuildDominatorTree();
   graph->TransformToSSA();
   graph->FindNaturalLoops();
+  // `Inline` conditions into ifs.
+  PrepareForRegisterAllocation(graph).Run();
   return graph;
 }
 
index 2d86169..246e7ef 100644 (file)
@@ -21,6 +21,7 @@
 #include "dex_instruction.h"
 #include "nodes.h"
 #include "optimizing_unit_test.h"
+#include "prepare_for_register_allocation.h"
 #include "ssa_liveness_analysis.h"
 #include "utils/arena_allocator.h"
 
@@ -50,6 +51,8 @@ static void TestCode(const uint16_t* data, const char* expected) {
   graph->BuildDominatorTree();
   graph->TransformToSSA();
   graph->FindNaturalLoops();
+  // `Inline` conditions into ifs.
+  PrepareForRegisterAllocation(graph).Run();
   x86::CodeGeneratorX86 codegen(graph);
   SsaLivenessAnalysis liveness(*graph, &codegen);
   liveness.Analyze();
index 4cac319..3a0b40c 100644 (file)
@@ -537,7 +537,7 @@ void HPhi::AddInput(HInstruction* input) {
   input->AddUseAt(this, inputs_.Size() - 1);
 }
 
-#define DEFINE_ACCEPT(name)                                                    \
+#define DEFINE_ACCEPT(name, super)                                             \
 void H##name::Accept(HGraphVisitor* visitor) {                                 \
   visitor->Visit##name(this);                                                  \
 }
@@ -575,24 +575,6 @@ HConstant* HBinaryOperation::TryStaticEvaluation(ArenaAllocator* allocator) cons
   return nullptr;
 }
 
-bool HCondition::NeedsMaterialization() const {
-  if (!HasOnlyOneUse()) {
-    return true;
-  }
-  HUseListNode<HInstruction>* uses = GetUses();
-  HInstruction* user = uses->GetUser();
-  if (!user->IsIf()) {
-    return true;
-  }
-
-  // TODO: if there is no intervening instructions with side-effect between this condition
-  // and the If instruction, we should move the condition just before the If.
-  if (GetNext() != user) {
-    return true;
-  }
-  return false;
-}
-
 bool HCondition::IsBeforeWhenDisregardMoves(HIf* if_) const {
   HInstruction* previous = if_->GetPrevious();
   while (previous != nullptr && previous->IsParallelMove()) {
index fc5b06d..41713a4 100644 (file)
@@ -465,50 +465,51 @@ class HBasicBlock : public ArenaObject {
   DISALLOW_COPY_AND_ASSIGN(HBasicBlock);
 };
 
-#define FOR_EACH_CONCRETE_INSTRUCTION(M)                   \
-  M(Add)                                                   \
-  M(Condition)                                             \
-  M(Equal)                                                 \
-  M(NotEqual)                                              \
-  M(LessThan)                                              \
-  M(LessThanOrEqual)                                       \
-  M(GreaterThan)                                           \
-  M(GreaterThanOrEqual)                                    \
-  M(Exit)                                                  \
-  M(Goto)                                                  \
-  M(If)                                                    \
-  M(IntConstant)                                           \
-  M(InvokeStatic)                                          \
-  M(InvokeVirtual)                                         \
-  M(LoadLocal)                                             \
-  M(Local)                                                 \
-  M(LongConstant)                                          \
-  M(NewInstance)                                           \
-  M(Not)                                                   \
-  M(ParameterValue)                                        \
-  M(ParallelMove)                                          \
-  M(Phi)                                                   \
-  M(Return)                                                \
-  M(ReturnVoid)                                            \
-  M(StoreLocal)                                            \
-  M(Sub)                                                   \
-  M(Compare)                                               \
-  M(InstanceFieldGet)                                      \
-  M(InstanceFieldSet)                                      \
-  M(ArrayGet)                                              \
-  M(ArraySet)                                              \
-  M(ArrayLength)                                           \
-  M(BoundsCheck)                                           \
-  M(NullCheck)                                             \
-  M(Temporary)                                             \
-  M(SuspendCheck)                                          \
-
-#define FOR_EACH_INSTRUCTION(M)                            \
-  FOR_EACH_CONCRETE_INSTRUCTION(M)                         \
-  M(Constant)                                              \
-  M(BinaryOperation)
-
-#define FORWARD_DECLARATION(type) class H##type;
+#define FOR_EACH_CONCRETE_INSTRUCTION(M)                                \
+  M(Add, BinaryOperation)                                               \
+  M(Condition, BinaryOperation)                                         \
+  M(Equal, Condition)                                                   \
+  M(NotEqual, Condition)                                                \
+  M(LessThan, Condition)                                                \
+  M(LessThanOrEqual, Condition)                                         \
+  M(GreaterThan, Condition)                                             \
+  M(GreaterThanOrEqual, Condition)                                      \
+  M(Exit, Instruction)                                                  \
+  M(Goto, Instruction)                                                  \
+  M(If, Instruction)                                                    \
+  M(IntConstant, Constant)                                              \
+  M(InvokeStatic, Invoke)                                               \
+  M(InvokeVirtual, Invoke)                                              \
+  M(LoadLocal, Instruction)                                             \
+  M(Local, Instruction)                                                 \
+  M(LongConstant, Constant)                                             \
+  M(NewInstance, Instruction)                                           \
+  M(Not, Instruction)                                                   \
+  M(ParameterValue, Instruction)                                        \
+  M(ParallelMove, Instruction)                                          \
+  M(Phi, Instruction)                                                   \
+  M(Return, Instruction)                                                \
+  M(ReturnVoid, Instruction)                                            \
+  M(StoreLocal, Instruction)                                            \
+  M(Sub, BinaryOperation)                                               \
+  M(Compare, BinaryOperation)                                           \
+  M(InstanceFieldGet, Instruction)                                      \
+  M(InstanceFieldSet, Instruction)                                      \
+  M(ArrayGet, Instruction)                                              \
+  M(ArraySet, Instruction)                                              \
+  M(ArrayLength, Instruction)                                           \
+  M(BoundsCheck, Instruction)                                           \
+  M(NullCheck, Instruction)                                             \
+  M(Temporary, Instruction)                                             \
+  M(SuspendCheck, Instruction)                                          \
+
+#define FOR_EACH_INSTRUCTION(M)                                         \
+  FOR_EACH_CONCRETE_INSTRUCTION(M)                                      \
+  M(Constant, Instruction)                                              \
+  M(BinaryOperation, Instruction) \
+  M(Invoke, Instruction)
+
+#define FORWARD_DECLARATION(type, super) class H##type;
 FOR_EACH_INSTRUCTION(FORWARD_DECLARATION)
 #undef FORWARD_DECLARATION
 
@@ -623,7 +624,7 @@ class HInstruction : public ArenaObject {
 
   virtual ~HInstruction() {}
 
-#define DECLARE_KIND(type) k##type,
+#define DECLARE_KIND(type, super) k##type,
   enum InstructionKind {
     FOR_EACH_INSTRUCTION(DECLARE_KIND)
   };
@@ -709,7 +710,7 @@ class HInstruction : public ArenaObject {
     return uses_ != nullptr && uses_->GetTail() == nullptr;
   }
 
-#define INSTRUCTION_TYPE_CHECK(type)                                           \
+#define INSTRUCTION_TYPE_CHECK(type, super)                                    \
   bool Is##type() const { return (As##type() != nullptr); }                    \
   virtual const H##type* As##type() const { return nullptr; }                  \
   virtual H##type* As##type() { return nullptr; }
@@ -1118,13 +1119,13 @@ class HBinaryOperation : public HExpression<2> {
 class HCondition : public HBinaryOperation {
  public:
   HCondition(HInstruction* first, HInstruction* second)
-      : HBinaryOperation(Primitive::kPrimBoolean, first, second) {}
+      : HBinaryOperation(Primitive::kPrimBoolean, first, second),
+        needs_materialization_(true) {}
 
   virtual bool IsCommutative() { return true; }
 
-  // For register allocation purposes, returns whether this instruction needs to be
-  // materialized (that is, not just be in the processor flags).
-  bool NeedsMaterialization() const;
+  bool NeedsMaterialization() const { return needs_materialization_; }
+  void ClearNeedsMaterialization() { needs_materialization_ = false; }
 
   // For code generation purposes, returns whether this instruction is just before
   // `if_`, and disregard moves in between.
@@ -1135,6 +1136,10 @@ class HCondition : public HBinaryOperation {
   virtual IfCondition GetCondition() const = 0;
 
  private:
+  // For register allocation purposes, returns whether this instruction needs to be
+  // materialized (that is, not just be in the processor flags).
+  bool needs_materialization_;
+
   DISALLOW_COPY_AND_ASSIGN(HCondition);
 };
 
@@ -1437,6 +1442,8 @@ class HInvoke : public HInstruction {
 
   uint32_t GetDexPc() const { return dex_pc_; }
 
+  DECLARE_INSTRUCTION(Invoke);
+
  protected:
   GrowableArray<HInstruction*> inputs_;
   const Primitive::Type return_type_;
@@ -1954,7 +1961,7 @@ class HGraphVisitor : public ValueObject {
   HGraph* GetGraph() const { return graph_; }
 
   // Visit functions for instruction classes.
-#define DECLARE_VISIT_INSTRUCTION(name)                                        \
+#define DECLARE_VISIT_INSTRUCTION(name, super)                                        \
   virtual void Visit##name(H##name* instr) { VisitInstruction(instr); }
 
   FOR_EACH_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
@@ -1967,6 +1974,23 @@ class HGraphVisitor : public ValueObject {
   DISALLOW_COPY_AND_ASSIGN(HGraphVisitor);
 };
 
+class HGraphDelegateVisitor : public HGraphVisitor {
+ public:
+  explicit HGraphDelegateVisitor(HGraph* graph) : HGraphVisitor(graph) {}
+  virtual ~HGraphDelegateVisitor() {}
+
+  // Visit functions that delegate to to super class.
+#define DECLARE_VISIT_INSTRUCTION(name, super)                                        \
+  virtual void Visit##name(H##name* instr) OVERRIDE { Visit##super(instr); }
+
+  FOR_EACH_INSTRUCTION(DECLARE_VISIT_INSTRUCTION)
+
+#undef DECLARE_VISIT_INSTRUCTION
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(HGraphDelegateVisitor);
+};
+
 class HInsertionOrderIterator : public ValueObject {
  public:
   explicit HInsertionOrderIterator(const HGraph& graph) : graph_(graph), index_(0) {}
index bfbbab5..a81dc1b 100644 (file)
@@ -37,4 +37,26 @@ void PrepareForRegisterAllocation::VisitBoundsCheck(HBoundsCheck* check) {
   check->ReplaceWith(check->InputAt(0));
 }
 
+void PrepareForRegisterAllocation::VisitCondition(HCondition* condition) {
+  bool needs_materialization = false;
+  if (!condition->HasOnlyOneUse()) {
+    needs_materialization = true;
+  } else {
+    HUseListNode<HInstruction>* uses = condition->GetUses();
+    HInstruction* user = uses->GetUser();
+    if (!user->IsIf()) {
+      needs_materialization = true;
+    } else {
+      // TODO: if there is no intervening instructions with side-effect between this condition
+      // and the If instruction, we should move the condition just before the If.
+      if (condition->GetNext() != user) {
+        needs_materialization = true;
+      }
+    }
+  }
+  if (!needs_materialization) {
+    condition->ClearNeedsMaterialization();
+  }
+}
+
 }  // namespace art
index 37f2871..e86a39b 100644 (file)
@@ -26,15 +26,16 @@ namespace art {
  * For example it changes uses of null checks and bounds checks to the original
  * objects, to avoid creating a live range for these checks.
  */
-class PrepareForRegisterAllocation : public HGraphVisitor {
+class PrepareForRegisterAllocation : public HGraphDelegateVisitor {
  public:
-  explicit PrepareForRegisterAllocation(HGraph* graph) : HGraphVisitor(graph) {}
+  explicit PrepareForRegisterAllocation(HGraph* graph) : HGraphDelegateVisitor(graph) {}
 
   void Run();
 
  private:
   virtual void VisitNullCheck(HNullCheck* check) OVERRIDE;
   virtual void VisitBoundsCheck(HBoundsCheck* check) OVERRIDE;
+  virtual void VisitCondition(HCondition* condition) OVERRIDE;
 
   DISALLOW_COPY_AND_ASSIGN(PrepareForRegisterAllocation);
 };
index a9d159e..c9c3d03 100644 (file)
@@ -821,6 +821,11 @@ void RegisterAllocator::InsertParallelMoveAtExitOf(HBasicBlock* block,
 
   DCHECK_EQ(block->GetSuccessors().Size(), 1u);
   HInstruction* last = block->GetLastInstruction();
+  // We insert moves at exit for phi predecessors and connecting blocks.
+  // A block ending with an if cannot branch to a block with phis because
+  // we do not allow critical edges. It can also not connect
+  // a split interval between two blocks: the move has to happen in the successor.
+  DCHECK(!last->IsIf());
   HInstruction* previous = last->GetPrevious();
   HParallelMove* move;
   // This is a parallel move for connecting blocks. We need to differentiate