Fix issue with Partial LSE and casts/instanceof
If PartialLSE encounters an instanceof or check-cast before the
escapes it could not correctly handle it. Due to how java language
code is typically developed and compiled this is generally not a
problem but could lead to incorrect codegen on release builds or
DCHECK failures on debug builds. This fixes the issues by (1) causing
partial LSE to consider check-cast and instance-ofs to be escaping.
This also updates the instruction simplifier to be much more
aggressive in removing instance-of and check-casts.
Test: ./test.py --host
Bug: 186041085
Change-Id: Ia513c4210a87a0dfa92f10adc530e17ee631d006
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 938a775..23a432e 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -611,7 +611,16 @@
if (!class_rti.IsValid()) {
// Happens when the loaded class is unresolved.
- return false;
+ if (obj_rti.IsExact()) {
+ // outcome == 'true' && obj_rti is valid implies that class_rti is valid.
+ // Since that's a contradiction we must not pass this check.
+ *outcome = false;
+ return true;
+ } else {
+ // We aren't able to say anything in particular since we don't know the
+ // exact type of the object.
+ return false;
+ }
}
DCHECK(class_rti.IsExact());
if (class_rti.IsSupertypeOf(obj_rti)) {
@@ -633,12 +642,6 @@
void InstructionSimplifierVisitor::VisitCheckCast(HCheckCast* check_cast) {
HInstruction* object = check_cast->InputAt(0);
- if (check_cast->GetTypeCheckKind() != TypeCheckKind::kBitstringCheck &&
- check_cast->GetTargetClass()->NeedsAccessCheck()) {
- // If we need to perform an access check we cannot remove the instruction.
- return;
- }
-
if (CanEnsureNotNullAt(object, check_cast)) {
check_cast->ClearMustDoNullCheck();
}
@@ -649,6 +652,11 @@
return;
}
+ // Minor correctness check.
+ DCHECK(check_cast->GetTargetClass()->StrictlyDominates(check_cast))
+ << "Illegal graph!\n"
+ << check_cast->DumpWithArgs();
+
// Historical note: The `outcome` was initialized to please Valgrind - the compiler can reorder
// the return value check with the `outcome` check, b/27651442.
bool outcome = false;
@@ -658,27 +666,23 @@
MaybeRecordStat(stats_, MethodCompilationStat::kRemovedCheckedCast);
if (check_cast->GetTypeCheckKind() != TypeCheckKind::kBitstringCheck) {
HLoadClass* load_class = check_cast->GetTargetClass();
- if (!load_class->HasUses()) {
+ if (!load_class->HasUses() && !load_class->NeedsAccessCheck()) {
// We cannot rely on DCE to remove the class because the `HLoadClass` thinks it can throw.
- // However, here we know that it cannot because the checkcast was successfull, hence
+ // However, here we know that it cannot because the checkcast was successful, hence
// the class was already loaded.
load_class->GetBlock()->RemoveInstruction(load_class);
}
}
} else {
- // Don't do anything for exceptional cases for now. Ideally we should remove
- // all instructions and blocks this instruction dominates.
+ // TODO Don't do anything for exceptional cases for now. Ideally we should
+ // remove all instructions and blocks this instruction dominates and
+ // replace it with a manual throw.
}
}
}
void InstructionSimplifierVisitor::VisitInstanceOf(HInstanceOf* instruction) {
HInstruction* object = instruction->InputAt(0);
- if (instruction->GetTypeCheckKind() != TypeCheckKind::kBitstringCheck &&
- instruction->GetTargetClass()->NeedsAccessCheck()) {
- // If we need to perform an access check we cannot remove the instruction.
- return;
- }
bool can_be_null = true;
if (CanEnsureNotNullAt(object, instruction)) {
@@ -695,6 +699,11 @@
return;
}
+ // Minor correctness check.
+ DCHECK(instruction->GetTargetClass()->StrictlyDominates(instruction))
+ << "Illegal graph!\n"
+ << instruction->DumpWithArgs();
+
// Historical note: The `outcome` was initialized to please Valgrind - the compiler can reorder
// the return value check with the `outcome` check, b/27651442.
bool outcome = false;
@@ -713,10 +722,11 @@
instruction->GetBlock()->RemoveInstruction(instruction);
if (outcome && instruction->GetTypeCheckKind() != TypeCheckKind::kBitstringCheck) {
HLoadClass* load_class = instruction->GetTargetClass();
- if (!load_class->HasUses()) {
- // We cannot rely on DCE to remove the class because the `HLoadClass` thinks it can throw.
- // However, here we know that it cannot because the instanceof check was successfull, hence
- // the class was already loaded.
+ if (!load_class->HasUses() && !load_class->NeedsAccessCheck()) {
+ // We cannot rely on DCE to remove the class because the `HLoadClass`
+ // thinks it can throw. However, here we know that it cannot because the
+ // instanceof check was successful and we don't need to check the
+ // access, hence the class was already loaded.
load_class->GetBlock()->RemoveInstruction(load_class);
}
}