Fix code generation with materialized conditions.
Change-Id: I8630af3c13fc1950d3fa718d7488407b00898796
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index ad62279..804d352 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -577,7 +577,7 @@
DCHECK(cond->IsCondition());
HCondition* condition = cond->AsCondition();
if (condition->NeedsMaterialization()) {
- locations->SetInAt(0, Location::Any());
+ locations->SetInAt(0, Location::RequiresRegister());
}
}
@@ -590,7 +590,7 @@
DCHECK(if_instr->GetLocations()->InAt(0).IsRegister());
__ cmp(if_instr->GetLocations()->InAt(0).AsArm().AsCoreRegister(),
ShifterOperand(0));
- __ b(codegen_->GetLabelOf(if_instr->IfTrueSuccessor()), EQ);
+ __ b(codegen_->GetLabelOf(if_instr->IfTrueSuccessor()), NE);
} else {
// Condition has not been materialized, use its inputs as the comparison and its
// condition as the branch condition.
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 3383cb2..f7b8495 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -541,14 +541,18 @@
DCHECK(cond->IsCondition());
HCondition* condition = cond->AsCondition();
if (condition->NeedsMaterialization()) {
- // Materialized condition, compare against 0
- Location lhs = if_instr->GetLocations()->InAt(0);
- if (lhs.IsRegister()) {
- __ cmpl(lhs.AsX86().AsCpuRegister(), Immediate(0));
- } else {
- __ cmpl(Address(ESP, lhs.GetStackIndex()), Immediate(0));
+ // 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 (!condition->IsBeforeWhenDisregardMoves(if_instr)) {
+ // Materialized condition, compare against 0
+ Location lhs = if_instr->GetLocations()->InAt(0);
+ if (lhs.IsRegister()) {
+ __ cmpl(lhs.AsX86().AsCpuRegister(), Immediate(0));
+ } else {
+ __ cmpl(Address(ESP, lhs.GetStackIndex()), Immediate(0));
+ }
}
- __ j(kEqual, codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
+ __ j(kNotEqual, codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
} else {
Location lhs = condition->GetLocations()->InAt(0);
Location rhs = condition->GetLocations()->InAt(1);
@@ -625,6 +629,9 @@
void InstructionCodeGeneratorX86::VisitCondition(HCondition* comp) {
if (comp->NeedsMaterialization()) {
LocationSummary* locations = comp->GetLocations();
+ Register reg = locations->Out().AsX86().AsCpuRegister();
+ // Clear register: setcc only sets the low byte.
+ __ xorl(reg, reg);
if (locations->InAt(1).IsRegister()) {
__ cmpl(locations->InAt(0).AsX86().AsCpuRegister(),
locations->InAt(1).AsX86().AsCpuRegister());
@@ -636,7 +643,7 @@
__ cmpl(locations->InAt(0).AsX86().AsCpuRegister(),
Address(ESP, locations->InAt(1).GetStackIndex()));
}
- __ setb(X86Condition(comp->GetCondition()), locations->Out().AsX86().AsCpuRegister());
+ __ setb(X86Condition(comp->GetCondition()), reg);
}
}
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index ca03af8..283d850 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -424,14 +424,18 @@
DCHECK(cond->IsCondition());
HCondition* condition = cond->AsCondition();
if (condition->NeedsMaterialization()) {
- // Materialized condition, compare against 0.
- Location lhs = if_instr->GetLocations()->InAt(0);
- if (lhs.IsRegister()) {
- __ cmpl(lhs.AsX86_64().AsCpuRegister(), Immediate(0));
- } else {
- __ cmpl(Address(CpuRegister(RSP), lhs.GetStackIndex()), Immediate(0));
+ // 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 (!condition->IsBeforeWhenDisregardMoves(if_instr)) {
+ // Materialized condition, compare against 0.
+ Location lhs = if_instr->GetLocations()->InAt(0);
+ if (lhs.IsRegister()) {
+ __ cmpl(lhs.AsX86_64().AsCpuRegister(), Immediate(0));
+ } else {
+ __ cmpl(Address(CpuRegister(RSP), lhs.GetStackIndex()), Immediate(0));
+ }
}
- __ j(kEqual, codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
+ __ j(kNotEqual, codegen_->GetLabelOf(if_instr->IfTrueSuccessor()));
} else {
Location lhs = condition->GetLocations()->InAt(0);
Location rhs = condition->GetLocations()->InAt(1);
@@ -505,6 +509,9 @@
void InstructionCodeGeneratorX86_64::VisitCondition(HCondition* comp) {
if (comp->NeedsMaterialization()) {
LocationSummary* locations = comp->GetLocations();
+ CpuRegister reg = locations->Out().AsX86_64().AsCpuRegister();
+ // Clear register: setcc only sets the low byte.
+ __ xorq(reg, reg);
if (locations->InAt(1).IsRegister()) {
__ cmpq(locations->InAt(0).AsX86_64().AsCpuRegister(),
locations->InAt(1).AsX86_64().AsCpuRegister());
@@ -515,8 +522,7 @@
__ cmpq(locations->InAt(0).AsX86_64().AsCpuRegister(),
Address(CpuRegister(RSP), locations->InAt(1).GetStackIndex()));
}
- __ setcc(X86_64Condition(comp->GetCondition()),
- comp->GetLocations()->Out().AsX86_64().AsCpuRegister());
+ __ setcc(X86_64Condition(comp->GetCondition()), reg);
}
}
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 7f64be4..0fb4737 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -82,6 +82,8 @@
}
char GetTypeId(Primitive::Type type) {
+ // Note that Primitive::Descriptor would not work for us
+ // because it does not handle reference types (that is kPrimNot).
switch (type) {
case Primitive::kPrimBoolean: return 'z';
case Primitive::kPrimByte: return 'b';
@@ -127,6 +129,12 @@
}
} else if (location.IsConstant()) {
output_ << "constant";
+ HConstant* constant = location.GetConstant();
+ if (constant->IsIntConstant()) {
+ output_ << " " << constant->AsIntConstant()->GetValue();
+ } else if (constant->IsLongConstant()) {
+ output_ << " " << constant->AsLongConstant()->GetValue();
+ }
} else if (location.IsInvalid()) {
output_ << "invalid";
} else if (location.IsStackSlot()) {
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 1a24677..73936e7 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -534,14 +534,22 @@
return true;
}
- // TODO: should we allow intervening instructions with no side-effect between this condition
- // and the If instruction?
+ // 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()) {
+ previous = previous->GetPrevious();
+ }
+ return previous == this;
+}
+
bool HInstruction::Equals(HInstruction* other) const {
if (!InstructionTypeEquals(other)) return false;
DCHECK_EQ(GetKind(), other->GetKind());
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index af173c8..d3ae7c3 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -1087,8 +1087,15 @@
: HBinaryOperation(Primitive::kPrimBoolean, first, second) {}
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;
+ // For code generation purposes, returns whether this instruction is just before
+ // `if_`, and disregard moves in between.
+ bool IsBeforeWhenDisregardMoves(HIf* if_) const;
+
DECLARE_INSTRUCTION(Condition);
virtual IfCondition GetCondition() const = 0;