Allow store elimination for singleton that's returned
Allow store elimination for singleton that's visible after method return
or deoptimization. Add additional detection for keeping stores for such
singletons at block merge/deoptimization point.
Bug: 35745320
Test: m test-art-host
Change-Id: I8a75a304491dafaeb689787402afa3d7468e3789
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 46ba048..48699b3 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -69,6 +69,13 @@
return is_singleton_and_not_returned_ && is_singleton_and_not_deopt_visible_;
}
+ // Returns true if reference_ is a singleton and returned to the caller or
+ // used as an environment local of an HDeoptimize instruction.
+ bool IsSingletonAndNonRemovable() const {
+ return is_singleton_ &&
+ (!is_singleton_and_not_returned_ || !is_singleton_and_not_deopt_visible_);
+ }
+
bool HasIndexAliasing() {
return has_index_aliasing_;
}
@@ -339,11 +346,7 @@
return false;
}
ReferenceInfo* ref_info = loc1->GetReferenceInfo();
- if (ref_info->IsSingleton()) {
- // This is guaranteed by the CanReferencesAlias() test above.
- DCHECK_EQ(ref_info, loc2->GetReferenceInfo());
- ref_info->SetHasIndexAliasing(true);
- }
+ ref_info->SetHasIndexAliasing(true);
}
return true;
}
@@ -628,14 +631,17 @@
for (size_t i = 0; i < heap_values.size(); i++) {
HeapLocation* location = heap_location_collector_.GetHeapLocation(i);
ReferenceInfo* ref_info = location->GetReferenceInfo();
- if (!ref_info->IsSingleton() || location->IsValueKilledByLoopSideEffects()) {
- // heap value is killed by loop side effects (stored into directly, or due to
- // aliasing).
+ if (ref_info->IsSingletonAndRemovable() &&
+ !location->IsValueKilledByLoopSideEffects()) {
+ // A removable singleton's field that's not stored into inside a loop is
+ // invariant throughout the loop. Nothing to do.
+ DCHECK(ref_info->IsSingletonAndRemovable());
+ } else {
+ // heap value is killed by loop side effects (stored into directly, or
+ // due to aliasing). Or the heap value may be needed after method return
+ // or deoptimization.
KeepIfIsStore(pre_header_heap_values[i]);
heap_values[i] = kUnknownHeapValue;
- } else {
- // A singleton's field that's not stored into inside a loop is invariant throughout
- // the loop.
}
}
}
@@ -654,7 +660,7 @@
bool from_all_predecessors = true;
ReferenceInfo* ref_info = heap_location_collector_.GetHeapLocation(i)->GetReferenceInfo();
HInstruction* singleton_ref = nullptr;
- if (ref_info->IsSingletonAndRemovable()) {
+ if (ref_info->IsSingleton()) {
// 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();
@@ -680,8 +686,9 @@
}
}
- if (merged_value == kUnknownHeapValue) {
- // There are conflicting heap values from different predecessors.
+ if (merged_value == kUnknownHeapValue || ref_info->IsSingletonAndNonRemovable()) {
+ // There are conflicting heap values from different predecessors,
+ // or the heap value may be needed after method return or deoptimization.
// Keep the last store in each predecessor since future loads cannot be eliminated.
for (HBasicBlock* predecessor : predecessors) {
ArenaVector<HInstruction*>& pred_values = heap_values_for_[predecessor->GetBlockId()];
@@ -828,15 +835,15 @@
// Store into the heap location with the same value.
same_value = true;
} else if (index != nullptr && ref_info->HasIndexAliasing()) {
- // For array element, don't eliminate stores if the index can be
- // aliased.
- } else if (ref_info->IsSingletonAndRemovable()) {
- // Store into a field/element of a singleton instance/array 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 merging or loop side effects. Stores whose values are
- // killed due to merging/loop side effects later will be removed from
- // possibly_removed_stores_ when that is detected.
+ // For array element, don't eliminate stores if the index can be aliased.
+ } else if (ref_info->IsSingleton()) {
+ // Store into a field of a singleton. 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
+ // merging or loop side effects. Stores whose values are killed due to merging/loop side
+ // effects later will be removed from possibly_removed_stores_ when that is detected.
+ // Stores whose values may be needed after method return or deoptimization
+ // are also removed from possibly_removed_stores_ when that is detected.
possibly_redundant = true;
HNewInstance* new_instance = ref_info->GetReference()->AsNewInstance();
if (new_instance != nullptr && new_instance->IsFinalizable()) {
@@ -945,6 +952,33 @@
value);
}
+ void VisitDeoptimize(HDeoptimize* instruction) {
+ const ArenaVector<HInstruction*>& heap_values =
+ heap_values_for_[instruction->GetBlock()->GetBlockId()];
+ for (HInstruction* heap_value : heap_values) {
+ // Filter out fake instructions before checking instruction kind below.
+ if (heap_value == kUnknownHeapValue || heap_value == kDefaultHeapValue) {
+ continue;
+ }
+ // A store is kept as the heap value for possibly removed stores.
+ if (heap_value->IsInstanceFieldSet() || heap_value->IsArraySet()) {
+ // Check whether the reference for a store is used by an environment local of
+ // HDeoptimize.
+ HInstruction* reference = heap_value->InputAt(0);
+ DCHECK(heap_location_collector_.FindReferenceInfoOf(reference)->IsSingleton());
+ for (const HUseListNode<HEnvironment*>& use : reference->GetEnvUses()) {
+ HEnvironment* user = use.GetUser();
+ if (user->GetHolder() == instruction) {
+ // The singleton for the store is visible at this deoptimization
+ // point. Need to keep the store so that the heap value is
+ // seen by the interpreter.
+ KeepIfIsStore(heap_value);
+ }
+ }
+ }
+ }
+ }
+
void HandleInvoke(HInstruction* invoke) {
ArenaVector<HInstruction*>& heap_values =
heap_values_for_[invoke->GetBlock()->GetBlockId()];
diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java
index 14a40ef..6632503 100644
--- a/test/530-checker-lse/src/Main.java
+++ b/test/530-checker-lse/src/Main.java
@@ -747,6 +747,69 @@
return 1.0f;
}
+ /// CHECK-START: TestClass2 Main.testStoreStore() load_store_elimination (before)
+ /// CHECK: NewInstance
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+
+ /// CHECK-START: TestClass2 Main.testStoreStore() load_store_elimination (after)
+ /// CHECK: NewInstance
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK-NOT: InstanceFieldSet
+
+ private static TestClass2 testStoreStore() {
+ TestClass2 obj = new TestClass2();
+ obj.i = 41;
+ obj.j = 42;
+ obj.i = 41;
+ obj.j = 43;
+ return obj;
+ }
+
+ /// CHECK-START: int Main.testStoreStoreWithDeoptimize(int[]) load_store_elimination (before)
+ /// CHECK: NewInstance
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK: Deoptimize
+ /// CHECK: ArraySet
+ /// CHECK: ArraySet
+ /// CHECK: ArraySet
+ /// CHECK: ArraySet
+ /// CHECK: ArrayGet
+ /// CHECK: ArrayGet
+ /// CHECK: ArrayGet
+ /// CHECK: ArrayGet
+
+ /// CHECK-START: int Main.testStoreStoreWithDeoptimize(int[]) load_store_elimination (after)
+ /// CHECK: NewInstance
+ /// CHECK: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
+ /// CHECK-NOT: InstanceFieldSet
+ /// CHECK: Deoptimize
+ /// CHECK: ArraySet
+ /// CHECK: ArraySet
+ /// CHECK: ArraySet
+ /// CHECK: ArraySet
+ /// CHECK-NOT: ArrayGet
+
+ private static int testStoreStoreWithDeoptimize(int[] arr) {
+ TestClass2 obj = new TestClass2();
+ obj.i = 41;
+ obj.j = 42;
+ obj.i = 41;
+ obj.j = 43;
+ arr[0] = 1; // One HDeoptimize here.
+ arr[1] = 1;
+ arr[2] = 1;
+ arr[3] = 1;
+ return arr[0] + arr[1] + arr[2] + arr[3];
+ }
+
/// CHECK-START: double Main.getCircleArea(double, boolean) load_store_elimination (before)
/// CHECK: NewInstance
@@ -950,6 +1013,10 @@
assertIntEquals(testAllocationEliminationOfArray2(), 11);
assertIntEquals(testAllocationEliminationOfArray3(2), 4);
assertIntEquals(testAllocationEliminationOfArray4(2), 6);
+
+ assertIntEquals(testStoreStore().i, 41);
+ assertIntEquals(testStoreStore().j, 43);
+ assertIntEquals(testStoreStoreWithDeoptimize(new int[4]), 4);
}
static boolean sFlag;