From bed50d2430e02a3d6b94972e8ab4873d7b3b8be0 Mon Sep 17 00:00:00 2001 From: Roland Levillain Date: Fri, 10 Jul 2015 17:18:11 +0000 Subject: [PATCH] Revert "Fuse long and FP compare & condition on ARM64 in Optimizing." This reverts commit 5cfe61f27ed9203498169355bb95db756486d292. Change-Id: I9879e76e7f8315cace05700e3b571a6a4749bf1a --- compiler/optimizing/code_generator_arm64.cc | 150 +++++++++----------------- compiler/optimizing/instruction_simplifier.cc | 6 +- 2 files changed, 52 insertions(+), 104 deletions(-) diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 4882981e9..a9a95d364 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -77,9 +77,10 @@ inline Condition ARM64Condition(IfCondition cond) { case kCondLE: return le; case kCondGT: return gt; case kCondGE: return ge; + default: + LOG(FATAL) << "Unknown if condition"; } - LOG(FATAL) << "Unreachable"; - UNREACHABLE(); + return nv; // Unreachable. } Location ARM64ReturnLocation(Primitive::Type return_type) { @@ -1644,11 +1645,6 @@ void InstructionCodeGeneratorARM64::VisitClinitCheck(HClinitCheck* check) { GenerateClassInitializationCheck(slow_path, InputRegisterAt(check, 0)); } -static bool IsFloatingPointZeroConstant(HInstruction* instruction) { - return (instruction->IsFloatConstant() && (instruction->AsFloatConstant()->GetValue() == 0.0f)) - || (instruction->IsDoubleConstant() && (instruction->AsDoubleConstant()->GetValue() == 0.0)); -} - void LocationsBuilderARM64::VisitCompare(HCompare* compare) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(compare, LocationSummary::kNoCall); @@ -1663,10 +1659,13 @@ void LocationsBuilderARM64::VisitCompare(HCompare* compare) { case Primitive::kPrimFloat: case Primitive::kPrimDouble: { locations->SetInAt(0, Location::RequiresFpuRegister()); - locations->SetInAt(1, - IsFloatingPointZeroConstant(compare->InputAt(1)) - ? Location::ConstantLocation(compare->InputAt(1)->AsConstant()) - : Location::RequiresFpuRegister()); + HInstruction* right = compare->InputAt(1); + if ((right->IsFloatConstant() && (right->AsFloatConstant()->GetValue() == 0.0f)) || + (right->IsDoubleConstant() && (right->AsDoubleConstant()->GetValue() == 0.0))) { + locations->SetInAt(1, Location::ConstantLocation(right->AsConstant())); + } else { + locations->SetInAt(1, Location::RequiresFpuRegister()); + } locations->SetOut(Location::RequiresRegister()); break; } @@ -1697,8 +1696,12 @@ void InstructionCodeGeneratorARM64::VisitCompare(HCompare* compare) { Register result = OutputRegister(compare); FPRegister left = InputFPRegisterAt(compare, 0); if (compare->GetLocations()->InAt(1).IsConstant()) { - DCHECK(IsFloatingPointZeroConstant(compare->GetLocations()->InAt(1).GetConstant())); - // 0.0 is the only immediate that can be encoded directly in an FCMP instruction. + if (kIsDebugBuild) { + HInstruction* right = compare->GetLocations()->InAt(1).GetConstant(); + DCHECK((right->IsFloatConstant() && (right->AsFloatConstant()->GetValue() == 0.0f)) || + (right->IsDoubleConstant() && (right->AsDoubleConstant()->GetValue() == 0.0))); + } + // 0.0 is the only immediate that can be encoded directly in a FCMP instruction. __ Fcmp(left, 0.0); } else { __ Fcmp(left, InputFPRegisterAt(compare, 1)); @@ -1718,19 +1721,8 @@ void InstructionCodeGeneratorARM64::VisitCompare(HCompare* compare) { void LocationsBuilderARM64::VisitCondition(HCondition* instruction) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction); - - if (Primitive::IsFloatingPointType(instruction->InputAt(0)->GetType())) { - locations->SetInAt(0, Location::RequiresFpuRegister()); - locations->SetInAt(1, - IsFloatingPointZeroConstant(instruction->InputAt(1)) - ? Location::ConstantLocation(instruction->InputAt(1)->AsConstant()) - : Location::RequiresFpuRegister()); - } else { - // Integer cases. - locations->SetInAt(0, Location::RequiresRegister()); - locations->SetInAt(1, ARM64EncodableConstantOrRegister(instruction->InputAt(1), instruction)); - } - + locations->SetInAt(0, Location::RequiresRegister()); + locations->SetInAt(1, ARM64EncodableConstantOrRegister(instruction->InputAt(1), instruction)); if (instruction->NeedsMaterialization()) { locations->SetOut(Location::RequiresRegister(), Location::kNoOutputOverlap); } @@ -1742,32 +1734,13 @@ void InstructionCodeGeneratorARM64::VisitCondition(HCondition* instruction) { } LocationSummary* locations = instruction->GetLocations(); + Register lhs = InputRegisterAt(instruction, 0); + Operand rhs = InputOperandAt(instruction, 1); Register res = RegisterFrom(locations->Out(), instruction->GetType()); - IfCondition if_cond = instruction->GetCondition(); - Condition arm64_cond = ARM64Condition(if_cond); - - if (Primitive::IsFloatingPointType(instruction->InputAt(0)->GetType())) { - FPRegister lhs = InputFPRegisterAt(instruction, 0); - if (locations->InAt(1).IsConstant()) { - DCHECK(IsFloatingPointZeroConstant(locations->InAt(1).GetConstant())); - // 0.0 is the only immediate that can be encoded directly in an FCMP instruction. - __ Fcmp(lhs, 0.0); - } else { - __ Fcmp(lhs, InputFPRegisterAt(instruction, 1)); - } - __ Cset(res, arm64_cond); - if (instruction->IsFPConditionTrueIfNaN()) { - __ Csel(res, res, Operand(1), vs); // VS for unordered. - } else if (instruction->IsFPConditionFalseIfNaN()) { - __ Csel(res, res, Operand(0), vs); // VS for unordered. - } - } else { - // Integer cases. - Register lhs = InputRegisterAt(instruction, 0); - Operand rhs = InputOperandAt(instruction, 1); - __ Cmp(lhs, rhs); - __ Cset(res, arm64_cond); - } + Condition cond = ARM64Condition(instruction->GetCondition()); + + __ Cmp(lhs, rhs); + __ Cset(res, cond); } #define FOR_EACH_CONDITION_INSTRUCTION(M) \ @@ -2099,58 +2072,33 @@ void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruct } else { // The condition instruction has not been materialized, use its inputs as // the comparison and its condition as the branch condition. - Primitive::Type type = - cond->IsCondition() ? cond->InputAt(0)->GetType() : Primitive::kPrimInt; - - if (Primitive::IsFloatingPointType(type)) { - // FP compares don't like null false_targets. - if (false_target == nullptr) { - false_target = codegen_->GetLabelOf(instruction->AsIf()->IfFalseSuccessor()); - } - FPRegister lhs = InputFPRegisterAt(condition, 0); - if (condition->GetLocations()->InAt(1).IsConstant()) { - DCHECK(IsFloatingPointZeroConstant(condition->GetLocations()->InAt(1).GetConstant())); - // 0.0 is the only immediate that can be encoded directly in an FCMP instruction. - __ Fcmp(lhs, 0.0); - } else { - __ Fcmp(lhs, InputFPRegisterAt(condition, 1)); - } - if (condition->IsFPConditionTrueIfNaN()) { - __ B(vs, true_target); // VS for unordered. - } else if (condition->IsFPConditionFalseIfNaN()) { - __ B(vs, false_target); // VS for unordered. + Register lhs = InputRegisterAt(condition, 0); + Operand rhs = InputOperandAt(condition, 1); + Condition arm64_cond = ARM64Condition(condition->GetCondition()); + if ((arm64_cond != gt && arm64_cond != le) && rhs.IsImmediate() && (rhs.immediate() == 0)) { + switch (arm64_cond) { + case eq: + __ Cbz(lhs, true_target); + break; + case ne: + __ Cbnz(lhs, true_target); + break; + case lt: + // Test the sign bit and branch accordingly. + __ Tbnz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, true_target); + break; + case ge: + // Test the sign bit and branch accordingly. + __ Tbz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, true_target); + break; + default: + // Without the `static_cast` the compiler throws an error for + // `-Werror=sign-promo`. + LOG(FATAL) << "Unexpected condition: " << static_cast(arm64_cond); } - __ B(ARM64Condition(condition->GetCondition()), true_target); } else { - // Integer cases. - Register lhs = InputRegisterAt(condition, 0); - Operand rhs = InputOperandAt(condition, 1); - Condition arm64_cond = ARM64Condition(condition->GetCondition()); - if ((arm64_cond != gt && arm64_cond != le) && rhs.IsImmediate() && (rhs.immediate() == 0)) { - switch (arm64_cond) { - case eq: - __ Cbz(lhs, true_target); - break; - case ne: - __ Cbnz(lhs, true_target); - break; - case lt: - // Test the sign bit and branch accordingly. - __ Tbnz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, true_target); - break; - case ge: - // Test the sign bit and branch accordingly. - __ Tbz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, true_target); - break; - default: - // Without the `static_cast` the compiler throws an error for - // `-Werror=sign-promo`. - LOG(FATAL) << "Unexpected condition: " << static_cast(arm64_cond); - } - } else { - __ Cmp(lhs, rhs); - __ B(arm64_cond, true_target); - } + __ Cmp(lhs, rhs); + __ B(arm64_cond, true_target); } } if (false_target != nullptr) { diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc index 04ba4e18c..017b6781a 100644 --- a/compiler/optimizing/instruction_simplifier.cc +++ b/compiler/optimizing/instruction_simplifier.cc @@ -517,10 +517,10 @@ void InstructionSimplifierVisitor::VisitLessThanOrEqual(HLessThanOrEqual* condit void InstructionSimplifierVisitor::VisitCondition(HCondition* condition) { // Try to fold an HCompare into this HCondition. - // This simplification is currently supported on x86, x86_64, ARM and ARM64. - // TODO: Implement it for MIPS64. + // This simplification is currently only supported on x86, x86_64 and ARM. + // TODO: Implement it for ARM64 and MIPS64. InstructionSet instruction_set = GetGraph()->GetInstructionSet(); - if (instruction_set == kMips64) { + if (instruction_set != kX86 && instruction_set != kX86_64 && instruction_set != kThumb2) { return; } -- 2.11.0