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

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;