Allow HDeoptimize to co-exist with LSE.
The env use of HDeoptimize counts as escaping at the end of
executing the compiled code for a singleton, just like a return.
So store elimination won't happen for that singleton's locations.
Test: make test-art-host
Bug: 31716107
Change-Id: I09838e80586d279714c676a2f7294ae518f61457
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 5b2cbf7..15e6059 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -33,11 +33,11 @@
public:
ReferenceInfo(HInstruction* reference, size_t pos) : reference_(reference), position_(pos) {
is_singleton_ = true;
- is_singleton_and_not_returned_ = true;
+ is_singleton_and_non_escaping_ = true;
if (!reference_->IsNewInstance() && !reference_->IsNewArray()) {
// For references not allocated in the method, don't assume anything.
is_singleton_ = false;
- is_singleton_and_not_returned_ = false;
+ is_singleton_and_non_escaping_ = false;
return;
}
@@ -50,7 +50,7 @@
// BoundType shouldn't normally be necessary for a NewInstance.
// Just be conservative for the uncommon cases.
is_singleton_ = false;
- is_singleton_and_not_returned_ = false;
+ is_singleton_and_non_escaping_ = false;
return;
}
if (user->IsPhi() || user->IsSelect() || user->IsInvoke() ||
@@ -62,21 +62,37 @@
// reference_ is merged to HPhi/HSelect, passed to a callee, or stored to heap.
// reference_ isn't the only name that can refer to its value anymore.
is_singleton_ = false;
- is_singleton_and_not_returned_ = false;
+ is_singleton_and_non_escaping_ = false;
return;
}
if ((user->IsUnresolvedInstanceFieldGet() && (reference_ == user->InputAt(0))) ||
(user->IsUnresolvedInstanceFieldSet() && (reference_ == user->InputAt(0)))) {
- // The field is accessed in an unresolved way. We mark the object as a singleton to
- // disable load/store optimizations on it.
+ // The field is accessed in an unresolved way. We mark the object as a non-singleton
+ // to disable load/store optimizations on it.
// Note that we could optimize this case and still perform some optimizations until
// we hit the unresolved access, but disabling is the simplest.
is_singleton_ = false;
- is_singleton_and_not_returned_ = false;
+ is_singleton_and_non_escaping_ = false;
return;
}
if (user->IsReturn()) {
- is_singleton_and_not_returned_ = false;
+ is_singleton_and_non_escaping_ = false;
+ }
+ }
+
+ if (!is_singleton_ || !is_singleton_and_non_escaping_) {
+ return;
+ }
+
+ // Look at Environment uses and if it's for HDeoptimize, it's treated the same
+ // as a return which escapes at the end of executing the compiled code. We don't
+ // do store elimination for singletons that escape through HDeoptimize.
+ // Other Environment uses are fine since LSE is disabled for debuggable.
+ for (const HUseListNode<HEnvironment*>& use : reference_->GetEnvUses()) {
+ HEnvironment* user = use.GetUser();
+ if (user->GetHolder()->IsDeoptimize()) {
+ is_singleton_and_non_escaping_ = false;
+ break;
}
}
}
@@ -96,17 +112,22 @@
return is_singleton_;
}
- // Returns true if reference_ is a singleton and not returned to the caller.
+ // Returns true if reference_ is a singleton and not returned to the caller or
+ // used as an environment local of an HDeoptimize instruction.
// The allocation and stores into reference_ may be eliminated for such cases.
- bool IsSingletonAndNotReturned() const {
- return is_singleton_and_not_returned_;
+ bool IsSingletonAndNonEscaping() const {
+ return is_singleton_and_non_escaping_;
}
private:
HInstruction* const reference_;
const size_t position_; // position in HeapLocationCollector's ref_info_array_.
bool is_singleton_; // can only be referred to by a single name in the method.
- bool is_singleton_and_not_returned_; // reference_ is singleton and not returned to caller.
+
+ // reference_ is singleton and does not escape in the end either by
+ // returning to the caller, or being used as an environment local of an
+ // HDeoptimize instruction.
+ bool is_singleton_and_non_escaping_;
DISALLOW_COPY_AND_ASSIGN(ReferenceInfo);
};
@@ -202,8 +223,7 @@
kArenaAllocLSE),
has_heap_stores_(false),
has_volatile_(false),
- has_monitor_operations_(false),
- may_deoptimize_(false) {}
+ has_monitor_operations_(false) {}
size_t GetNumberOfHeapLocations() const {
return heap_locations_.size();
@@ -236,13 +256,6 @@
return has_monitor_operations_;
}
- // Returns whether this method may be deoptimized.
- // Currently we don't have meta data support for deoptimizing
- // a method that eliminates allocations/stores.
- bool MayDeoptimize() const {
- return may_deoptimize_;
- }
-
// Find and return the heap location index in heap_locations_.
size_t FindHeapLocationIndex(ReferenceInfo* ref_info,
size_t offset,
@@ -493,10 +506,6 @@
CreateReferenceInfoForReferenceType(instruction);
}
- void VisitDeoptimize(HDeoptimize* instruction ATTRIBUTE_UNUSED) OVERRIDE {
- may_deoptimize_ = true;
- }
-
void VisitMonitorOperation(HMonitorOperation* monitor ATTRIBUTE_UNUSED) OVERRIDE {
has_monitor_operations_ = true;
}
@@ -508,7 +517,6 @@
// alias analysis and won't be as effective.
bool has_volatile_; // If there are volatile field accesses.
bool has_monitor_operations_; // If there are monitor operations.
- bool may_deoptimize_; // Only true for HDeoptimize with single-frame deoptimization.
DISALLOW_COPY_AND_ASSIGN(HeapLocationCollector);
};
@@ -671,7 +679,7 @@
bool from_all_predecessors = true;
ReferenceInfo* ref_info = heap_location_collector_.GetHeapLocation(i)->GetReferenceInfo();
HInstruction* singleton_ref = nullptr;
- if (ref_info->IsSingletonAndNotReturned()) {
+ if (ref_info->IsSingletonAndNonEscaping()) {
// We do more analysis of liveness when merging heap values for such
// cases since stores into such references may potentially be eliminated.
singleton_ref = ref_info->GetReference();
@@ -844,8 +852,7 @@
} else if (index != nullptr) {
// For array element, don't eliminate stores since it can be easily aliased
// with non-constant index.
- } else if (!heap_location_collector_.MayDeoptimize() &&
- ref_info->IsSingletonAndNotReturned()) {
+ } else if (ref_info->IsSingletonAndNonEscaping()) {
// Store into a field of a singleton that's not returned. The value cannot be
// killed due to aliasing/invocation. It can be redundant since future loads can
// directly get the value set by this instruction. The value can still be killed due to
@@ -1019,8 +1026,7 @@
// new_instance isn't used for field accesses. No need to process it.
return;
}
- if (!heap_location_collector_.MayDeoptimize() &&
- ref_info->IsSingletonAndNotReturned() &&
+ if (ref_info->IsSingletonAndNonEscaping() &&
!new_instance->IsFinalizable() &&
!new_instance->NeedsAccessCheck()) {
singleton_new_instances_.push_back(new_instance);
diff --git a/test/530-checker-lse/expected.txt b/test/530-checker-lse/expected.txt
index e69de29..ddae16a 100644
--- a/test/530-checker-lse/expected.txt
+++ b/test/530-checker-lse/expected.txt
@@ -0,0 +1 @@
+java.lang.ArrayIndexOutOfBoundsException: length=3; index=3
diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java
index a61b9a0..9f4be6c 100644
--- a/test/530-checker-lse/src/Main.java
+++ b/test/530-checker-lse/src/Main.java
@@ -18,6 +18,9 @@
Circle(double radius) {
this.radius = radius;
}
+ public double getRadius() {
+ return radius;
+ }
public double getArea() {
return radius * radius * Math.PI;
}
@@ -758,6 +761,30 @@
return area;
}
+ /// CHECK-START: double Main.testDeoptimize(int[], double[], double) load_store_elimination (before)
+ /// CHECK: Deoptimize
+ /// CHECK: NewInstance
+ /// CHECK: Deoptimize
+ /// CHECK: NewInstance
+
+ /// CHECK-START: double Main.testDeoptimize(int[], double[], double) load_store_elimination (after)
+ /// CHECK: Deoptimize
+ /// CHECK: NewInstance
+ /// CHECK: Deoptimize
+ /// CHECK-NOT: NewInstance
+
+ private static double testDeoptimize(int[] iarr, double[] darr, double radius) {
+ iarr[0] = 1; // One HDeoptimize here. Not triggered.
+ iarr[1] = 1;
+ Circle circle1 = new Circle(radius);
+ iarr[2] = 1;
+ darr[0] = circle1.getRadius(); // One HDeoptimize here, which holds circle1 live. Triggered.
+ darr[1] = circle1.getRadius();
+ darr[2] = circle1.getRadius();
+ darr[3] = circle1.getRadius();
+ return new Circle(Math.PI).getArea();
+ }
+
static void assertIntEquals(int result, int expected) {
if (expected != result) {
throw new Error("Expected: " + expected + ", found: " + result);
@@ -824,6 +851,20 @@
assertFloatEquals(mF, 0f);
assertDoubleEquals(Math.PI * Math.PI * Math.PI, getCircleArea(Math.PI, true));
assertDoubleEquals(0d, getCircleArea(Math.PI, false));
+
+ int[] iarray = {0, 0, 0};
+ double[] darray = {0d, 0d, 0d};
+ try {
+ assertDoubleEquals(Math.PI * Math.PI * Math.PI, testDeoptimize(iarray, darray, Math.PI));
+ } catch (Exception e) {
+ System.out.println(e);
+ }
+ assertIntEquals(iarray[0], 1);
+ assertIntEquals(iarray[1], 1);
+ assertIntEquals(iarray[2], 1);
+ assertDoubleEquals(darray[0], Math.PI);
+ assertDoubleEquals(darray[1], Math.PI);
+ assertDoubleEquals(darray[2], Math.PI);
}
static boolean sFlag;
diff --git a/test/530-checker-lse2/expected.txt b/test/530-checker-lse2/expected.txt
new file mode 100644
index 0000000..e18fc7e
--- /dev/null
+++ b/test/530-checker-lse2/expected.txt
@@ -0,0 +1,8 @@
+Start....
+r = 9.649776E8
+mZ = false
+mI = 0
+mJ = -576460752303423488
+mF = NaN
+mD = NaN
+Done....
diff --git a/test/530-checker-lse2/info.txt b/test/530-checker-lse2/info.txt
new file mode 100644
index 0000000..8dd3f50
--- /dev/null
+++ b/test/530-checker-lse2/info.txt
@@ -0,0 +1,2 @@
+Checker test for testing store/allocation elimination in presence of
+HDeoptimize.
diff --git a/test/530-checker-lse2/src/Main.java b/test/530-checker-lse2/src/Main.java
new file mode 100644
index 0000000..0fe3d87
--- /dev/null
+++ b/test/530-checker-lse2/src/Main.java
@@ -0,0 +1,208 @@
+/*
+ * 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.
+ */
+
+import java.util.Arrays;
+
+// Modified from a fuzz test.
+public class Main {
+
+ private interface X {
+ int x();
+ }
+
+ private class A {
+ public int a() {
+ return (+ (Math.multiplyExact(mI, mI)));
+ }
+ }
+
+ private class B extends A implements X {
+ public int a() {
+ return super.a() + ((int) (Math.max(364746077.0f, ((float) mD))));
+ }
+ public int x() {
+ return (mI >> (mI++));
+ }
+ }
+
+ private static class C implements X {
+ public static int s() {
+ return 671468641;
+ }
+ public int c() {
+ return -383762838;
+ }
+ public int x() {
+ return -138813312;
+ }
+ }
+
+ private A mA = new B();
+ private B mB = new B();
+ private X mBX = new B();
+ private C mC = new C();
+ private X mCX = new C();
+
+ private boolean mZ = false;
+ private int mI = 0;
+ private long mJ = 0;
+ private float mF = 0;
+ private double mD = 0;
+
+ private boolean[] mArray = new boolean[576];
+
+ private Main() {
+ boolean a = false;
+ for (int i0 = 0; i0 < 576; i0++) {
+ mArray[i0] = a;
+ a = !a;
+ }
+ }
+
+ /// CHECK-START: float Main.testMethod() load_store_elimination (before)
+ /// CHECK-DAG: Deoptimize
+ /// CHECK-DAG: Deoptimize
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-DAG: NewInstance
+ /// CHECK-NOT: NewInstance
+
+ /// CHECK-START: float Main.testMethod() load_store_elimination (after)
+ /// CHECK-DAG: Deoptimize
+ /// CHECK-DAG: Deoptimize
+ /// CHECK-NOT: NewInstance
+
+ private float testMethod() {
+ {
+ int lI0 = (-1456058746 << mI);
+ mD = ((double)(int)(double) mD);
+ for (int i0 = 56 - 1; i0 >= 0; i0--) {
+ mArray[i0] &= (Boolean.logicalOr(((true ? ((boolean) new Boolean((mZ))) : mZ) || mArray[i0]), (mZ)));
+ mF *= (mF * mF);
+ if ((mZ ^ true)) {
+ mF *= ((float)(int)(float) 267827331.0f);
+ mZ ^= ((false & ((boolean) new Boolean(false))) | mZ);
+ for (int i1 = 576 - 1; i1 >= 0; i1--) {
+ mZ &= ((mArray[279]) | ((boolean) new Boolean(true)));
+ mD -= (--mD);
+ for (int i2 = 56 - 1; i2 >= 0; i2--) {
+ mF /= (mF - mF);
+ mI = (Math.min(((int) new Integer(mI)), (766538816 * (++mI))));
+ mF += (mZ ? (mB.a()) : ((! mZ) ? -752042357.0f : (++mF)));
+ mJ |= ((long) new Long((-2084191070L + (mJ | mJ))));
+ lI0 |= ((int) new Integer(((int) new Integer(mI))));
+ if (((boolean) new Boolean(false))) {
+ mZ &= (mZ);
+ mF *= (mF--);
+ mD = (Double.POSITIVE_INFINITY);
+ mF += ((float)(int)(float) (-2026938813.0f * 638401585.0f));
+ mJ = (--mJ);
+ for (int i3 = 56 - 1; i3 >= 0; i3--) {
+ mI &= (- mI);
+ mD = (--mD);
+ mArray[426] = (mZ || false);
+ mF -= (((this instanceof Main) ? mF : mF) + 976981405.0f);
+ mZ &= ((mZ) & (this instanceof Main));
+ }
+ mZ ^= (Float.isFinite(-1975953895.0f));
+ } else {
+ mJ /= ((long) (Math.nextDown(-1519600008.0f)));
+ mJ <<= (Math.round(1237681786.0));
+ }
+ }
+ mArray[i0] &= (false || ((1256071300.0f != -353296391.0f) ? false : (mZ ^ mArray[i0])));
+ mF *= (+ ((float) mD));
+ for (int i2 = 0; i2 < 576; i2++) {
+ mD *= ((double) lI0);
+ lI0 = (lI0 & (Integer.MIN_VALUE));
+ mF -= (--mF);
+ }
+ if ((this instanceof Main)) {
+ mZ ^= ((boolean) new Boolean(true));
+ } else {
+ {
+ int lI1 = (mZ ? (--lI0) : 1099574344);
+ mJ >>= (Math.incrementExact(mJ));
+ mJ = (~ -2103354070L);
+ }
+ }
+ }
+ } else {
+ mJ *= (- ((long) new Long(479832084L)));
+ mJ %= (Long.MAX_VALUE);
+ mD /= (--mD);
+ if ((mI > ((mBX.x()) << mI))) {
+ {
+ long lJ0 = (mJ--);
+ mI >>>= (mBX.x());
+ }
+ mF = (+ 505094603.0f);
+ mD *= (((boolean) new Boolean((! false))) ? mD : 1808773781.0);
+ mI *= (Integer.MIN_VALUE);
+ for (int i1 = 576 - 1; i1 >= 0; i1--) {
+ if (((boolean) new Boolean(false))) {
+ mD += ((double)(float)(double) -1051436901.0);
+ } else {
+ mF -= ((float)(int)(float) (Float.min(mF, (mF--))));
+ }
+ for (int i2 = 0; i2 < 576; i2++) {
+ mJ -= ((long) new Long(-1968644857L));
+ mJ ^= (+ (mC.s()));
+ }
+ }
+ } else {
+ mF -= ((- mF) + -2145489966.0f);
+ }
+ mD -= (mD++);
+ mD = (949112777.0 * 1209996119.0);
+ }
+ mZ &= (Boolean.logicalAnd(true, ((mZ) & (((boolean) new Boolean(true)) && true))));
+ }
+ }
+ return ((float) 964977619L);
+ }
+
+ public static void main(String[] args) {
+ System.out.println("Start....");
+ Main t = new Main();
+ float r = 1883600237.0f;
+ try {
+ r = t.testMethod();
+ } catch (Exception e) {
+ // Arithmetic, null pointer, index out of bounds, etc.
+ System.out.println("An exception was caught.");
+ }
+ System.out.println("r = " + r);
+ System.out.println("mZ = " + t.mZ);
+ System.out.println("mI = " + t.mI);
+ System.out.println("mJ = " + t.mJ);
+ System.out.println("mF = " + t.mF);
+ System.out.println("mD = " + t.mD);
+ System.out.println("Done....");
+ }
+}
+
