Fix braino when handling branches fallthrough in arm backend.
bug: 62210114
Test: 657-branches
Change-Id: I753a9a57e404c792cd4375ea66c91839684bdee2
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index 0b3ac20..6b9f232 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -2875,21 +2875,28 @@
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 34397e6..2561ed0 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -3608,9 +3608,6 @@
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) {
@@ -3711,10 +3708,6 @@
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 a8b00c3..9a2402b 100644
--- a/compiler/optimizing/code_generator_arm_vixl.cc
+++ b/compiler/optimizing/code_generator_arm_vixl.cc
@@ -2964,21 +2964,28 @@
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 0000000..d9fd864
--- /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 0000000..84a2bb9
--- /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 0000000..2b62c5f
--- /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);
+ }
+}
