[optimizing] Fix a bug in moving the null check to the user.
When taking the decision to move a null check to the user we did not
verify if the next instruction checks the same object.
Change-Id: I2f4533a4bb18aa4b0b6d5e419f37dcccd60354d2
diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index 8ab759d..b14b69b 100644
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -827,7 +827,9 @@
bool CodeGenerator::CanMoveNullCheckToUser(HNullCheck* null_check) {
HInstruction* first_next_not_move = null_check->GetNextDisregardingMoves();
- return (first_next_not_move != nullptr) && first_next_not_move->CanDoImplicitNullCheck();
+
+ return (first_next_not_move != nullptr)
+ && first_next_not_move->CanDoImplicitNullCheckOn(null_check->InputAt(0));
}
void CodeGenerator::MaybeRecordImplicitNullCheck(HInstruction* instr) {
@@ -842,7 +844,7 @@
return;
}
- if (!instr->CanDoImplicitNullCheck()) {
+ if (!instr->CanDoImplicitNullCheckOn(instr->InputAt(0))) {
return;
}
diff --git a/compiler/optimizing/intrinsics_arm.cc b/compiler/optimizing/intrinsics_arm.cc
index 9a6062f..932192e 100644
--- a/compiler/optimizing/intrinsics_arm.cc
+++ b/compiler/optimizing/intrinsics_arm.cc
@@ -863,7 +863,7 @@
LocationSummary* locations = invoke->GetLocations();
// Note that the null check must have been done earlier.
- DCHECK(!invoke->CanDoImplicitNullCheck());
+ DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0)));
Register argument = locations->InAt(1).AsRegister<Register>();
__ cmp(argument, ShifterOperand(0));
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
index d3a4e6c..117d6a4 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -1007,7 +1007,7 @@
LocationSummary* locations = invoke->GetLocations();
// Note that the null check must have been done earlier.
- DCHECK(!invoke->CanDoImplicitNullCheck());
+ DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0)));
Register argument = WRegisterFrom(locations->InAt(1));
__ Cmp(argument, 0);
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 95ab90d..a8e2cdf 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -962,7 +962,7 @@
LocationSummary* locations = invoke->GetLocations();
// Note that the null check must have been done earlier.
- DCHECK(!invoke->CanDoImplicitNullCheck());
+ DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0)));
Register argument = locations->InAt(1).AsRegister<Register>();
__ testl(argument, argument);
diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index d9a1c31..c718ece 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -886,7 +886,7 @@
LocationSummary* locations = invoke->GetLocations();
// Note that the null check must have been done earlier.
- DCHECK(!invoke->CanDoImplicitNullCheck());
+ DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0)));
CpuRegister argument = locations->InAt(1).AsRegister<CpuRegister>();
__ testl(argument, argument);
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 1565f58..08fcdbb 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -1158,7 +1158,10 @@
return true;
}
- virtual bool CanDoImplicitNullCheck() const { return false; }
+ virtual bool CanDoImplicitNullCheckOn(HInstruction* obj) const {
+ UNUSED(obj);
+ return false;
+ }
void SetReferenceTypeInfo(ReferenceTypeInfo reference_type_info) {
DCHECK_EQ(GetType(), Primitive::kPrimNot);
@@ -2225,7 +2228,8 @@
invoke_type_(invoke_type),
is_recursive_(is_recursive) {}
- bool CanDoImplicitNullCheck() const OVERRIDE {
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
+ UNUSED(obj);
// We access the method via the dex cache so we can't do an implicit null check.
// TODO: for intrinsics we can generate implicit null checks.
return false;
@@ -2257,9 +2261,9 @@
: HInvoke(arena, number_of_arguments, return_type, dex_pc, dex_method_index),
vtable_index_(vtable_index) {}
- bool CanDoImplicitNullCheck() const OVERRIDE {
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
// TODO: Add implicit null checks in intrinsics.
- return !GetLocations()->Intrinsified();
+ return (obj == InputAt(0)) && !GetLocations()->Intrinsified();
}
uint32_t GetVTableIndex() const { return vtable_index_; }
@@ -2283,9 +2287,9 @@
: HInvoke(arena, number_of_arguments, return_type, dex_pc, dex_method_index),
imt_index_(imt_index) {}
- bool CanDoImplicitNullCheck() const OVERRIDE {
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
// TODO: Add implicit null checks in intrinsics.
- return !GetLocations()->Intrinsified();
+ return (obj == InputAt(0)) && !GetLocations()->Intrinsified();
}
uint32_t GetImtIndex() const { return imt_index_; }
@@ -2855,8 +2859,8 @@
return GetFieldOffset().SizeValue() == other_get->GetFieldOffset().SizeValue();
}
- bool CanDoImplicitNullCheck() const OVERRIDE {
- return GetFieldOffset().Uint32Value() < kPageSize;
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
+ return (obj == InputAt(0)) && GetFieldOffset().Uint32Value() < kPageSize;
}
size_t ComputeHashCode() const OVERRIDE {
@@ -2889,8 +2893,8 @@
SetRawInputAt(1, value);
}
- bool CanDoImplicitNullCheck() const OVERRIDE {
- return GetFieldOffset().Uint32Value() < kPageSize;
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
+ return (obj == InputAt(0)) && GetFieldOffset().Uint32Value() < kPageSize;
}
const FieldInfo& GetFieldInfo() const { return field_info_; }
@@ -2920,7 +2924,8 @@
UNUSED(other);
return true;
}
- bool CanDoImplicitNullCheck() const OVERRIDE {
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
+ UNUSED(obj);
// TODO: We can be smarter here.
// Currently, the array access is always preceded by an ArrayLength or a NullCheck
// which generates the implicit null check. There are cases when these can be removed
@@ -2962,7 +2967,8 @@
return needs_type_check_;
}
- bool CanDoImplicitNullCheck() const OVERRIDE {
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
+ UNUSED(obj);
// TODO: Same as for ArrayGet.
return false;
}
@@ -3014,7 +3020,9 @@
UNUSED(other);
return true;
}
- bool CanDoImplicitNullCheck() const OVERRIDE { return true; }
+ bool CanDoImplicitNullCheckOn(HInstruction* obj) const OVERRIDE {
+ return obj == InputAt(0);
+ }
DECLARE_INSTRUCTION(ArrayLength);