diff options
| author | David Brazdil <dbrazdil@google.com> | 2015-11-12 18:37:00 +0000 |
|---|---|---|
| committer | David Brazdil <dbrazdil@google.com> | 2015-11-17 15:12:00 +0000 |
| commit | 0debae7bc89eb05f7a2bf7dccd223318fad7c88d (patch) | |
| tree | 3da9656dc77d271b12c1b02793e7d713ae3da790 | |
| parent | 991842a5273d20695dd5f35eb9a9e28c386c5b4b (diff) | |
| download | art-0debae7bc89eb05f7a2bf7dccd223318fad7c88d.tar.gz art-0debae7bc89eb05f7a2bf7dccd223318fad7c88d.tar.bz2 art-0debae7bc89eb05f7a2bf7dccd223318fad7c88d.zip | |
ART: Refactor GenerateTestAndBranch
Each code generator implements a method for generating condition
evaluation and branching to arbitrary labels. This patch refactors
it for better clarity but also to generate fewer jumps when the true
branch is the fallthrough successor.
This is preliminary work for implementing HSelect.
Change-Id: Iaa545a5ecbacb761c5aa241fa69140cf6eb5952f
| -rw-r--r-- | compiler/optimizing/code_generator_arm.cc | 179 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_arm.h | 10 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_arm64.cc | 132 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_arm64.h | 4 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_mips.cc | 139 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_mips.h | 4 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_mips64.cc | 138 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_mips64.h | 4 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_utils.cc | 15 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_utils.h | 12 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_x86.cc | 226 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_x86.h | 10 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_x86_64.cc | 222 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_x86_64.h | 10 |
14 files changed, 587 insertions, 518 deletions
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index 54c6cc8890..655bbb8a8e 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -1240,26 +1240,19 @@ void InstructionCodeGeneratorARM::GenerateLongComparesAndJumps(HCondition* cond, __ b(true_label, final_condition); } -void InstructionCodeGeneratorARM::GenerateCompareTestAndBranch(HIf* if_instr, - HCondition* condition, - Label* true_target, - Label* false_target, - Label* always_true_target) { +void InstructionCodeGeneratorARM::GenerateCompareTestAndBranch(HCondition* condition, + Label* true_target_in, + Label* false_target_in) { + // Generated branching requires both targets to be explicit. If either of the + // targets is nullptr (fallthrough) use and bind `fallthrough_target` instead. + Label fallthrough_target; + Label* true_target = true_target_in == nullptr ? &fallthrough_target : true_target_in; + Label* false_target = false_target_in == nullptr ? &fallthrough_target : false_target_in; + LocationSummary* locations = condition->GetLocations(); Location left = locations->InAt(0); Location right = locations->InAt(1); - // We don't want true_target as a nullptr. - if (true_target == nullptr) { - true_target = always_true_target; - } - bool falls_through = (false_target == nullptr); - - // FP compares don't like null false_targets. - if (false_target == nullptr) { - false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - } - Primitive::Type type = condition->InputAt(0)->GetType(); switch (type) { case Primitive::kPrimLong: @@ -1278,117 +1271,125 @@ void InstructionCodeGeneratorARM::GenerateCompareTestAndBranch(HIf* if_instr, LOG(FATAL) << "Unexpected compare type " << type; } - if (!falls_through) { + if (false_target != &fallthrough_target) { __ b(false_target); } + + if (fallthrough_target.IsLinked()) { + __ Bind(&fallthrough_target); + } } void InstructionCodeGeneratorARM::GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target) { - HInstruction* cond = instruction->InputAt(0); - if (cond->IsIntConstant()) { + Label* false_target) { + HInstruction* cond = instruction->InputAt(condition_input_index); + + if (true_target == nullptr && false_target == nullptr) { + // Nothing to do. The code always falls through. + return; + } else if (cond->IsIntConstant()) { // Constant condition, statically compared against 1. - int32_t cond_value = cond->AsIntConstant()->GetValue(); - if (cond_value == 1) { - if (always_true_target != nullptr) { - __ b(always_true_target); + if (cond->AsIntConstant()->IsOne()) { + if (true_target != nullptr) { + __ b(true_target); } - return; } else { - DCHECK_EQ(cond_value, 0); + DCHECK(cond->AsIntConstant()->IsZero()); + if (false_target != nullptr) { + __ b(false_target); + } + } + return; + } + + // The following code generates these patterns: + // (1) true_target == nullptr && false_target != nullptr + // - opposite condition true => branch to false_target + // (2) true_target != nullptr && false_target == nullptr + // - condition true => branch to true_target + // (3) true_target != nullptr && false_target != nullptr + // - condition true => branch to true_target + // - branch to false_target + if (IsBooleanValueOrMaterializedCondition(cond)) { + // Condition has been materialized, compare the output to 0. + Location cond_val = instruction->GetLocations()->InAt(condition_input_index); + DCHECK(cond_val.IsRegister()); + if (true_target == nullptr) { + __ CompareAndBranchIfZero(cond_val.AsRegister<Register>(), false_target); + } else { + __ CompareAndBranchIfNonZero(cond_val.AsRegister<Register>(), true_target); } } else { - // Can we optimize the jump if we know that the next block is the true case? + // Condition has not been materialized. Use its inputs as the comparison and + // its condition as the branch condition. HCondition* condition = cond->AsCondition(); - bool can_jump_to_false = CanReverseCondition(always_true_target, false_target, condition); - if (condition == nullptr || condition->NeedsMaterialization()) { - // Condition has been materialized, compare the output to 0. - DCHECK(instruction->GetLocations()->InAt(0).IsRegister()); - if (can_jump_to_false) { - __ CompareAndBranchIfZero(instruction->GetLocations()->InAt(0).AsRegister<Register>(), - false_target); - return; - } - __ CompareAndBranchIfNonZero(instruction->GetLocations()->InAt(0).AsRegister<Register>(), - true_target); - } else { - // Condition has not been materialized, use its inputs as the - // comparison and its condition as the branch condition. - Primitive::Type type = (condition != nullptr) - ? cond->InputAt(0)->GetType() - : Primitive::kPrimInt; - // Is this a long or FP comparison that has been folded into the HCondition? - if (type == Primitive::kPrimLong || Primitive::IsFloatingPointType(type)) { - // Generate the comparison directly. - GenerateCompareTestAndBranch(instruction->AsIf(), condition, - true_target, false_target, always_true_target); - return; - } - LocationSummary* locations = cond->GetLocations(); - DCHECK(locations->InAt(0).IsRegister()) << locations->InAt(0); - Register left = locations->InAt(0).AsRegister<Register>(); - Location right = locations->InAt(1); - if (right.IsRegister()) { - __ cmp(left, ShifterOperand(right.AsRegister<Register>())); - } else { - DCHECK(right.IsConstant()); - GenerateCompareWithImmediate(left, CodeGenerator::GetInt32ValueOf(right.GetConstant())); - } - if (can_jump_to_false) { - __ b(false_target, ARMCondition(condition->GetOppositeCondition())); - return; - } + // If this is a long or FP comparison that has been folded into + // the HCondition, generate the comparison directly. + Primitive::Type type = condition->InputAt(0)->GetType(); + if (type == Primitive::kPrimLong || Primitive::IsFloatingPointType(type)) { + GenerateCompareTestAndBranch(condition, true_target, false_target); + return; + } + LocationSummary* locations = cond->GetLocations(); + DCHECK(locations->InAt(0).IsRegister()); + Register left = locations->InAt(0).AsRegister<Register>(); + Location right = locations->InAt(1); + if (right.IsRegister()) { + __ cmp(left, ShifterOperand(right.AsRegister<Register>())); + } else { + DCHECK(right.IsConstant()); + GenerateCompareWithImmediate(left, CodeGenerator::GetInt32ValueOf(right.GetConstant())); + } + if (true_target == nullptr) { + __ b(false_target, ARMCondition(condition->GetOppositeCondition())); + } else { __ b(true_target, ARMCondition(condition->GetCondition())); } } - if (false_target != nullptr) { + + // If neither branch falls through (case 3), the conditional branch to `true_target` + // was already emitted (case 2) and we need to emit a jump to `false_target`. + if (true_target != nullptr && false_target != nullptr) { __ b(false_target); } } void LocationsBuilderARM::VisitIf(HIf* if_instr) { - LocationSummary* locations = - new (GetGraph()->GetArena()) LocationSummary(if_instr, LocationSummary::kNoCall); - HInstruction* cond = if_instr->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(if_instr); + if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } void InstructionCodeGeneratorARM::VisitIf(HIf* if_instr) { - Label* true_target = codegen_->GetLabelOf(if_instr->IfTrueSuccessor()); - Label* false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - Label* always_true_target = true_target; - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfTrueSuccessor())) { - always_true_target = nullptr; - } - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfFalseSuccessor())) { - false_target = nullptr; - } - GenerateTestAndBranch(if_instr, true_target, false_target, always_true_target); + HBasicBlock* true_successor = if_instr->IfTrueSuccessor(); + HBasicBlock* false_successor = if_instr->IfFalseSuccessor(); + Label* true_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), true_successor) ? + nullptr : codegen_->GetLabelOf(true_successor); + Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ? + nullptr : codegen_->GetLabelOf(false_successor); + GenerateTestAndBranch(if_instr, /* condition_input_index */ 0, true_target, false_target); } void LocationsBuilderARM::VisitDeoptimize(HDeoptimize* deoptimize) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(deoptimize, LocationSummary::kCallOnSlowPath); - HInstruction* cond = deoptimize->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(deoptimize->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } void InstructionCodeGeneratorARM::VisitDeoptimize(HDeoptimize* deoptimize) { - SlowPathCode* slow_path = new (GetGraph()->GetArena()) - DeoptimizationSlowPathARM(deoptimize); + SlowPathCode* slow_path = new (GetGraph()->GetArena()) DeoptimizationSlowPathARM(deoptimize); codegen_->AddSlowPath(slow_path); - Label* slow_path_entry = slow_path->GetEntryLabel(); - GenerateTestAndBranch(deoptimize, slow_path_entry, nullptr, slow_path_entry); + GenerateTestAndBranch(deoptimize, + /* condition_input_index */ 0, + slow_path->GetEntryLabel(), + /* false_target */ nullptr); } void LocationsBuilderARM::VisitCondition(HCondition* cond) { diff --git a/compiler/optimizing/code_generator_arm.h b/compiler/optimizing/code_generator_arm.h index cef1095c5d..32bfe0f0be 100644 --- a/compiler/optimizing/code_generator_arm.h +++ b/compiler/optimizing/code_generator_arm.h @@ -228,15 +228,13 @@ class InstructionCodeGeneratorARM : public HGraphVisitor { void GenerateImplicitNullCheck(HNullCheck* instruction); void GenerateExplicitNullCheck(HNullCheck* instruction); void GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target); + Label* false_target); void GenerateCompareWithImmediate(Register left, int32_t right); - void GenerateCompareTestAndBranch(HIf* if_instr, - HCondition* condition, + void GenerateCompareTestAndBranch(HCondition* condition, Label* true_target, - Label* false_target, - Label* always_true_target); + Label* false_target); void GenerateFPJumps(HCondition* cond, Label* true_label, Label* false_label); void GenerateLongComparesAndJumps(HCondition* cond, Label* true_label, Label* false_label); void DivRemOneOrMinusOne(HBinaryOperation* instruction); diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 7e248b402a..7a6125483c 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -2283,38 +2283,56 @@ void InstructionCodeGeneratorARM64::VisitTryBoundary(HTryBoundary* try_boundary) } void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, vixl::Label* true_target, - vixl::Label* false_target, - vixl::Label* always_true_target) { - HInstruction* cond = instruction->InputAt(0); - HCondition* condition = cond->AsCondition(); - - if (cond->IsIntConstant()) { - int32_t cond_value = cond->AsIntConstant()->GetValue(); - if (cond_value == 1) { - if (always_true_target != nullptr) { - __ B(always_true_target); + vixl::Label* false_target) { + // FP branching requires both targets to be explicit. If either of the targets + // is nullptr (fallthrough) use and bind `fallthrough_target` instead. + vixl::Label fallthrough_target; + HInstruction* cond = instruction->InputAt(condition_input_index); + + if (true_target == nullptr && false_target == nullptr) { + // Nothing to do. The code always falls through. + return; + } else if (cond->IsIntConstant()) { + // Constant condition, statically compared against 1. + if (cond->AsIntConstant()->IsOne()) { + if (true_target != nullptr) { + __ B(true_target); } - return; } else { - DCHECK_EQ(cond_value, 0); + DCHECK(cond->AsIntConstant()->IsZero()); + if (false_target != nullptr) { + __ B(false_target); + } } - } else if (!cond->IsCondition() || condition->NeedsMaterialization()) { + return; + } + + // The following code generates these patterns: + // (1) true_target == nullptr && false_target != nullptr + // - opposite condition true => branch to false_target + // (2) true_target != nullptr && false_target == nullptr + // - condition true => branch to true_target + // (3) true_target != nullptr && false_target != nullptr + // - condition true => branch to true_target + // - branch to false_target + if (IsBooleanValueOrMaterializedCondition(cond)) { // The condition instruction has been materialized, compare the output to 0. - Location cond_val = instruction->GetLocations()->InAt(0); + Location cond_val = instruction->GetLocations()->InAt(condition_input_index); DCHECK(cond_val.IsRegister()); - __ Cbnz(InputRegisterAt(instruction, 0), true_target); + if (true_target == nullptr) { + __ Cbz(InputRegisterAt(instruction, condition_input_index), false_target); + } else { + __ Cbnz(InputRegisterAt(instruction, condition_input_index), true_target); + } } 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; + HCondition* condition = cond->AsCondition(); + Primitive::Type type = condition->InputAt(0)->GetType(); 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())); @@ -2324,31 +2342,45 @@ void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruct __ Fcmp(lhs, InputFPRegisterAt(condition, 1)); } if (condition->IsFPConditionTrueIfNaN()) { - __ B(vs, true_target); // VS for unordered. + __ B(vs, true_target == nullptr ? &fallthrough_target : true_target); } else if (condition->IsFPConditionFalseIfNaN()) { - __ B(vs, false_target); // VS for unordered. + __ B(vs, false_target == nullptr ? &fallthrough_target : false_target); + } + if (true_target == nullptr) { + __ B(ARM64Condition(condition->GetOppositeCondition()), false_target); + } else { + __ B(ARM64Condition(condition->GetCondition()), true_target); } - __ 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()); + + Condition arm64_cond; + vixl::Label* non_fallthrough_target; + if (true_target == nullptr) { + arm64_cond = ARM64Condition(condition->GetOppositeCondition()); + non_fallthrough_target = false_target; + } else { + arm64_cond = ARM64Condition(condition->GetCondition()); + non_fallthrough_target = true_target; + } + if ((arm64_cond != gt && arm64_cond != le) && rhs.IsImmediate() && (rhs.immediate() == 0)) { switch (arm64_cond) { case eq: - __ Cbz(lhs, true_target); + __ Cbz(lhs, non_fallthrough_target); break; case ne: - __ Cbnz(lhs, true_target); + __ Cbnz(lhs, non_fallthrough_target); break; case lt: // Test the sign bit and branch accordingly. - __ Tbnz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, true_target); + __ Tbnz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, non_fallthrough_target); break; case ge: // Test the sign bit and branch accordingly. - __ Tbz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, true_target); + __ Tbz(lhs, (lhs.IsX() ? kXRegSize : kWRegSize) - 1, non_fallthrough_target); break; default: // Without the `static_cast` the compiler throws an error for @@ -2357,43 +2389,43 @@ void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruct } } else { __ Cmp(lhs, rhs); - __ B(arm64_cond, true_target); + __ B(arm64_cond, non_fallthrough_target); } } } - if (false_target != nullptr) { + + // If neither branch falls through (case 3), the conditional branch to `true_target` + // was already emitted (case 2) and we need to emit a jump to `false_target`. + if (true_target != nullptr && false_target != nullptr) { __ B(false_target); } + + if (fallthrough_target.IsLinked()) { + __ Bind(&fallthrough_target); + } } void LocationsBuilderARM64::VisitIf(HIf* if_instr) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(if_instr); - HInstruction* cond = if_instr->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } void InstructionCodeGeneratorARM64::VisitIf(HIf* if_instr) { - vixl::Label* true_target = codegen_->GetLabelOf(if_instr->IfTrueSuccessor()); - vixl::Label* false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - vixl::Label* always_true_target = true_target; - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfTrueSuccessor())) { - always_true_target = nullptr; - } - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfFalseSuccessor())) { - false_target = nullptr; - } - GenerateTestAndBranch(if_instr, true_target, false_target, always_true_target); + HBasicBlock* true_successor = if_instr->IfTrueSuccessor(); + HBasicBlock* false_successor = if_instr->IfFalseSuccessor(); + vixl::Label* true_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), true_successor) ? + nullptr : codegen_->GetLabelOf(true_successor); + vixl::Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ? + nullptr : codegen_->GetLabelOf(false_successor); + GenerateTestAndBranch(if_instr, /* condition_input_index */ 0, true_target, false_target); } void LocationsBuilderARM64::VisitDeoptimize(HDeoptimize* deoptimize) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(deoptimize, LocationSummary::kCallOnSlowPath); - HInstruction* cond = deoptimize->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(deoptimize->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } @@ -2402,8 +2434,10 @@ void InstructionCodeGeneratorARM64::VisitDeoptimize(HDeoptimize* deoptimize) { SlowPathCodeARM64* slow_path = new (GetGraph()->GetArena()) DeoptimizationSlowPathARM64(deoptimize); codegen_->AddSlowPath(slow_path); - vixl::Label* slow_path_entry = slow_path->GetEntryLabel(); - GenerateTestAndBranch(deoptimize, slow_path_entry, nullptr, slow_path_entry); + GenerateTestAndBranch(deoptimize, + /* condition_input_index */ 0, + slow_path->GetEntryLabel(), + /* false_target */ nullptr); } void LocationsBuilderARM64::VisitInstanceFieldGet(HInstanceFieldGet* instruction) { diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h index aa5ad386e1..d127ff6aee 100644 --- a/compiler/optimizing/code_generator_arm64.h +++ b/compiler/optimizing/code_generator_arm64.h @@ -203,9 +203,9 @@ class InstructionCodeGeneratorARM64 : public HGraphVisitor { void GenerateImplicitNullCheck(HNullCheck* instruction); void GenerateExplicitNullCheck(HNullCheck* instruction); void GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, vixl::Label* true_target, - vixl::Label* false_target, - vixl::Label* always_true_target); + vixl::Label* false_target); void DivRemOneOrMinusOne(HBinaryOperation* instruction); void DivRemByPowerOfTwo(HBinaryOperation* instruction); void GenerateDivRemWithAnyConstant(HBinaryOperation* instruction); diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index 959adb4238..919ed2db78 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -2420,30 +2420,51 @@ void InstructionCodeGeneratorMIPS::VisitTryBoundary(HTryBoundary* try_boundary) } void InstructionCodeGeneratorMIPS::GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, MipsLabel* true_target, - MipsLabel* false_target, - MipsLabel* always_true_target) { - HInstruction* cond = instruction->InputAt(0); - HCondition* condition = cond->AsCondition(); - - if (cond->IsIntConstant()) { - int32_t cond_value = cond->AsIntConstant()->GetValue(); - if (cond_value == 1) { - if (always_true_target != nullptr) { - __ B(always_true_target); + MipsLabel* false_target) { + HInstruction* cond = instruction->InputAt(condition_input_index); + + if (true_target == nullptr && false_target == nullptr) { + // Nothing to do. The code always falls through. + return; + } else if (cond->IsIntConstant()) { + // Constant condition, statically compared against 1. + if (cond->AsIntConstant()->IsOne()) { + if (true_target != nullptr) { + __ B(true_target); } - return; } else { - DCHECK_EQ(cond_value, 0); + DCHECK(cond->AsIntConstant()->IsZero()); + if (false_target != nullptr) { + __ B(false_target); + } } - } else if (!cond->IsCondition() || condition->NeedsMaterialization()) { + return; + } + + // The following code generates these patterns: + // (1) true_target == nullptr && false_target != nullptr + // - opposite condition true => branch to false_target + // (2) true_target != nullptr && false_target == nullptr + // - condition true => branch to true_target + // (3) true_target != nullptr && false_target != nullptr + // - condition true => branch to true_target + // - branch to false_target + if (IsBooleanValueOrMaterializedCondition(cond)) { // The condition instruction has been materialized, compare the output to 0. - Location cond_val = instruction->GetLocations()->InAt(0); + Location cond_val = instruction->GetLocations()->InAt(condition_input_index); DCHECK(cond_val.IsRegister()); - __ Bnez(cond_val.AsRegister<Register>(), true_target); + if (true_target == nullptr) { + __ Beqz(cond_val.AsRegister<Register>(), false_target); + } else { + __ Bnez(cond_val.AsRegister<Register>(), true_target); + } } else { // The condition instruction has not been materialized, use its inputs as // the comparison and its condition as the branch condition. + HCondition* condition = cond->AsCondition(); + Register lhs = condition->GetLocations()->InAt(0).AsRegister<Register>(); Location rhs_location = condition->GetLocations()->InAt(1); Register rhs_reg = ZERO; @@ -2455,37 +2476,46 @@ void InstructionCodeGeneratorMIPS::GenerateTestAndBranch(HInstruction* instructi rhs_reg = rhs_location.AsRegister<Register>(); } - IfCondition if_cond = condition->GetCondition(); + IfCondition if_cond; + MipsLabel* non_fallthrough_target; + if (true_target == nullptr) { + if_cond = condition->GetOppositeCondition(); + non_fallthrough_target = false_target; + } else { + if_cond = condition->GetCondition(); + non_fallthrough_target = true_target; + } + if (use_imm && rhs_imm == 0) { switch (if_cond) { case kCondEQ: - __ Beqz(lhs, true_target); + __ Beqz(lhs, non_fallthrough_target); break; case kCondNE: - __ Bnez(lhs, true_target); + __ Bnez(lhs, non_fallthrough_target); break; case kCondLT: - __ Bltz(lhs, true_target); + __ Bltz(lhs, non_fallthrough_target); break; case kCondGE: - __ Bgez(lhs, true_target); + __ Bgez(lhs, non_fallthrough_target); break; case kCondLE: - __ Blez(lhs, true_target); + __ Blez(lhs, non_fallthrough_target); break; case kCondGT: - __ Bgtz(lhs, true_target); + __ Bgtz(lhs, non_fallthrough_target); break; case kCondB: break; // always false case kCondBE: - __ Beqz(lhs, true_target); // <= 0 if zero + __ Beqz(lhs, non_fallthrough_target); // <= 0 if zero break; case kCondA: - __ Bnez(lhs, true_target); // > 0 if non-zero + __ Bnez(lhs, non_fallthrough_target); // > 0 if non-zero break; case kCondAE: - __ B(true_target); // always true + __ B(non_fallthrough_target); // always true break; } } else { @@ -2496,81 +2526,78 @@ void InstructionCodeGeneratorMIPS::GenerateTestAndBranch(HInstruction* instructi } switch (if_cond) { case kCondEQ: - __ Beq(lhs, rhs_reg, true_target); + __ Beq(lhs, rhs_reg, non_fallthrough_target); break; case kCondNE: - __ Bne(lhs, rhs_reg, true_target); + __ Bne(lhs, rhs_reg, non_fallthrough_target); break; case kCondLT: - __ Blt(lhs, rhs_reg, true_target); + __ Blt(lhs, rhs_reg, non_fallthrough_target); break; case kCondGE: - __ Bge(lhs, rhs_reg, true_target); + __ Bge(lhs, rhs_reg, non_fallthrough_target); break; case kCondLE: - __ Bge(rhs_reg, lhs, true_target); + __ Bge(rhs_reg, lhs, non_fallthrough_target); break; case kCondGT: - __ Blt(rhs_reg, lhs, true_target); + __ Blt(rhs_reg, lhs, non_fallthrough_target); break; case kCondB: - __ Bltu(lhs, rhs_reg, true_target); + __ Bltu(lhs, rhs_reg, non_fallthrough_target); break; case kCondAE: - __ Bgeu(lhs, rhs_reg, true_target); + __ Bgeu(lhs, rhs_reg, non_fallthrough_target); break; case kCondBE: - __ Bgeu(rhs_reg, lhs, true_target); + __ Bgeu(rhs_reg, lhs, non_fallthrough_target); break; case kCondA: - __ Bltu(rhs_reg, lhs, true_target); + __ Bltu(rhs_reg, lhs, non_fallthrough_target); break; } } } - if (false_target != nullptr) { + + // If neither branch falls through (case 3), the conditional branch to `true_target` + // was already emitted (case 2) and we need to emit a jump to `false_target`. + if (true_target != nullptr && false_target != nullptr) { __ B(false_target); } } void LocationsBuilderMIPS::VisitIf(HIf* if_instr) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(if_instr); - HInstruction* cond = if_instr->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } void InstructionCodeGeneratorMIPS::VisitIf(HIf* if_instr) { - MipsLabel* true_target = codegen_->GetLabelOf(if_instr->IfTrueSuccessor()); - MipsLabel* false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - MipsLabel* always_true_target = true_target; - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfTrueSuccessor())) { - always_true_target = nullptr; - } - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfFalseSuccessor())) { - false_target = nullptr; - } - GenerateTestAndBranch(if_instr, true_target, false_target, always_true_target); + HBasicBlock* true_successor = if_instr->IfTrueSuccessor(); + HBasicBlock* false_successor = if_instr->IfFalseSuccessor(); + MipsLabel* true_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), true_successor) ? + nullptr : codegen_->GetLabelOf(true_successor); + MipsLabel* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ? + nullptr : codegen_->GetLabelOf(false_successor); + GenerateTestAndBranch(if_instr, /* condition_input_index */ 0, true_target, false_target); } void LocationsBuilderMIPS::VisitDeoptimize(HDeoptimize* deoptimize) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(deoptimize, LocationSummary::kCallOnSlowPath); - HInstruction* cond = deoptimize->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(deoptimize->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } void InstructionCodeGeneratorMIPS::VisitDeoptimize(HDeoptimize* deoptimize) { - SlowPathCodeMIPS* slow_path = new (GetGraph()->GetArena()) - DeoptimizationSlowPathMIPS(deoptimize); + SlowPathCodeMIPS* slow_path = new (GetGraph()->GetArena()) DeoptimizationSlowPathMIPS(deoptimize); codegen_->AddSlowPath(slow_path); - MipsLabel* slow_path_entry = slow_path->GetEntryLabel(); - GenerateTestAndBranch(deoptimize, slow_path_entry, nullptr, slow_path_entry); + GenerateTestAndBranch(deoptimize, + /* condition_input_index */ 0, + slow_path->GetEntryLabel(), + /* false_target */ nullptr); } void LocationsBuilderMIPS::HandleFieldGet(HInstruction* instruction, const FieldInfo& field_info) { diff --git a/compiler/optimizing/code_generator_mips.h b/compiler/optimizing/code_generator_mips.h index 059131dcfc..e3a2cb40ef 100644 --- a/compiler/optimizing/code_generator_mips.h +++ b/compiler/optimizing/code_generator_mips.h @@ -226,9 +226,9 @@ class InstructionCodeGeneratorMIPS : public HGraphVisitor { void GenerateImplicitNullCheck(HNullCheck* instruction); void GenerateExplicitNullCheck(HNullCheck* instruction); void GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, MipsLabel* true_target, - MipsLabel* false_target, - MipsLabel* always_true_target); + MipsLabel* false_target); void HandleGoto(HInstruction* got, HBasicBlock* successor); MipsAssembler* const assembler_; diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 851bced09a..5864660890 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -2340,30 +2340,51 @@ void InstructionCodeGeneratorMIPS64::VisitTryBoundary(HTryBoundary* try_boundary } void InstructionCodeGeneratorMIPS64::GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target) { - HInstruction* cond = instruction->InputAt(0); - HCondition* condition = cond->AsCondition(); - - if (cond->IsIntConstant()) { - int32_t cond_value = cond->AsIntConstant()->GetValue(); - if (cond_value == 1) { - if (always_true_target != nullptr) { - __ B(always_true_target); + Label* false_target) { + HInstruction* cond = instruction->InputAt(condition_input_index); + + if (true_target == nullptr && false_target == nullptr) { + // Nothing to do. The code always falls through. + return; + } else if (cond->IsIntConstant()) { + // Constant condition, statically compared against 1. + if (cond->AsIntConstant()->IsOne()) { + if (true_target != nullptr) { + __ B(true_target); } - return; } else { - DCHECK_EQ(cond_value, 0); + DCHECK(cond->AsIntConstant()->IsZero()); + if (false_target != nullptr) { + __ B(false_target); + } } - } else if (!cond->IsCondition() || condition->NeedsMaterialization()) { + return; + } + + // The following code generates these patterns: + // (1) true_target == nullptr && false_target != nullptr + // - opposite condition true => branch to false_target + // (2) true_target != nullptr && false_target == nullptr + // - condition true => branch to true_target + // (3) true_target != nullptr && false_target != nullptr + // - condition true => branch to true_target + // - branch to false_target + if (IsBooleanValueOrMaterializedCondition(cond)) { // The condition instruction has been materialized, compare the output to 0. - Location cond_val = instruction->GetLocations()->InAt(0); + Location cond_val = instruction->GetLocations()->InAt(condition_input_index); DCHECK(cond_val.IsRegister()); - __ Bnezc(cond_val.AsRegister<GpuRegister>(), true_target); + if (true_target == nullptr) { + __ Beqzc(cond_val.AsRegister<GpuRegister>(), false_target); + } else { + __ Bnezc(cond_val.AsRegister<GpuRegister>(), true_target); + } } else { // The condition instruction has not been materialized, use its inputs as // the comparison and its condition as the branch condition. + HCondition* condition = cond->AsCondition(); + GpuRegister lhs = condition->GetLocations()->InAt(0).AsRegister<GpuRegister>(); Location rhs_location = condition->GetLocations()->InAt(1); GpuRegister rhs_reg = ZERO; @@ -2375,37 +2396,46 @@ void InstructionCodeGeneratorMIPS64::GenerateTestAndBranch(HInstruction* instruc rhs_reg = rhs_location.AsRegister<GpuRegister>(); } - IfCondition if_cond = condition->GetCondition(); + IfCondition if_cond; + Label* non_fallthrough_target; + if (true_target == nullptr) { + if_cond = condition->GetOppositeCondition(); + non_fallthrough_target = false_target; + } else { + if_cond = condition->GetCondition(); + non_fallthrough_target = true_target; + } + if (use_imm && rhs_imm == 0) { switch (if_cond) { case kCondEQ: - __ Beqzc(lhs, true_target); + __ Beqzc(lhs, non_fallthrough_target); break; case kCondNE: - __ Bnezc(lhs, true_target); + __ Bnezc(lhs, non_fallthrough_target); break; case kCondLT: - __ Bltzc(lhs, true_target); + __ Bltzc(lhs, non_fallthrough_target); break; case kCondGE: - __ Bgezc(lhs, true_target); + __ Bgezc(lhs, non_fallthrough_target); break; case kCondLE: - __ Blezc(lhs, true_target); + __ Blezc(lhs, non_fallthrough_target); break; case kCondGT: - __ Bgtzc(lhs, true_target); + __ Bgtzc(lhs, non_fallthrough_target); break; case kCondB: break; // always false case kCondBE: - __ Beqzc(lhs, true_target); // <= 0 if zero + __ Beqzc(lhs, non_fallthrough_target); // <= 0 if zero break; case kCondA: - __ Bnezc(lhs, true_target); // > 0 if non-zero + __ Bnezc(lhs, non_fallthrough_target); // > 0 if non-zero break; case kCondAE: - __ B(true_target); // always true + __ B(non_fallthrough_target); // always true break; } } else { @@ -2424,7 +2454,7 @@ void InstructionCodeGeneratorMIPS64::GenerateTestAndBranch(HInstruction* instruc case kCondBE: case kCondAE: // if lhs == rhs for a positive condition, then it is a branch - __ B(true_target); + __ B(non_fallthrough_target); break; case kCondNE: case kCondLT: @@ -2437,72 +2467,68 @@ void InstructionCodeGeneratorMIPS64::GenerateTestAndBranch(HInstruction* instruc } else { switch (if_cond) { case kCondEQ: - __ Beqc(lhs, rhs_reg, true_target); + __ Beqc(lhs, rhs_reg, non_fallthrough_target); break; case kCondNE: - __ Bnec(lhs, rhs_reg, true_target); + __ Bnec(lhs, rhs_reg, non_fallthrough_target); break; case kCondLT: - __ Bltc(lhs, rhs_reg, true_target); + __ Bltc(lhs, rhs_reg, non_fallthrough_target); break; case kCondGE: - __ Bgec(lhs, rhs_reg, true_target); + __ Bgec(lhs, rhs_reg, non_fallthrough_target); break; case kCondLE: - __ Bgec(rhs_reg, lhs, true_target); + __ Bgec(rhs_reg, lhs, non_fallthrough_target); break; case kCondGT: - __ Bltc(rhs_reg, lhs, true_target); + __ Bltc(rhs_reg, lhs, non_fallthrough_target); break; case kCondB: - __ Bltuc(lhs, rhs_reg, true_target); + __ Bltuc(lhs, rhs_reg, non_fallthrough_target); break; case kCondAE: - __ Bgeuc(lhs, rhs_reg, true_target); + __ Bgeuc(lhs, rhs_reg, non_fallthrough_target); break; case kCondBE: - __ Bgeuc(rhs_reg, lhs, true_target); + __ Bgeuc(rhs_reg, lhs, non_fallthrough_target); break; case kCondA: - __ Bltuc(rhs_reg, lhs, true_target); + __ Bltuc(rhs_reg, lhs, non_fallthrough_target); break; } } } } - if (false_target != nullptr) { + + // If neither branch falls through (case 3), the conditional branch to `true_target` + // was already emitted (case 2) and we need to emit a jump to `false_target`. + if (true_target != nullptr && false_target != nullptr) { __ B(false_target); } } void LocationsBuilderMIPS64::VisitIf(HIf* if_instr) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(if_instr); - HInstruction* cond = if_instr->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } void InstructionCodeGeneratorMIPS64::VisitIf(HIf* if_instr) { - Label* true_target = codegen_->GetLabelOf(if_instr->IfTrueSuccessor()); - Label* false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - Label* always_true_target = true_target; - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfTrueSuccessor())) { - always_true_target = nullptr; - } - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfFalseSuccessor())) { - false_target = nullptr; - } - GenerateTestAndBranch(if_instr, true_target, false_target, always_true_target); + HBasicBlock* true_successor = if_instr->IfTrueSuccessor(); + HBasicBlock* false_successor = if_instr->IfFalseSuccessor(); + Label* true_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), true_successor) ? + nullptr : codegen_->GetLabelOf(true_successor); + Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ? + nullptr : codegen_->GetLabelOf(false_successor); + GenerateTestAndBranch(if_instr, /* condition_input_index */ 0, true_target, false_target); } void LocationsBuilderMIPS64::VisitDeoptimize(HDeoptimize* deoptimize) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(deoptimize, LocationSummary::kCallOnSlowPath); - HInstruction* cond = deoptimize->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(deoptimize->InputAt(0))) { locations->SetInAt(0, Location::RequiresRegister()); } } @@ -2511,8 +2537,10 @@ void InstructionCodeGeneratorMIPS64::VisitDeoptimize(HDeoptimize* deoptimize) { SlowPathCodeMIPS64* slow_path = new (GetGraph()->GetArena()) DeoptimizationSlowPathMIPS64(deoptimize); codegen_->AddSlowPath(slow_path); - Label* slow_path_entry = slow_path->GetEntryLabel(); - GenerateTestAndBranch(deoptimize, slow_path_entry, nullptr, slow_path_entry); + GenerateTestAndBranch(deoptimize, + /* condition_input_index */ 0, + slow_path->GetEntryLabel(), + /* false_target */ nullptr); } void LocationsBuilderMIPS64::HandleFieldGet(HInstruction* instruction, diff --git a/compiler/optimizing/code_generator_mips64.h b/compiler/optimizing/code_generator_mips64.h index ac3162f736..a078dd1819 100644 --- a/compiler/optimizing/code_generator_mips64.h +++ b/compiler/optimizing/code_generator_mips64.h @@ -230,9 +230,9 @@ class InstructionCodeGeneratorMIPS64 : public HGraphVisitor { void GenerateImplicitNullCheck(HNullCheck* instruction); void GenerateExplicitNullCheck(HNullCheck* instruction); void GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target); + Label* false_target); void DivRemOneOrMinusOne(HBinaryOperation* instruction); void DivRemByPowerOfTwo(HBinaryOperation* instruction); void GenerateDivRemWithAnyConstant(HBinaryOperation* instruction); diff --git a/compiler/optimizing/code_generator_utils.cc b/compiler/optimizing/code_generator_utils.cc index bf354e7ee2..644a3fb75e 100644 --- a/compiler/optimizing/code_generator_utils.cc +++ b/compiler/optimizing/code_generator_utils.cc @@ -95,19 +95,8 @@ void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long, *shift = is_long ? p - 64 : p - 32; } -// Is it valid to reverse the condition? Uses the values supplied to -// GenerateTestAndBranch() in instruction generators. -bool CanReverseCondition(Label* always_true_target, - Label* false_target, - HCondition* condition) { - // 'always_true_target' is null when the 'true' path is to the next - // block to be generated. Check the type of the condition to ensure that - // FP conditions are not swapped. This is for future fusing of HCompare and - // HCondition. - // Note: If the condition is nullptr, then it is always okay to reverse. - return always_true_target == nullptr && false_target != nullptr && - (condition == nullptr || - !Primitive::IsFloatingPointType(condition->InputAt(0)->GetType())); +bool IsBooleanValueOrMaterializedCondition(HInstruction* cond_input) { + return !cond_input->IsCondition() || cond_input->AsCondition()->NeedsMaterialization(); } } // namespace art diff --git a/compiler/optimizing/code_generator_utils.h b/compiler/optimizing/code_generator_utils.h index 628eee8885..7efed8c9ec 100644 --- a/compiler/optimizing/code_generator_utils.h +++ b/compiler/optimizing/code_generator_utils.h @@ -21,18 +21,16 @@ namespace art { -class Label; -class HCondition; +class HInstruction; // Computes the magic number and the shift needed in the div/rem by constant algorithm, as out // arguments `magic` and `shift` void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long, int64_t* magic, int* shift); -// Is it valid to reverse the condition? Uses the values supplied to -// GenerateTestAndBranch() in instruction generators. -bool CanReverseCondition(Label* always_true_target, - Label* false_target, - HCondition* condition); +// Returns true if `cond_input` is expected to have a location. Assumes that +// `cond_input` is a conditional input of the currently emitted instruction and +// that it has been previously visited by the InstructionCodeGenerator. +bool IsBooleanValueOrMaterializedCondition(HInstruction* cond_input); } // namespace art diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 32dc636d1d..999306c34b 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -1370,26 +1370,19 @@ void InstructionCodeGeneratorX86::GenerateLongComparesAndJumps(HCondition* cond, __ j(final_condition, true_label); } -void InstructionCodeGeneratorX86::GenerateCompareTestAndBranch(HIf* if_instr, - HCondition* condition, - Label* true_target, - Label* false_target, - Label* always_true_target) { +void InstructionCodeGeneratorX86::GenerateCompareTestAndBranch(HCondition* condition, + Label* true_target_in, + Label* false_target_in) { + // Generated branching requires both targets to be explicit. If either of the + // targets is nullptr (fallthrough) use and bind `fallthrough_target` instead. + Label fallthrough_target; + Label* true_target = true_target_in == nullptr ? &fallthrough_target : true_target_in; + Label* false_target = false_target_in == nullptr ? &fallthrough_target : false_target_in; + LocationSummary* locations = condition->GetLocations(); Location left = locations->InAt(0); Location right = locations->InAt(1); - // We don't want true_target as a nullptr. - if (true_target == nullptr) { - true_target = always_true_target; - } - bool falls_through = (false_target == nullptr); - - // FP compares don't like null false_targets. - if (false_target == nullptr) { - false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - } - Primitive::Type type = condition->InputAt(0)->GetType(); switch (type) { case Primitive::kPrimLong: @@ -1407,138 +1400,141 @@ void InstructionCodeGeneratorX86::GenerateCompareTestAndBranch(HIf* if_instr, LOG(FATAL) << "Unexpected compare type " << type; } - if (!falls_through) { + if (false_target != &fallthrough_target) { __ jmp(false_target); } + + if (fallthrough_target.IsLinked()) { + __ Bind(&fallthrough_target); + } +} + +static bool AreEflagsSetFrom(HInstruction* cond, HInstruction* branch) { + // Moves may affect the eflags register (move zero uses xorl), so the EFLAGS + // are set only strictly before `branch`. We can't use the eflags on long/FP + // conditions if they are materialized due to the complex branching. + return cond->IsCondition() && + cond->GetNext() == branch && + cond->InputAt(0)->GetType() != Primitive::kPrimLong && + !Primitive::IsFloatingPointType(cond->InputAt(0)->GetType()); } void InstructionCodeGeneratorX86::GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target) { - HInstruction* cond = instruction->InputAt(0); - if (cond->IsIntConstant()) { + Label* false_target) { + HInstruction* cond = instruction->InputAt(condition_input_index); + + if (true_target == nullptr && false_target == nullptr) { + // Nothing to do. The code always falls through. + return; + } else if (cond->IsIntConstant()) { // Constant condition, statically compared against 1. - int32_t cond_value = cond->AsIntConstant()->GetValue(); - if (cond_value == 1) { - if (always_true_target != nullptr) { - __ jmp(always_true_target); + if (cond->AsIntConstant()->IsOne()) { + if (true_target != nullptr) { + __ jmp(true_target); } - return; } else { - DCHECK_EQ(cond_value, 0); + DCHECK(cond->AsIntConstant()->IsZero()); + if (false_target != nullptr) { + __ jmp(false_target); + } } - } else { - HCondition* condition = cond->AsCondition(); - bool is_materialized = - condition == nullptr || condition->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. We can't use the eflags on long/FP conditions if they are - // materialized due to the complex branching. - Primitive::Type type = (condition != nullptr) - ? cond->InputAt(0)->GetType() - : Primitive::kPrimInt; - bool eflags_set = condition != nullptr - && condition->IsBeforeWhenDisregardMoves(instruction) - && (type != Primitive::kPrimLong && !Primitive::IsFloatingPointType(type)); - // Can we optimize the jump if we know that the next block is the true case? - bool can_jump_to_false = CanReverseCondition(always_true_target, false_target, condition); - if (is_materialized) { - if (!eflags_set) { - // Materialized condition, compare against 0. - Location lhs = instruction->GetLocations()->InAt(0); - if (lhs.IsRegister()) { - __ testl(lhs.AsRegister<Register>(), lhs.AsRegister<Register>()); - } else { - __ cmpl(Address(ESP, lhs.GetStackIndex()), Immediate(0)); - } - if (can_jump_to_false) { - __ j(kEqual, false_target); - return; - } - __ j(kNotEqual, true_target); + return; + } + + // The following code generates these patterns: + // (1) true_target == nullptr && false_target != nullptr + // - opposite condition true => branch to false_target + // (2) true_target != nullptr && false_target == nullptr + // - condition true => branch to true_target + // (3) true_target != nullptr && false_target != nullptr + // - condition true => branch to true_target + // - branch to false_target + if (IsBooleanValueOrMaterializedCondition(cond)) { + if (AreEflagsSetFrom(cond, instruction)) { + if (true_target == nullptr) { + __ j(X86Condition(cond->AsCondition()->GetOppositeCondition()), false_target); } else { - if (can_jump_to_false) { - __ j(X86Condition(condition->GetOppositeCondition()), false_target); - return; - } - __ j(X86Condition(condition->GetCondition()), true_target); + __ j(X86Condition(cond->AsCondition()->GetCondition()), true_target); } } else { - // Condition has not been materialized, use its inputs as the - // comparison and its condition as the branch condition. - - // Is this a long or FP comparison that has been folded into the HCondition? - if (type == Primitive::kPrimLong || Primitive::IsFloatingPointType(type)) { - // Generate the comparison directly. - GenerateCompareTestAndBranch(instruction->AsIf(), - condition, - true_target, - false_target, - always_true_target); - return; + // Materialized condition, compare against 0. + Location lhs = instruction->GetLocations()->InAt(condition_input_index); + if (lhs.IsRegister()) { + __ testl(lhs.AsRegister<Register>(), lhs.AsRegister<Register>()); + } else { + __ cmpl(Address(ESP, lhs.GetStackIndex()), Immediate(0)); } - - Location lhs = cond->GetLocations()->InAt(0); - Location rhs = cond->GetLocations()->InAt(1); - // LHS is guaranteed to be in a register (see - // LocationsBuilderX86::VisitCondition). - if (rhs.IsRegister()) { - __ cmpl(lhs.AsRegister<Register>(), rhs.AsRegister<Register>()); - } else if (rhs.IsConstant()) { - int32_t constant = CodeGenerator::GetInt32ValueOf(rhs.GetConstant()); - if (constant == 0) { - __ testl(lhs.AsRegister<Register>(), lhs.AsRegister<Register>()); - } else { - __ cmpl(lhs.AsRegister<Register>(), Immediate(constant)); - } + if (true_target == nullptr) { + __ j(kEqual, false_target); } else { - __ cmpl(lhs.AsRegister<Register>(), Address(ESP, rhs.GetStackIndex())); + __ j(kNotEqual, true_target); } + } + } else { + // Condition has not been materialized, use its inputs as the comparison and + // its condition as the branch condition. + HCondition* condition = cond->AsCondition(); - if (can_jump_to_false) { - __ j(X86Condition(condition->GetOppositeCondition()), false_target); - return; - } + // If this is a long or FP comparison that has been folded into + // the HCondition, generate the comparison directly. + Primitive::Type type = condition->InputAt(0)->GetType(); + if (type == Primitive::kPrimLong || Primitive::IsFloatingPointType(type)) { + GenerateCompareTestAndBranch(condition, true_target, false_target); + return; + } + Location lhs = condition->GetLocations()->InAt(0); + Location rhs = condition->GetLocations()->InAt(1); + // LHS is guaranteed to be in a register (see LocationsBuilderX86::VisitCondition). + if (rhs.IsRegister()) { + __ cmpl(lhs.AsRegister<Register>(), rhs.AsRegister<Register>()); + } else if (rhs.IsConstant()) { + int32_t constant = CodeGenerator::GetInt32ValueOf(rhs.GetConstant()); + if (constant == 0) { + __ testl(lhs.AsRegister<Register>(), lhs.AsRegister<Register>()); + } else { + __ cmpl(lhs.AsRegister<Register>(), Immediate(constant)); + } + } else { + __ cmpl(lhs.AsRegister<Register>(), Address(ESP, rhs.GetStackIndex())); + } + if (true_target == nullptr) { + __ j(X86Condition(condition->GetOppositeCondition()), false_target); + } else { __ j(X86Condition(condition->GetCondition()), true_target); } } - if (false_target != nullptr) { + + // If neither branch falls through (case 3), the conditional branch to `true_target` + // was already emitted (case 2) and we need to emit a jump to `false_target`. + if (true_target != nullptr && false_target != nullptr) { __ jmp(false_target); } } void LocationsBuilderX86::VisitIf(HIf* if_instr) { - LocationSummary* locations = - new (GetGraph()->GetArena()) LocationSummary(if_instr, LocationSummary::kNoCall); - HInstruction* cond = if_instr->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(if_instr); + if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) { locations->SetInAt(0, Location::Any()); } } void InstructionCodeGeneratorX86::VisitIf(HIf* if_instr) { - Label* true_target = codegen_->GetLabelOf(if_instr->IfTrueSuccessor()); - Label* false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - Label* always_true_target = true_target; - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfTrueSuccessor())) { - always_true_target = nullptr; - } - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfFalseSuccessor())) { - false_target = nullptr; - } - GenerateTestAndBranch(if_instr, true_target, false_target, always_true_target); + HBasicBlock* true_successor = if_instr->IfTrueSuccessor(); + HBasicBlock* false_successor = if_instr->IfFalseSuccessor(); + Label* true_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), true_successor) ? + nullptr : codegen_->GetLabelOf(true_successor); + Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ? + nullptr : codegen_->GetLabelOf(false_successor); + GenerateTestAndBranch(if_instr, /* condition_input_index */ 0, true_target, false_target); } void LocationsBuilderX86::VisitDeoptimize(HDeoptimize* deoptimize) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(deoptimize, LocationSummary::kCallOnSlowPath); - HInstruction* cond = deoptimize->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(deoptimize->InputAt(0))) { locations->SetInAt(0, Location::Any()); } } @@ -1547,8 +1543,10 @@ void InstructionCodeGeneratorX86::VisitDeoptimize(HDeoptimize* deoptimize) { SlowPathCode* slow_path = new (GetGraph()->GetArena()) DeoptimizationSlowPathX86(deoptimize); codegen_->AddSlowPath(slow_path); - Label* slow_path_entry = slow_path->GetEntryLabel(); - GenerateTestAndBranch(deoptimize, slow_path_entry, nullptr, slow_path_entry); + GenerateTestAndBranch(deoptimize, + /* condition_input_index */ 0, + slow_path->GetEntryLabel(), + /* false_target */ nullptr); } void LocationsBuilderX86::VisitLocal(HLocal* local) { diff --git a/compiler/optimizing/code_generator_x86.h b/compiler/optimizing/code_generator_x86.h index cd606f697e..064051c7f4 100644 --- a/compiler/optimizing/code_generator_x86.h +++ b/compiler/optimizing/code_generator_x86.h @@ -227,14 +227,12 @@ class InstructionCodeGeneratorX86 : public HGraphVisitor { void GenerateImplicitNullCheck(HNullCheck* instruction); void GenerateExplicitNullCheck(HNullCheck* instruction); void GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target); - void GenerateCompareTestAndBranch(HIf* if_inst, - HCondition* condition, + Label* false_target); + void GenerateCompareTestAndBranch(HCondition* condition, Label* true_target, - Label* false_target, - Label* always_true_target); + Label* false_target); void GenerateFPJumps(HCondition* cond, Label* true_label, Label* false_label); void GenerateLongComparesAndJumps(HCondition* cond, Label* true_label, Label* false_label); void HandleGoto(HInstruction* got, HBasicBlock* successor); diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index d55c084618..4088160b3f 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -1304,26 +1304,19 @@ void InstructionCodeGeneratorX86_64::GenerateFPJumps(HCondition* cond, __ j(X86_64FPCondition(cond->GetCondition()), true_label); } -void InstructionCodeGeneratorX86_64::GenerateCompareTestAndBranch(HIf* if_instr, - HCondition* condition, - Label* true_target, - Label* false_target, - Label* always_true_target) { +void InstructionCodeGeneratorX86_64::GenerateCompareTestAndBranch(HCondition* condition, + Label* true_target_in, + Label* false_target_in) { + // Generated branching requires both targets to be explicit. If either of the + // targets is nullptr (fallthrough) use and bind `fallthrough_target` instead. + Label fallthrough_target; + Label* true_target = true_target_in == nullptr ? &fallthrough_target : true_target_in; + Label* false_target = false_target_in == nullptr ? &fallthrough_target : false_target_in; + LocationSummary* locations = condition->GetLocations(); Location left = locations->InAt(0); Location right = locations->InAt(1); - // We don't want true_target as a nullptr. - if (true_target == nullptr) { - true_target = always_true_target; - } - bool falls_through = (false_target == nullptr); - - // FP compares don't like null false_targets. - if (false_target == nullptr) { - false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - } - Primitive::Type type = condition->InputAt(0)->GetType(); switch (type) { case Primitive::kPrimLong: { @@ -1382,135 +1375,140 @@ void InstructionCodeGeneratorX86_64::GenerateCompareTestAndBranch(HIf* if_instr, LOG(FATAL) << "Unexpected condition type " << type; } - if (!falls_through) { + if (false_target != &fallthrough_target) { __ jmp(false_target); } + + if (fallthrough_target.IsLinked()) { + __ Bind(&fallthrough_target); + } +} + +static bool AreEflagsSetFrom(HInstruction* cond, HInstruction* branch) { + // Moves may affect the eflags register (move zero uses xorl), so the EFLAGS + // are set only strictly before `branch`. We can't use the eflags on long + // conditions if they are materialized due to the complex branching. + return cond->IsCondition() && + cond->GetNext() == branch && + !Primitive::IsFloatingPointType(cond->InputAt(0)->GetType()); } void InstructionCodeGeneratorX86_64::GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target) { - HInstruction* cond = instruction->InputAt(0); - if (cond->IsIntConstant()) { + Label* false_target) { + HInstruction* cond = instruction->InputAt(condition_input_index); + + if (true_target == nullptr && false_target == nullptr) { + // Nothing to do. The code always falls through. + return; + } else if (cond->IsIntConstant()) { // Constant condition, statically compared against 1. - int32_t cond_value = cond->AsIntConstant()->GetValue(); - if (cond_value == 1) { - if (always_true_target != nullptr) { - __ jmp(always_true_target); + if (cond->AsIntConstant()->IsOne()) { + if (true_target != nullptr) { + __ jmp(true_target); } - return; } else { - DCHECK_EQ(cond_value, 0); + DCHECK(cond->AsIntConstant()->IsZero()); + if (false_target != nullptr) { + __ jmp(false_target); + } } - } else { - HCondition* condition = cond->AsCondition(); - bool is_materialized = condition == nullptr || condition->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. We can't use the eflags on FP conditions if they are - // materialized due to the complex branching. - Primitive::Type type = (condition != nullptr) - ? cond->InputAt(0)->GetType() - : Primitive::kPrimInt; - bool eflags_set = condition != nullptr - && condition->IsBeforeWhenDisregardMoves(instruction) - && !Primitive::IsFloatingPointType(type); - // Can we optimize the jump if we know that the next block is the true case? - bool can_jump_to_false = CanReverseCondition(always_true_target, false_target, condition); - - if (is_materialized) { - if (!eflags_set) { - // Materialized condition, compare against 0. - Location lhs = instruction->GetLocations()->InAt(0); - if (lhs.IsRegister()) { - __ testl(lhs.AsRegister<CpuRegister>(), lhs.AsRegister<CpuRegister>()); - } else { - __ cmpl(Address(CpuRegister(RSP), lhs.GetStackIndex()), - Immediate(0)); - } - if (can_jump_to_false) { - __ j(kEqual, false_target); - return; - } - __ j(kNotEqual, true_target); + return; + } + + // The following code generates these patterns: + // (1) true_target == nullptr && false_target != nullptr + // - opposite condition true => branch to false_target + // (2) true_target != nullptr && false_target == nullptr + // - condition true => branch to true_target + // (3) true_target != nullptr && false_target != nullptr + // - condition true => branch to true_target + // - branch to false_target + if (IsBooleanValueOrMaterializedCondition(cond)) { + if (AreEflagsSetFrom(cond, instruction)) { + if (true_target == nullptr) { + __ j(X86_64IntegerCondition(cond->AsCondition()->GetOppositeCondition()), false_target); } else { - if (can_jump_to_false) { - __ j(X86_64IntegerCondition(condition->GetOppositeCondition()), false_target); - return; - } - __ j(X86_64IntegerCondition(condition->GetCondition()), true_target); + __ j(X86_64IntegerCondition(cond->AsCondition()->GetCondition()), true_target); } } else { - // Condition has not been materialized, use its inputs as the - // comparison and its condition as the branch condition. - - // Is this a long or FP comparison that has been folded into the HCondition? - if (type == Primitive::kPrimLong || Primitive::IsFloatingPointType(type)) { - // Generate the comparison directly. - GenerateCompareTestAndBranch(instruction->AsIf(), condition, - true_target, false_target, always_true_target); - return; + // Materialized condition, compare against 0. + Location lhs = instruction->GetLocations()->InAt(condition_input_index); + if (lhs.IsRegister()) { + __ testl(lhs.AsRegister<CpuRegister>(), lhs.AsRegister<CpuRegister>()); + } else { + __ cmpl(Address(CpuRegister(RSP), lhs.GetStackIndex()), Immediate(0)); } - - Location lhs = cond->GetLocations()->InAt(0); - Location rhs = cond->GetLocations()->InAt(1); - if (rhs.IsRegister()) { - __ cmpl(lhs.AsRegister<CpuRegister>(), rhs.AsRegister<CpuRegister>()); - } else if (rhs.IsConstant()) { - int32_t constant = CodeGenerator::GetInt32ValueOf(rhs.GetConstant()); - if (constant == 0) { - __ testl(lhs.AsRegister<CpuRegister>(), lhs.AsRegister<CpuRegister>()); - } else { - __ cmpl(lhs.AsRegister<CpuRegister>(), Immediate(constant)); - } + if (true_target == nullptr) { + __ j(kEqual, false_target); } else { - __ cmpl(lhs.AsRegister<CpuRegister>(), - Address(CpuRegister(RSP), rhs.GetStackIndex())); + __ j(kNotEqual, true_target); } + } + } else { + // Condition has not been materialized, use its inputs as the + // comparison and its condition as the branch condition. + HCondition* condition = cond->AsCondition(); - if (can_jump_to_false) { - __ j(X86_64IntegerCondition(condition->GetOppositeCondition()), false_target); - return; - } + // If this is a long or FP comparison that has been folded into + // the HCondition, generate the comparison directly. + Primitive::Type type = condition->InputAt(0)->GetType(); + if (type == Primitive::kPrimLong || Primitive::IsFloatingPointType(type)) { + GenerateCompareTestAndBranch(condition, true_target, false_target); + return; + } + Location lhs = condition->GetLocations()->InAt(0); + Location rhs = condition->GetLocations()->InAt(1); + if (rhs.IsRegister()) { + __ cmpl(lhs.AsRegister<CpuRegister>(), rhs.AsRegister<CpuRegister>()); + } else if (rhs.IsConstant()) { + int32_t constant = CodeGenerator::GetInt32ValueOf(rhs.GetConstant()); + if (constant == 0) { + __ testl(lhs.AsRegister<CpuRegister>(), lhs.AsRegister<CpuRegister>()); + } else { + __ cmpl(lhs.AsRegister<CpuRegister>(), Immediate(constant)); + } + } else { + __ cmpl(lhs.AsRegister<CpuRegister>(), + Address(CpuRegister(RSP), rhs.GetStackIndex())); + } + if (true_target == nullptr) { + __ j(X86_64IntegerCondition(condition->GetOppositeCondition()), false_target); + } else { __ j(X86_64IntegerCondition(condition->GetCondition()), true_target); } } - if (false_target != nullptr) { + + // If neither branch falls through (case 3), the conditional branch to `true_target` + // was already emitted (case 2) and we need to emit a jump to `false_target`. + if (true_target != nullptr && false_target != nullptr) { __ jmp(false_target); } } void LocationsBuilderX86_64::VisitIf(HIf* if_instr) { - LocationSummary* locations = - new (GetGraph()->GetArena()) LocationSummary(if_instr, LocationSummary::kNoCall); - HInstruction* cond = if_instr->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(if_instr); + if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) { locations->SetInAt(0, Location::Any()); } } void InstructionCodeGeneratorX86_64::VisitIf(HIf* if_instr) { - Label* true_target = codegen_->GetLabelOf(if_instr->IfTrueSuccessor()); - Label* false_target = codegen_->GetLabelOf(if_instr->IfFalseSuccessor()); - Label* always_true_target = true_target; - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfTrueSuccessor())) { - always_true_target = nullptr; - } - if (codegen_->GoesToNextBlock(if_instr->GetBlock(), - if_instr->IfFalseSuccessor())) { - false_target = nullptr; - } - GenerateTestAndBranch(if_instr, true_target, false_target, always_true_target); + HBasicBlock* true_successor = if_instr->IfTrueSuccessor(); + HBasicBlock* false_successor = if_instr->IfFalseSuccessor(); + Label* true_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), true_successor) ? + nullptr : codegen_->GetLabelOf(true_successor); + Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ? + nullptr : codegen_->GetLabelOf(false_successor); + GenerateTestAndBranch(if_instr, /* condition_input_index */ 0, true_target, false_target); } void LocationsBuilderX86_64::VisitDeoptimize(HDeoptimize* deoptimize) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(deoptimize, LocationSummary::kCallOnSlowPath); - HInstruction* cond = deoptimize->InputAt(0); - if (!cond->IsCondition() || cond->AsCondition()->NeedsMaterialization()) { + if (IsBooleanValueOrMaterializedCondition(deoptimize->InputAt(0))) { locations->SetInAt(0, Location::Any()); } } @@ -1519,8 +1517,10 @@ void InstructionCodeGeneratorX86_64::VisitDeoptimize(HDeoptimize* deoptimize) { SlowPathCode* slow_path = new (GetGraph()->GetArena()) DeoptimizationSlowPathX86_64(deoptimize); codegen_->AddSlowPath(slow_path); - Label* slow_path_entry = slow_path->GetEntryLabel(); - GenerateTestAndBranch(deoptimize, slow_path_entry, nullptr, slow_path_entry); + GenerateTestAndBranch(deoptimize, + /* condition_input_index */ 0, + slow_path->GetEntryLabel(), + /* false_target */ nullptr); } void LocationsBuilderX86_64::VisitLocal(HLocal* local) { diff --git a/compiler/optimizing/code_generator_x86_64.h b/compiler/optimizing/code_generator_x86_64.h index 5791fcd0e6..145b1f33b4 100644 --- a/compiler/optimizing/code_generator_x86_64.h +++ b/compiler/optimizing/code_generator_x86_64.h @@ -217,14 +217,12 @@ class InstructionCodeGeneratorX86_64 : public HGraphVisitor { void PushOntoFPStack(Location source, uint32_t temp_offset, uint32_t stack_adjustment, bool is_float); void GenerateTestAndBranch(HInstruction* instruction, + size_t condition_input_index, Label* true_target, - Label* false_target, - Label* always_true_target); - void GenerateCompareTestAndBranch(HIf* if_inst, - HCondition* condition, + Label* false_target); + void GenerateCompareTestAndBranch(HCondition* condition, Label* true_target, - Label* false_target, - Label* always_true_target); + Label* false_target); void GenerateFPJumps(HCondition* cond, Label* true_label, Label* false_label); void HandleGoto(HInstruction* got, HBasicBlock* successor); |
