Refine where stores are visible in LSE
Code in catch blocks behaves as regular code, as long as it is not
inside of an outer try.
Bug: 227283233
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Change-Id: I8334a4716429bc743ae1c552f5d8d09b10d9e0e2
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index d4ceca5..cafc3e9 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -1020,9 +1020,25 @@
VisitSetLocation(instruction, idx, instruction->GetValue());
}
+ static bool IsBlockInsideATry(HBasicBlock* block) {
+ TryCatchInformation* try_catch_info = block->GetTryCatchInformation();
+ if (try_catch_info == nullptr) {
+ return false;
+ }
+
+ if (try_catch_info->IsTryBlock()) {
+ return true;
+ }
+
+ DCHECK(try_catch_info->IsCatchBlock());
+
+ // The catch block has an xhandler iff it is inside of an outer try.
+ return block->GetExceptionalSuccessors().size() != 0;
+ }
+
void VisitDeoptimize(HDeoptimize* instruction) override {
- // If we are in a try catch, even singletons are observable.
- const bool in_try_catch = instruction->GetBlock()->GetTryCatchInformation() != nullptr;
+ // If we are in a try, even singletons are observable.
+ const bool inside_a_try = IsBlockInsideATry(instruction->GetBlock());
HBasicBlock* block = instruction->GetBlock();
ScopedArenaVector<ValueRecord>& heap_values = heap_values_for_[block->GetBlockId()];
for (size_t i = 0u, size = heap_values.size(); i != size; ++i) {
@@ -1034,7 +1050,7 @@
// for singletons that don't escape in the deoptimization environment.
bool observable = true;
ReferenceInfo* info = heap_location_collector_.GetHeapLocation(i)->GetReferenceInfo();
- if (!in_try_catch && info->IsSingleton()) {
+ if (!inside_a_try && info->IsSingleton()) {
HInstruction* reference = info->GetReference();
// Finalizable objects always escape.
const bool finalizable_object =
@@ -1080,10 +1096,8 @@
void HandleThrowingInstruction(HInstruction* instruction) {
DCHECK(instruction->CanThrow());
- // If we are inside of a try catch, singletons can become visible since we may not exit the
- // method.
- HandleExit(instruction->GetBlock(),
- instruction->GetBlock()->GetTryCatchInformation() != nullptr);
+ // If we are inside of a try, singletons can become visible since we may not exit the method.
+ HandleExit(instruction->GetBlock(), IsBlockInsideATry(instruction->GetBlock()));
}
void VisitMethodEntryHook(HMethodEntryHook* method_entry) override {
@@ -1147,9 +1161,9 @@
void HandleInvoke(HInstruction* instruction) {
// If `instruction` can throw we have to presume all stores are visible.
const bool can_throw = instruction->CanThrow();
- // If we are in a try catch, even singletons are observable.
- const bool can_throw_in_try_catch =
- can_throw && instruction->GetBlock()->GetTryCatchInformation() != nullptr;
+ // If we are in a try, even singletons are observable.
+ const bool can_throw_inside_a_try =
+ can_throw && IsBlockInsideATry(instruction->GetBlock());
SideEffects side_effects = instruction->GetSideEffects();
ScopedArenaVector<ValueRecord>& heap_values =
heap_values_for_[instruction->GetBlock()->GetBlockId()];
@@ -1175,7 +1189,7 @@
return cohort.PrecedesBlock(blk);
});
};
- if (!can_throw_in_try_catch &&
+ if (!can_throw_inside_a_try &&
(ref_info->IsSingleton() ||
// partial and we aren't currently escaping and we haven't escaped yet.
(ref_info->IsPartialSingleton() && partial_singleton_did_not_escape(ref_info, blk)))) {
@@ -1224,8 +1238,8 @@
}
void VisitNewInstance(HNewInstance* new_instance) override {
- // If we are in a try catch, even singletons are observable.
- const bool in_try_catch = new_instance->GetBlock()->GetTryCatchInformation() != nullptr;
+ // If we are in a try, even singletons are observable.
+ const bool inside_a_try = IsBlockInsideATry(new_instance->GetBlock());
ReferenceInfo* ref_info = heap_location_collector_.FindReferenceInfoOf(new_instance);
if (ref_info == nullptr) {
// new_instance isn't used for field accesses. No need to process it.
@@ -1254,7 +1268,7 @@
heap_values[i].value = Value::ForInstruction(new_instance->GetLoadClass());
heap_values[i].stored_by = Value::PureUnknown();
}
- } else if (in_try_catch || IsEscapingObject(info, block, i)) {
+ } else if (inside_a_try || IsEscapingObject(info, block, i)) {
// Since NewInstance can throw, we presume all previous stores could be visible.
KeepStores(heap_values[i].stored_by);
heap_values[i].stored_by = Value::PureUnknown();
@@ -1263,8 +1277,8 @@
}
void VisitNewArray(HNewArray* new_array) override {
- // If we are in a try catch, even singletons are observable.
- const bool in_try_catch = new_array->GetBlock()->GetTryCatchInformation() != nullptr;
+ // If we are in a try, even singletons are observable.
+ const bool inside_a_try = IsBlockInsideATry(new_array->GetBlock());
ReferenceInfo* ref_info = heap_location_collector_.FindReferenceInfoOf(new_array);
if (ref_info == nullptr) {
// new_array isn't used for array accesses. No need to process it.
@@ -1289,7 +1303,7 @@
// Array elements are set to default heap values.
heap_values[i].value = Value::Default();
heap_values[i].stored_by = Value::PureUnknown();
- } else if (in_try_catch || IsEscapingObject(info, block, i)) {
+ } else if (inside_a_try || IsEscapingObject(info, block, i)) {
// Since NewArray can throw, we presume all previous stores could be visible.
KeepStores(heap_values[i].stored_by);
heap_values[i].stored_by = Value::PureUnknown();
diff --git a/test/530-checker-lse-try-catch/src/Main.java b/test/530-checker-lse-try-catch/src/Main.java
index 56ef8bd..68c877b 100644
--- a/test/530-checker-lse-try-catch/src/Main.java
+++ b/test/530-checker-lse-try-catch/src/Main.java
@@ -30,7 +30,11 @@
assertEquals(1, $noinline$testTryCatchBlocking(new Point(), boolean_throw));
assertEquals(1, $noinline$testTryCatchPhi(new Point(), boolean_throw));
assertEquals(2, $noinline$testTryCatchPhiWithTwoCatches(new Point(), new int[0]));
- assertEquals(1, $noinline$testKeepStoreInsideTryCatch());
+ assertEquals(1, $noinline$testKeepStoreInsideTry());
+ assertEquals(10, $noinline$testDontKeepStoreInsideCatch(new int[]{10}));
+ assertEquals(30, $noinline$testDontKeepStoreInsideCatch(new int[]{}));
+ assertEquals(10, $noinline$testKeepStoreInsideCatchWithOuterTry(new int[]{10}));
+ assertEquals(30, $noinline$testKeepStoreInsideCatchWithOuterTry(new int[]{}));
assertEquals(150, $noinline$test40());
}
@@ -205,17 +209,17 @@
// is observable.
// Consistency check to make sure the try/catch wasn't removed by an earlier pass.
- /// CHECK-START: int Main.$noinline$testKeepStoreInsideTryCatch() load_store_elimination (after)
+ /// CHECK-START: int Main.$noinline$testKeepStoreInsideTry() load_store_elimination (after)
/// CHECK-DAG: TryBoundary kind:entry
- /// CHECK-START: int Main.$noinline$testKeepStoreInsideTryCatch() load_store_elimination (before)
+ /// CHECK-START: int Main.$noinline$testKeepStoreInsideTry() load_store_elimination (before)
/// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
/// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
- /// CHECK-START: int Main.$noinline$testKeepStoreInsideTryCatch() load_store_elimination (after)
+ /// CHECK-START: int Main.$noinline$testKeepStoreInsideTry() load_store_elimination (after)
/// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
/// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
- private static int $noinline$testKeepStoreInsideTryCatch() {
+ private static int $noinline$testKeepStoreInsideTry() {
Main main = new Main();
main.sumForKeepStoreInsideTryCatch = 0;
try {
@@ -228,6 +232,58 @@
}
}
+ private static int $noinline$returnValue(int value) {
+ return value;
+ }
+
+ /// CHECK-START: int Main.$noinline$testDontKeepStoreInsideCatch(int[]) load_store_elimination (before)
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+
+ /// CHECK-START: int Main.$noinline$testDontKeepStoreInsideCatch(int[]) load_store_elimination (after)
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ private static int $noinline$testDontKeepStoreInsideCatch(int[] array) {
+ Main main = new Main();
+ int value = 0;
+ try {
+ value = array[0];
+ } catch (Exception e) {
+ // These sets can be eliminated even though we have invokes since this catch is not part of an
+ // outer try.
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(10);
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(20);
+ }
+ return main.sumForKeepStoreInsideTryCatch + value;
+ }
+
+ /// CHECK-START: int Main.$noinline$testKeepStoreInsideCatchWithOuterTry(int[]) load_store_elimination (before)
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+
+ /// CHECK-START: int Main.$noinline$testKeepStoreInsideCatchWithOuterTry(int[]) load_store_elimination (after)
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ /// CHECK-NOT: InstanceFieldSet field_name:Main.sumForKeepStoreInsideTryCatch
+ private static int $noinline$testKeepStoreInsideCatchWithOuterTry(int[] array) {
+ Main main = new Main();
+ int value = 0;
+ try {
+ try {
+ value = array[0];
+ } catch (Exception e) {
+ // These sets can't be eliminated since this catch is part of a outer try.
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(10);
+ main.sumForKeepStoreInsideTryCatch += $noinline$returnValue(20);
+ }
+ } catch (Exception e) {
+ value = 100000;
+ }
+
+ return main.sumForKeepStoreInsideTryCatch + value;
+ }
+
/// CHECK-START: int Main.$noinline$test40() load_store_elimination (before)
/// CHECK: ArraySet
/// CHECK: DivZeroCheck