diff options
| -rw-r--r-- | compiler/optimizing/code_generator_arm.cc | 9 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_arm64.cc | 7 | ||||
| -rw-r--r-- | compiler/optimizing/code_generator_arm_vixl.cc | 9 | ||||
| -rw-r--r-- | test/657-branches/expected.txt | 1 | ||||
| -rw-r--r-- | test/657-branches/info.txt | 2 | ||||
| -rw-r--r-- | test/657-branches/src/Main.java | 47 |
6 files changed, 66 insertions, 9 deletions
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index 097e4833d0..914ae177c4 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -2874,21 +2874,28 @@ void InstructionCodeGeneratorARM::GenerateCompareTestAndBranch(HCondition* condi if (CanGenerateTest(condition, codegen_->GetAssembler())) { Label* non_fallthrough_target; bool invert; + bool emit_both_branches; if (true_target_in == nullptr) { + // The true target is fallthrough. DCHECK(false_target_in != nullptr); non_fallthrough_target = false_target_in; invert = true; + emit_both_branches = false; } else { + // Either the false target is fallthrough, or there is no fallthrough + // and both branches must be emitted. non_fallthrough_target = true_target_in; invert = false; + emit_both_branches = (false_target_in != nullptr); } const auto cond = GenerateTest(condition, invert, codegen_); __ b(non_fallthrough_target, cond.first); - if (false_target_in != nullptr && false_target_in != non_fallthrough_target) { + if (emit_both_branches) { + // No target falls through, we need to branch. __ b(false_target_in); } diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index d8e709c7a9..f2b312362f 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -3607,9 +3607,6 @@ void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruct size_t condition_input_index, vixl::aarch64::Label* true_target, vixl::aarch64::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::aarch64::Label fallthrough_target; HInstruction* cond = instruction->InputAt(condition_input_index); if (true_target == nullptr && false_target == nullptr) { @@ -3710,10 +3707,6 @@ void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruct if (true_target != nullptr && false_target != nullptr) { __ B(false_target); } - - if (fallthrough_target.IsLinked()) { - __ Bind(&fallthrough_target); - } } void LocationsBuilderARM64::VisitIf(HIf* if_instr) { diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index 4d5f88e14a..93cbc3b17c 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -2963,21 +2963,28 @@ void InstructionCodeGeneratorARMVIXL::GenerateCompareTestAndBranch(HCondition* c if (CanGenerateTest(condition, codegen_->GetAssembler())) { vixl32::Label* non_fallthrough_target; bool invert; + bool emit_both_branches; if (true_target_in == nullptr) { + // The true target is fallthrough. DCHECK(false_target_in != nullptr); non_fallthrough_target = false_target_in; invert = true; + emit_both_branches = false; } else { non_fallthrough_target = true_target_in; invert = false; + // Either the false target is fallthrough, or there is no fallthrough + // and both branches must be emitted. + emit_both_branches = (false_target_in != nullptr); } const auto cond = GenerateTest(condition, invert, codegen_); __ B(cond.first, non_fallthrough_target, is_far_target); - if (false_target_in != nullptr && false_target_in != non_fallthrough_target) { + if (emit_both_branches) { + // No target falls through, we need to branch. __ B(false_target_in); } diff --git a/test/657-branches/expected.txt b/test/657-branches/expected.txt new file mode 100644 index 0000000000..d9fd864709 --- /dev/null +++ b/test/657-branches/expected.txt @@ -0,0 +1 @@ +Hello World: 4.0 diff --git a/test/657-branches/info.txt b/test/657-branches/info.txt new file mode 100644 index 0000000000..84a2bb908c --- /dev/null +++ b/test/657-branches/info.txt @@ -0,0 +1,2 @@ +Regression test for the ARM backend, which used to have a bug +handling branches fallthrough. diff --git a/test/657-branches/src/Main.java b/test/657-branches/src/Main.java new file mode 100644 index 0000000000..2b62c5faa1 --- /dev/null +++ b/test/657-branches/src/Main.java @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class Main { + + public static void foo(float f) { + // The reason this used to break: + // 1) We inline the 'foo' call, so blocks now only contain HLoadClass instructions. + // 2) We then run the select_generator pass, which cannot change the + // if/else because blocks contain instructions. + // 3) We run GVN which will remove the HLoadClass instructions in the blocks. + // 4) At code generation, we are in the unlikely situation that a diamond shape + // contains no instruction (usually removed by select_generator). This used + // to trip the ARM code generators. + if (f < 1.2f) { + foo(Main.class, Object.class); + if (f < 0.2f) { + foo(Main.class, Object.class); + } else { + foo(Main.class, Object.class); + } + } else { + System.out.println("Hello World: " + f); + } + } + + public static void foo(Object a, Object b) {} + + public static void main(String[] args) { + foo(0f); + foo(4f); + foo(0.1f); + } +} |