Fix arm64 simplifier bug that tries to remove same statement twice.
With fail-before/pass-after test (on arm64).
Rationale:
This visitor removes statement "forward", which is a bit unusual, and
exposes a bug if statement is revisited and qualifies for removal again.
BUG=27851582
Change-Id: Ia8cddba32b4dfe9fd480852deb358eaa977f0e1f
diff --git a/compiler/optimizing/instruction_simplifier_arm64.h b/compiler/optimizing/instruction_simplifier_arm64.h
index 338120b..da26998 100644
--- a/compiler/optimizing/instruction_simplifier_arm64.h
+++ b/compiler/optimizing/instruction_simplifier_arm64.h
@@ -51,6 +51,22 @@
return TryMergeIntoShifterOperand(use, bitfield_op, true);
}
+ /**
+ * This simplifier uses a special-purpose BB visitor.
+ * (1) No need to visit Phi nodes.
+ * (2) Since statements can be removed in a "forward" fashion,
+ * the visitor should test if each statement is still there.
+ */
+ void VisitBasicBlock(HBasicBlock* block) OVERRIDE {
+ // TODO: fragile iteration, provide more robust iterators?
+ for (HInstructionIterator it(block->GetInstructions()); !it.Done(); it.Advance()) {
+ HInstruction* instruction = it.Current();
+ if (instruction->IsInBlock()) {
+ instruction->Accept(this);
+ }
+ }
+ }
+
// HInstruction visitors, sorted alphabetically.
void VisitAnd(HAnd* instruction) OVERRIDE;
void VisitArrayGet(HArrayGet* instruction) OVERRIDE;
diff --git a/test/593-checker-shift-and-simplifier/expected.txt b/test/593-checker-shift-and-simplifier/expected.txt
new file mode 100644
index 0000000..b0aad4d
--- /dev/null
+++ b/test/593-checker-shift-and-simplifier/expected.txt
@@ -0,0 +1 @@
+passed
diff --git a/test/593-checker-shift-and-simplifier/info.txt b/test/593-checker-shift-and-simplifier/info.txt
new file mode 100644
index 0000000..2fae678
--- /dev/null
+++ b/test/593-checker-shift-and-simplifier/info.txt
@@ -0,0 +1 @@
+Regression test on pattern that caused double removal of AND by AMD64 simplifier.
diff --git a/test/593-checker-shift-and-simplifier/src/Main.java b/test/593-checker-shift-and-simplifier/src/Main.java
new file mode 100644
index 0000000..65e809a
--- /dev/null
+++ b/test/593-checker-shift-and-simplifier/src/Main.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2016 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 {
+
+ private static int[] a = { 10 };
+
+ // A very particular set of operations that caused a double removal by the
+ // ARM64 simplifier doing "forward" removals (b/27851582).
+
+ /// CHECK-START-ARM64: int Main.operations() instruction_simplifier_arm64 (before)
+ /// CHECK-DAG: <<Get:i\d+>> ArrayGet
+ /// CHECK-DAG: <<Not:i\d+>> Not [<<Get>>]
+ /// CHECK-DAG: <<Shl:i\d+>> Shl [<<Get>>,i{{\d+}}]
+ /// CHECK-DAG: And [<<Not>>,<<Shl>>]
+ //
+ /// CHECK-START-ARM64: int Main.operations() instruction_simplifier_arm64 (after)
+ /// CHECK-DAG: <<Get:i\d+>> ArrayGet
+ /// CHECK-DAG: <<Not:i\d+>> Not [<<Get>>]
+ /// CHECK-DAG: Arm64DataProcWithShifterOp [<<Not>>,<<Get>>] kind:And+LSL shift:2
+ private static int operations() {
+ int r = a[0];
+ int n = ~r;
+ int s = r << 2;
+ int a = s & n;
+ return a;
+ }
+
+ public static void main(String[] args) {
+ if (operations() != 32) {
+ System.out.println("failed");
+ } else {
+ System.out.println("passed");
+ }
+ }
+}
