Remove TryRemovingNullCheck from LSE
TryRemovingNullCheck would remove null checks right before eliminated loads. However, it was doing it without really checking if the object it was loading from was null or not.
This has been there since the first iteration of LSE, and my guess why it was implemented it was to remove NullChecks with no uses (i.e. it was assuming that the NullCheck was only uses by the load which was being eliminated).
Bug: 243136201
Test: Ran ART benchmark automator and saw no increase in code size
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Change-Id: I96be9650f8b16fee7fcca114b9b330d4f3b7e0b4
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 89d9fca..d4ceca5 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -855,25 +855,6 @@
}
}
- // `instruction` is being removed. Try to see if the null check on it
- // can be removed. This can happen if the same value is set in two branches
- // but not in dominators. Such as:
- // int[] a = foo();
- // if () {
- // a[0] = 2;
- // } else {
- // a[0] = 2;
- // }
- // // a[0] can now be replaced with constant 2, and the null check on it can be removed.
- void TryRemovingNullCheck(HInstruction* instruction) {
- HInstruction* prev = instruction->GetPrevious();
- if ((prev != nullptr) && prev->IsNullCheck() && (prev == instruction->InputAt(0))) {
- // Previous instruction is a null check for this instruction. Remove the null check.
- prev->ReplaceWith(prev->InputAt(0));
- prev->GetBlock()->RemoveInstruction(prev);
- }
- }
-
HInstruction* GetDefaultValue(DataType::Type type) {
switch (type) {
case DataType::Type::kReference:
@@ -1884,7 +1865,6 @@
}
HInstruction* heap_value = FindSubstitute(record.value.GetInstruction());
AddRemovedLoad(instruction, heap_value);
- TryRemovingNullCheck(instruction);
}
}
@@ -2645,7 +2625,6 @@
record.value = local_heap_values[idx];
HInstruction* heap_value = local_heap_values[idx].GetInstruction();
AddRemovedLoad(load_or_store, heap_value);
- TryRemovingNullCheck(load_or_store);
}
}
}
@@ -2704,7 +2683,6 @@
record.value = Replacement(record.value);
HInstruction* heap_value = record.value.GetInstruction();
AddRemovedLoad(load, heap_value);
- TryRemovingNullCheck(load);
}
}
}
diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java
index 5d55d3e..1a3708b 100644
--- a/test/530-checker-lse/src/Main.java
+++ b/test/530-checker-lse/src/Main.java
@@ -217,12 +217,9 @@
/// CHECK-DAG: Return
/// CHECK-START: int Main.test4(TestClass, boolean) load_store_elimination (after)
- /// CHECK: NullCheck
- /// CHECK: NullCheck
- /// CHECK-NOT: NullCheck
+ /// CHECK-NOT: InstanceFieldGet
/// CHECK-START: int Main.test4(TestClass, boolean) load_store_elimination (after)
- /// CHECK-NOT: InstanceFieldGet
/// CHECK-NOT: Phi
// Set and merge the same value in two branches.