summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--compiler/optimizing/graph_checker.cc15
-rw-r--r--compiler/optimizing/graph_checker.h1
-rw-r--r--compiler/optimizing/graph_visualizer.cc3
-rw-r--r--compiler/optimizing/instruction_simplifier.cc8
-rw-r--r--compiler/optimizing/nodes.h4
-rw-r--r--compiler/optimizing/prepare_for_register_allocation.cc2
-rw-r--r--test/521-checker-array-set-null/src/Main.java62
-rw-r--r--test/527-checker-array-access-split/src/Main.java13
-rw-r--r--test/590-checker-arr-set-null-regression/src/Main.java6
9 files changed, 89 insertions, 25 deletions
diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index e4203fe492..3d80de0e04 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -1064,6 +1064,21 @@ void GraphChecker::VisitNeg(HNeg* instruction) {
}
}
+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 c3cdba5d42..950868b4ba 100644
--- a/compiler/optimizing/graph_checker.h
+++ b/compiler/optimizing/graph_checker.h
@@ -55,6 +55,7 @@ class GraphChecker : public HGraphDelegateVisitor {
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 646c34e384..f6076525d8 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -480,6 +480,9 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor {
<< 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 ac212e07fe..82c1f6d3ff 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -1142,13 +1142,13 @@ void InstructionSimplifierVisitor::VisitArraySet(HArraySet* instruction) {
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 @@ void InstructionSimplifierVisitor::VisitArraySet(HArraySet* instruction) {
}
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 17ede39d1e..006ccfb8ec 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -6597,8 +6597,10 @@ class HArraySet final : public HExpression<3> {
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 5ce63284ae..707783e47b 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -109,7 +109,7 @@ void PrepareForRegisterAllocation::VisitArraySet(HArraySet* instruction) {
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 f166b92fa4..12a1985bfa 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 f39b5e26a0..cc1dc6b8fc 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 @@ public class Main {
/// 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 @@ public class Main {
/// 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 792ee4ecd6..ad47716c40 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 @@ public class Main {
/// 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 @@ public class Main {
/// 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 @@ public class Main {
/// 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;