Update CanTriggerGC flag for ArraySet
ArraySet instructions can trigger a GC only when perforing a type
check. When clearing the needs type check flag, we should also
clear the CanTriggerGC flag from the instruction's side effects.
Note that once we clear the needs type check flag, we never set
it again.
Add check in graph checker that the needs type check and the can
trigger GC flag are consistent.
Add can_trigger_gc property to graph visualizer that reflects
whether the ArraySet instruction can trigger a GC.
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Change-Id: I4c74f902aabf2339bd292e9b24737f55d2737440
diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index e4203fe..3d80de0 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -1064,6 +1064,21 @@
}
}
+void GraphChecker::VisitArraySet(HArraySet* instruction) {
+ VisitInstruction(instruction);
+
+ if (instruction->NeedsTypeCheck() !=
+ instruction->GetSideEffects().Includes(SideEffects::CanTriggerGC())) {
+ AddError(StringPrintf(
+ "%s %d has a flag mismatch. An ArraySet instruction can trigger a GC iff it "
+ "needs a type check. Needs type check: %s, Can trigger GC: %s",
+ instruction->DebugName(),
+ instruction->GetId(),
+ instruction->NeedsTypeCheck() ? "true" : "false",
+ instruction->GetSideEffects().Includes(SideEffects::CanTriggerGC()) ? "true" : "false"));
+ }
+}
+
void GraphChecker::VisitBinaryOperation(HBinaryOperation* op) {
VisitInstruction(op);
DataType::Type lhs_type = op->InputAt(0)->GetType();
diff --git a/compiler/optimizing/graph_checker.h b/compiler/optimizing/graph_checker.h
index c3cdba5..950868b 100644
--- a/compiler/optimizing/graph_checker.h
+++ b/compiler/optimizing/graph_checker.h
@@ -55,6 +55,7 @@
void VisitInstruction(HInstruction* instruction) override;
void VisitPhi(HPhi* phi) override;
+ void VisitArraySet(HArraySet* instruction) override;
void VisitBinaryOperation(HBinaryOperation* op) override;
void VisitBooleanNot(HBooleanNot* instruction) override;
void VisitBoundType(HBoundType* instruction) override;
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 646c34e..f607652 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -480,6 +480,9 @@
<< array_set->GetValueCanBeNull() << std::noboolalpha;
StartAttributeStream("needs_type_check") << std::boolalpha
<< array_set->NeedsTypeCheck() << std::noboolalpha;
+ StartAttributeStream("can_trigger_gc")
+ << std::boolalpha << array_set->GetSideEffects().Includes(SideEffects::CanTriggerGC())
+ << std::noboolalpha;
}
void VisitCompare(HCompare* compare) override {
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index ac212e0..82c1f6d 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -1142,13 +1142,13 @@
if (value->IsArrayGet()) {
if (value->AsArrayGet()->GetArray() == instruction->GetArray()) {
// If the code is just swapping elements in the array, no need for a type check.
- instruction->ClearNeedsTypeCheck();
+ instruction->ClearTypeCheck();
return;
}
}
if (value->IsNullConstant()) {
- instruction->ClearNeedsTypeCheck();
+ instruction->ClearTypeCheck();
return;
}
@@ -1160,13 +1160,13 @@
}
if (value_rti.IsValid() && array_rti.CanArrayHold(value_rti)) {
- instruction->ClearNeedsTypeCheck();
+ instruction->ClearTypeCheck();
return;
}
if (array_rti.IsObjectArray()) {
if (array_rti.IsExact()) {
- instruction->ClearNeedsTypeCheck();
+ instruction->ClearTypeCheck();
return;
}
instruction->SetStaticTypeOfArrayIsObjectArray();
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 17ede39..006ccfb 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -6597,8 +6597,10 @@
return false;
}
- void ClearNeedsTypeCheck() {
+ void ClearTypeCheck() {
SetPackedFlag<kFlagNeedsTypeCheck>(false);
+ // Clear the `CanTriggerGC` flag too as we can only trigger a GC when doing a type check.
+ SetSideEffects(GetSideEffects().Exclusion(SideEffects::CanTriggerGC()));
}
void ClearValueCanBeNull() {
diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index 5ce6328..707783e 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -109,7 +109,7 @@
if (value->IsNullConstant()) {
DCHECK_EQ(value->GetType(), DataType::Type::kReference);
if (instruction->NeedsTypeCheck()) {
- instruction->ClearNeedsTypeCheck();
+ instruction->ClearTypeCheck();
}
}
}
diff --git a/test/521-checker-array-set-null/src/Main.java b/test/521-checker-array-set-null/src/Main.java
index f166b92..12a1985 100644
--- a/test/521-checker-array-set-null/src/Main.java
+++ b/test/521-checker-array-set-null/src/Main.java
@@ -16,26 +16,64 @@
public class Main {
public static void main(String[] args) {
- testWithNull(new Object[2]);
- testWithUnknown(new Object[2], new Object());
- testWithSame(new Object[2]);
+ $noinline$testWithNull(new Object[2]);
+ $noinline$testWithUnknown(new Object[2], new Object());
+ $noinline$testWithSame(new Object[2]);
+ $noinline$testWithSameRTI();
}
- /// CHECK-START: void Main.testWithNull(java.lang.Object[]) disassembly (after)
- /// CHECK: ArraySet needs_type_check:false
- public static void testWithNull(Object[] o) {
+ // Known null can eliminate the type check in early stages.
+
+ /// CHECK-START: void Main.$noinline$testWithNull(java.lang.Object[]) instruction_simplifier (before)
+ /// CHECK: ArraySet needs_type_check:true can_trigger_gc:true
+
+ /// CHECK-START: void Main.$noinline$testWithNull(java.lang.Object[]) instruction_simplifier (after)
+ /// CHECK: ArraySet needs_type_check:false can_trigger_gc:false
+
+ /// CHECK-START: void Main.$noinline$testWithNull(java.lang.Object[]) disassembly (after)
+ /// CHECK: ArraySet needs_type_check:false can_trigger_gc:false
+ public static void $noinline$testWithNull(Object[] o) {
o[0] = null;
}
- /// CHECK-START: void Main.testWithUnknown(java.lang.Object[], java.lang.Object) disassembly (after)
- /// CHECK: ArraySet needs_type_check:true
- public static void testWithUnknown(Object[] o, Object obj) {
+ /// CHECK-START: void Main.$noinline$testWithUnknown(java.lang.Object[], java.lang.Object) disassembly (after)
+ /// CHECK: ArraySet needs_type_check:true can_trigger_gc:true
+ public static void $noinline$testWithUnknown(Object[] o, Object obj) {
o[0] = obj;
}
- /// CHECK-START: void Main.testWithSame(java.lang.Object[]) disassembly (after)
- /// CHECK: ArraySet needs_type_check:false
- public static void testWithSame(Object[] o) {
+ // After GVN we know that we are setting values from the same array so there's no need for a type
+ // check.
+
+ /// CHECK-START: void Main.$noinline$testWithSame(java.lang.Object[]) instruction_simplifier$after_gvn (before)
+ /// CHECK: ArraySet needs_type_check:true can_trigger_gc:true
+
+ /// CHECK-START: void Main.$noinline$testWithSame(java.lang.Object[]) instruction_simplifier$after_gvn (after)
+ /// CHECK: ArraySet needs_type_check:false can_trigger_gc:false
+
+ /// CHECK-START: void Main.$noinline$testWithSame(java.lang.Object[]) disassembly (after)
+ /// CHECK: ArraySet needs_type_check:false can_trigger_gc:false
+ public static void $noinline$testWithSame(Object[] o) {
o[0] = o[1];
}
+
+ // We know that the array and the static Object have the same RTI in early stages. No need for a
+ // type check.
+
+ /// CHECK-START: java.lang.Object[] Main.$noinline$testWithSameRTI() instruction_simplifier (before)
+ /// CHECK: ArraySet needs_type_check:true can_trigger_gc:true
+
+ /// CHECK-START: java.lang.Object[] Main.$noinline$testWithSameRTI() instruction_simplifier (after)
+ /// CHECK: ArraySet needs_type_check:false can_trigger_gc:false
+
+ /// CHECK-START: java.lang.Object[] Main.$noinline$testWithSameRTI() disassembly (after)
+ /// CHECK: ArraySet needs_type_check:false can_trigger_gc:false
+ public static Object[] $noinline$testWithSameRTI() {
+ Object[] arr = new Object[1];
+ arr[0] = static_obj;
+ // Return so that LSE doesn't eliminate the ArraySet.
+ return arr;
+ }
+
+ static Object static_obj;
}
diff --git a/test/527-checker-array-access-split/src/Main.java b/test/527-checker-array-access-split/src/Main.java
index f39b5e2..cc1dc6b 100644
--- a/test/527-checker-array-access-split/src/Main.java
+++ b/test/527-checker-array-access-split/src/Main.java
@@ -593,12 +593,16 @@
/// CHECK: ArrayGet [<<IntAddr1>>,{{i\d+}}]
/// CHECK: <<IntAddr2:i\d+>> IntermediateAddress [<<Array>>,<<DataOffset>>]
/// CHECK: ArrayGet [<<IntAddr2>>,{{i\d+}}]
- /// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}]
+ /// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}] needs_type_check:false can_trigger_gc:false
/// CHECK: <<IntAddr3:i\d+>> IntermediateAddress [<<Array>>,<<DataOffset>>]
/// CHECK: ArrayGet [<<IntAddr3>>,{{i\d+}}]
/// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}]
/// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}]
//
+ /// CHECK-START-ARM64: int Main.checkObjectArrayGet(int, java.lang.Integer[], java.lang.Integer[]) instruction_simplifier_arm64 (after)
+ /// CHECK: IntermediateAddress
+ /// CHECK: IntermediateAddress
+ /// CHECK: IntermediateAddress
/// CHECK-NOT: IntermediateAddress
/// CHECK-START-ARM64: int Main.checkObjectArrayGet(int, java.lang.Integer[], java.lang.Integer[]) GVN$after_arch (after)
@@ -608,12 +612,13 @@
/// CHECK: <<IntAddr1:i\d+>> IntermediateAddress [<<Array>>,<<DataOffset>>]
/// CHECK: ArrayGet [<<IntAddr1>>,{{i\d+}}]
/// CHECK: ArrayGet [<<IntAddr1>>,{{i\d+}}]
- /// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}]
- /// CHECK: <<IntAddr3:i\d+>> IntermediateAddress [<<Array>>,<<DataOffset>>]
- /// CHECK: ArrayGet [<<IntAddr3>>,{{i\d+}}]
+ /// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}] needs_type_check:false can_trigger_gc:false
+ /// CHECK: ArrayGet [<<IntAddr1>>,{{i\d+}}]
/// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}]
/// CHECK: ArraySet [<<Array>>,{{i\d+}},{{l\d+}}]
//
+ /// CHECK-START-ARM64: int Main.checkObjectArrayGet(int, java.lang.Integer[], java.lang.Integer[]) GVN$after_arch (after)
+ /// CHECK: IntermediateAddress
/// CHECK-NOT: IntermediateAddress
public final static int checkObjectArrayGet(int index, Integer[] a, Integer[] b) {
Integer five = Integer.valueOf(5);
diff --git a/test/590-checker-arr-set-null-regression/src/Main.java b/test/590-checker-arr-set-null-regression/src/Main.java
index 792ee4e..ad47716 100644
--- a/test/590-checker-arr-set-null-regression/src/Main.java
+++ b/test/590-checker-arr-set-null-regression/src/Main.java
@@ -33,7 +33,7 @@
/// CHECK-DAG: <<CheckedArray:l\d+>> NullCheck [<<Array>>]
/// CHECK-DAG: <<Length:i\d+>> ArrayLength [<<CheckedArray>>]
/// CHECK-DAG: <<CheckedIndex:i\d+>> BoundsCheck [<<Index>>,<<Length>>]
- /// CHECK-DAG: <<ArraySet:v\d+>> ArraySet [<<CheckedArray>>,<<CheckedIndex>>,<<CheckedValue>>] needs_type_check:true
+ /// CHECK-DAG: <<ArraySet:v\d+>> ArraySet [<<CheckedArray>>,<<CheckedIndex>>,<<CheckedValue>>] needs_type_check:true can_trigger_gc:true
/// CHECK-START: void Main.testArraySetCheckCastNull(Main$Element[]) instruction_simplifier (after)
/// CHECK-NOT: CheckCast
@@ -47,7 +47,7 @@
/// CHECK-DAG: <<CheckedArray:l\d+>> NullCheck [<<Array>>]
/// CHECK-DAG: <<Length:i\d+>> ArrayLength [<<CheckedArray>>]
/// CHECK-DAG: <<CheckedIndex:i\d+>> BoundsCheck [<<Index>>,<<Length>>]
- /// CHECK-DAG: <<ArraySet:v\d+>> ArraySet [<<CheckedArray>>,<<CheckedIndex>>,<<CheckedValue>>] needs_type_check:true
+ /// CHECK-DAG: <<ArraySet:v\d+>> ArraySet [<<CheckedArray>>,<<CheckedIndex>>,<<CheckedValue>>] needs_type_check:true can_trigger_gc:true
/// CHECK-START: void Main.testArraySetCheckCastNull(Main$Element[]) prepare_for_register_allocation (after)
/// CHECK: <<Array:l\d+>> ParameterValue
@@ -55,7 +55,7 @@
/// CHECK-DAG: <<Null:l\d+>> NullConstant
/// CHECK-DAG: <<Class:l\d+>> LoadClass
/// CHECK-DAG: <<Length:i\d+>> ArrayLength [<<Array>>]
- /// CHECK-DAG: <<ArraySet:v\d+>> ArraySet [<<Array>>,<<Index>>,<<Null>>] needs_type_check:false
+ /// CHECK-DAG: <<ArraySet:v\d+>> ArraySet [<<Array>>,<<Index>>,<<Null>>] needs_type_check:false can_trigger_gc:false
static void testArraySetCheckCastNull(Element[] elements) {
Object object = null;