diff options
| -rw-r--r-- | compiler/optimizing/code_generator.cc | 7 | ||||
| -rw-r--r-- | compiler/optimizing/nodes.h | 46 | ||||
| -rw-r--r-- | compiler/optimizing/side_effects_test.cc | 1 | ||||
| -rw-r--r-- | test/510-checker-try-catch/smali/Runtime.smali | 70 | ||||
| -rw-r--r-- | test/510-checker-try-catch/src/Main.java | 119 | ||||
| -rw-r--r-- | test/527-checker-array-access-split/src/Main.java | 27 | ||||
| -rw-r--r-- | test/706-checker-scheduler/src/Main.java | 8 |
7 files changed, 265 insertions, 13 deletions
diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index a90ff3f885..e84896b113 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -1489,7 +1489,12 @@ void CodeGenerator::ValidateInvokeRuntime(QuickEntrypointEnum entrypoint, << " instruction->GetSideEffects().ToString()=" << instruction->GetSideEffects().ToString(); } else { - DCHECK(instruction->GetSideEffects().Includes(SideEffects::CanTriggerGC()) || + // 'CanTriggerGC' side effect is used to restrict optimization of instructions which depend + // on GC (e.g. IntermediateAddress) - to ensure they are not alive across GC points. However + // if execution never returns to the compiled code from a GC point this restriction is + // unnecessary - in particular for fatal slow paths which might trigger GC. + DCHECK((slow_path->IsFatal() && !instruction->GetLocations()->WillCall()) || + instruction->GetSideEffects().Includes(SideEffects::CanTriggerGC()) || // When (non-Baker) read barriers are enabled, some instructions // use a slow path to emit a read barrier, which does not trigger // GC. diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 5feffa0511..68f1a2406a 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -1626,6 +1626,21 @@ using HConstInputsRef = TransformArrayRef<const HUserRecord<HInstruction*>, HInp * the same, and any reference read depends on any reference read without * further regard of its type). * + * kDependsOnGCBit is defined in the following way: instructions with kDependsOnGCBit must not be + * alive across the point where garbage collection might happen. + * + * Note: Instructions with kCanTriggerGCBit do not depend on each other. + * + * kCanTriggerGCBit must be used for instructions for which GC might happen on the path across + * those instructions from the compiler perspective (between this instruction and the next one + * in the IR). + * + * Note: Instructions which can cause GC only on a fatal slow path do not need + * kCanTriggerGCBit as the execution never returns to the instruction next to the exceptional + * one. However the execution may return to compiled code if there is a catch block in the + * current method; for this purpose the TryBoundary exit instruction has kCanTriggerGCBit + * set. + * * The internal representation uses 38-bit and is described in the table below. * The first line indicates the side effect, and for field/array accesses the * second line indicates the type of the access (in the order of the @@ -1698,10 +1713,17 @@ class SideEffects : public ValueObject { return SideEffects(TypeFlag(type, kArrayReadOffset)); } + // Returns whether GC might happen across this instruction from the compiler perspective so + // the next instruction in the IR would see that. + // + // See the SideEffect class comments. static SideEffects CanTriggerGC() { return SideEffects(1ULL << kCanTriggerGCBit); } + // Returns whether the instruction must not be alive across a GC point. + // + // See the SideEffect class comments. static SideEffects DependsOnGC() { return SideEffects(1ULL << kDependsOnGCBit); } @@ -3136,8 +3158,15 @@ class HTryBoundary final : public HExpression<0> { kLast = kExit }; + // SideEffects::CanTriggerGC prevents instructions with SideEffects::DependOnGC to be alive + // across the catch block entering edges as GC might happen during throwing an exception. + // TryBoundary with BoundaryKind::kExit is conservatively used for that as there is no + // HInstruction which a catch block must start from. explicit HTryBoundary(BoundaryKind kind, uint32_t dex_pc = kNoDexPc) - : HExpression(kTryBoundary, SideEffects::None(), dex_pc) { + : HExpression(kTryBoundary, + (kind == BoundaryKind::kExit) ? SideEffects::CanTriggerGC() + : SideEffects::None(), + dex_pc) { SetPackedField<BoundaryKindField>(kind); } @@ -5163,9 +5192,10 @@ class HAbs final : public HUnaryOperation { class HDivZeroCheck final : public HExpression<1> { public: // `HDivZeroCheck` can trigger GC, as it may call the `ArithmeticException` - // constructor. + // constructor. However it can only do it on a fatal slow path so execution never returns to the + // instruction following the current one; thus 'SideEffects::None()' is used. HDivZeroCheck(HInstruction* value, uint32_t dex_pc) - : HExpression(kDivZeroCheck, value->GetType(), SideEffects::CanTriggerGC(), dex_pc) { + : HExpression(kDivZeroCheck, value->GetType(), SideEffects::None(), dex_pc) { SetRawInputAt(0, value); } @@ -5642,9 +5672,10 @@ static constexpr uint32_t kNoRegNumber = -1; class HNullCheck final : public HExpression<1> { public: // `HNullCheck` can trigger GC, as it may call the `NullPointerException` - // constructor. + // constructor. However it can only do it on a fatal slow path so execution never returns to the + // instruction following the current one; thus 'SideEffects::None()' is used. HNullCheck(HInstruction* value, uint32_t dex_pc) - : HExpression(kNullCheck, value->GetType(), SideEffects::CanTriggerGC(), dex_pc) { + : HExpression(kNullCheck, value->GetType(), SideEffects::None(), dex_pc) { SetRawInputAt(0, value); } @@ -6071,12 +6102,13 @@ class HArrayLength final : public HExpression<1> { class HBoundsCheck final : public HExpression<2> { public: // `HBoundsCheck` can trigger GC, as it may call the `IndexOutOfBoundsException` - // constructor. + // constructor. However it can only do it on a fatal slow path so execution never returns to the + // instruction following the current one; thus 'SideEffects::None()' is used. HBoundsCheck(HInstruction* index, HInstruction* length, uint32_t dex_pc, bool is_string_char_at = false) - : HExpression(kBoundsCheck, index->GetType(), SideEffects::CanTriggerGC(), dex_pc) { + : HExpression(kBoundsCheck, index->GetType(), SideEffects::None(), dex_pc) { DCHECK_EQ(DataType::Type::kInt32, DataType::Kind(index->GetType())); SetPackedFlag<kFlagIsStringCharAt>(is_string_char_at); SetRawInputAt(0, index); diff --git a/compiler/optimizing/side_effects_test.cc b/compiler/optimizing/side_effects_test.cc index 97317124ef..4b0be07f3b 100644 --- a/compiler/optimizing/side_effects_test.cc +++ b/compiler/optimizing/side_effects_test.cc @@ -202,6 +202,7 @@ TEST(SideEffectsTest, GC) { EXPECT_TRUE(depends_on_gc.MayDependOn(all_changes)); EXPECT_TRUE(depends_on_gc.Union(can_trigger_gc).MayDependOn(all_changes)); EXPECT_FALSE(can_trigger_gc.MayDependOn(all_changes)); + EXPECT_FALSE(can_trigger_gc.MayDependOn(can_trigger_gc)); EXPECT_TRUE(all_changes.Includes(can_trigger_gc)); EXPECT_FALSE(all_changes.Includes(depends_on_gc)); diff --git a/test/510-checker-try-catch/smali/Runtime.smali b/test/510-checker-try-catch/smali/Runtime.smali index 19b43a3f30..d080a0c94b 100644 --- a/test/510-checker-try-catch/smali/Runtime.smali +++ b/test/510-checker-try-catch/smali/Runtime.smali @@ -549,6 +549,76 @@ .end array-data .end method + +## CHECK-START-{ARM,ARM64}: int Runtime.testIntAddressCatch(int, int[]) GVN$after_arch (after) +## CHECK-DAG: <<Const1:i\d+>> IntConstant 1 +## CHECK-DAG: <<Offset:i\d+>> IntConstant 12 +## CHECK-DAG: <<IndexParam:i\d+>> ParameterValue +## CHECK-DAG: <<Array:l\d+>> ParameterValue + +## CHECK-DAG: <<NullCh1:l\d+>> NullCheck [<<Array>>] +## CHECK-DAG: <<Length:i\d+>> ArrayLength +## CHECK-DAG: <<BoundsCh1:i\d+>> BoundsCheck [<<IndexParam>>,<<Length>>] +## CHECK-DAG: <<IntAddr1:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] +## CHECK-DAG: ArrayGet [<<IntAddr1>>,<<BoundsCh1>>] +## CHECK-DAG: TryBoundary + +## CHECK-DAG: <<Xplus1:i\d+>> Add [<<IndexParam>>,<<Const1>>] +## CHECK-DAG: <<BoundsCh2:i\d+>> BoundsCheck [<<Xplus1>>,<<Length>>] +## CHECK-DAG: ArrayGet [<<IntAddr1>>,<<BoundsCh2>>] +## CHECK-DAG: TryBoundary + +## CHECK-DAG: <<Phi:i\d+>> Phi [<<Xplus1>>] +## CHECK-DAG: <<Phiplus1:i\d+>> Add [<<Phi>>,<<Const1>>] +## CHECK-DAG: <<BoundsCh3:i\d+>> BoundsCheck [<<Phiplus1>>,<<Length>>] +## CHECK-DAG: <<IntAddr3:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] +## CHECK-DAG: ArrayGet [<<IntAddr3>>,<<BoundsCh3>>] + +## CHECK-START-{ARM,ARM64}: int Runtime.testIntAddressCatch(int, int[]) GVN$after_arch (after) +## CHECK: NullCheck +## CHECK-NOT: NullCheck + +## CHECK-START-{ARM,ARM64}: int Runtime.testIntAddressCatch(int, int[]) GVN$after_arch (after) +## CHECK: IntermediateAddress +## CHECK: IntermediateAddress +## CHECK-NOT: IntermediateAddress + +## CHECK-START-{ARM,ARM64}: int Runtime.testIntAddressCatch(int, int[]) GVN$after_arch (after) +## CHECK: BoundsCheck +## CHECK: BoundsCheck +## CHECK: BoundsCheck +## CHECK-NOT: BoundsCheck + +## CHECK-START-{ARM,ARM64}: int Runtime.testIntAddressCatch(int, int[]) GVN$after_arch (after) +## CHECK: ArrayGet +## CHECK: ArrayGet +## CHECK: ArrayGet +## CHECK-NOT: ArrayGet +.method public static testIntAddressCatch(I[I)I + .registers 4 + aget v0, p1, p0 + add-int v1, v0, v0 + + :try_start + const/4 v0, 0x1 + add-int p0, p0, v0 + aget v0, p1, p0 + + :try_end + .catch Ljava/lang/ArithmException; {:try_start .. :try_end} :catch_block + + :return + add-int v1, v1, v0 + return v1 + + :catch_block + const/4 v0, 0x1 + add-int p0, p0, v0 + aget v0, p1, p0 + + goto :return +.end method + .field public static intArray:[I .field public static longArray:[J .field public static floatArray:[F diff --git a/test/510-checker-try-catch/src/Main.java b/test/510-checker-try-catch/src/Main.java index d6dcd30f3c..18658cd5e0 100644 --- a/test/510-checker-try-catch/src/Main.java +++ b/test/510-checker-try-catch/src/Main.java @@ -37,6 +37,114 @@ public class Main { public int expected; } + // Test that IntermediateAddress instruction is not alive across BoundsCheck which can throw to + // a catch block. + // + /// CHECK-START-{ARM,ARM64}: void Main.boundsCheckAndCatch(int, int[], int[]) GVN$after_arch (before) + /// CHECK-DAG: <<Const0:i\d+>> IntConstant 0 + /// CHECK-DAG: <<Const1:i\d+>> IntConstant 1 + /// CHECK-DAG: <<Const2:i\d+>> IntConstant 2 + /// CHECK-DAG: <<Offset:i\d+>> IntConstant 12 + /// CHECK-DAG: <<IndexParam:i\d+>> ParameterValue + /// CHECK-DAG: <<ArrayA:l\d+>> ParameterValue + /// CHECK-DAG: <<ArrayB:l\d+>> ParameterValue + // + + /// CHECK-DAG: <<NullCh1:l\d+>> NullCheck [<<ArrayA>>] + /// CHECK-DAG: <<LengthA:i\d+>> ArrayLength + /// CHECK-DAG: <<BoundsCh1:i\d+>> BoundsCheck [<<IndexParam>>,<<LengthA>>] + /// CHECK-DAG: <<IntAddr1:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] + /// CHECK-DAG: ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const1>>] + /// CHECK-DAG: TryBoundary + // + /// CHECK-DAG: <<IntAddr2:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] + /// CHECK-DAG: ArraySet [<<IntAddr2>>,<<BoundsCh1>>,<<Const2>>] + /// CHECK-DAG: <<NullChB:l\d+>> NullCheck [<<ArrayB>>] + /// CHECK-DAG: <<LengthB:i\d+>> ArrayLength + /// CHECK-DAG: <<BoundsChB:i\d+>> BoundsCheck [<<Const0>>,<<LengthB>>] + /// CHECK-DAG: <<GetB:i\d+>> ArrayGet [<<NullChB>>,<<BoundsChB>>] + /// CHECK-DAG: <<ZeroCheck:i\d+>> DivZeroCheck [<<IndexParam>>] + /// CHECK-DAG: <<Div:i\d+>> Div [<<GetB>>,<<ZeroCheck>>] + /// CHECK-DAG: <<Xplus1:i\d+>> Add [<<IndexParam>>,<<Const1>>] + /// CHECK-DAG: <<BoundsCh2:i\d+>> BoundsCheck [<<Xplus1>>,<<LengthA>>] + /// CHECK-DAG: <<IntAddr3:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] + /// CHECK-DAG: ArraySet [<<IntAddr3>>,<<BoundsCh2>>,<<Div>>] + /// CHECK-DAG: TryBoundary + // + /// CHECK-DAG: ClearException + /// CHECK-DAG: <<IntAddr4:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] + /// CHECK-DAG: ArraySet [<<IntAddr4>>,<<BoundsCh1>>,<<Const1>>] + // + /// CHECK-NOT: NullCheck + /// CHECK-NOT: IntermediateAddress + + /// CHECK-START-{ARM,ARM64}: void Main.boundsCheckAndCatch(int, int[], int[]) GVN$after_arch (after) + /// CHECK-DAG: <<Const0:i\d+>> IntConstant 0 + /// CHECK-DAG: <<Const1:i\d+>> IntConstant 1 + /// CHECK-DAG: <<Const2:i\d+>> IntConstant 2 + /// CHECK-DAG: <<Offset:i\d+>> IntConstant 12 + /// CHECK-DAG: <<IndexParam:i\d+>> ParameterValue + /// CHECK-DAG: <<ArrayA:l\d+>> ParameterValue + /// CHECK-DAG: <<ArrayB:l\d+>> ParameterValue + // + /// CHECK-DAG: <<NullCh1:l\d+>> NullCheck [<<ArrayA>>] + /// CHECK-DAG: <<LengthA:i\d+>> ArrayLength + /// CHECK-DAG: <<BoundsCh1:i\d+>> BoundsCheck [<<IndexParam>>,<<LengthA>>] + /// CHECK-DAG: <<IntAddr1:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] + /// CHECK-DAG: ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const1>>] + /// CHECK-DAG: TryBoundary + // + /// CHECK-DAG: ArraySet [<<IntAddr1>>,<<BoundsCh1>>,<<Const2>>] + /// CHECK-DAG: <<NullChB:l\d+>> NullCheck [<<ArrayB>>] + /// CHECK-DAG: <<LengthB:i\d+>> ArrayLength + /// CHECK-DAG: <<BoundsChB:i\d+>> BoundsCheck [<<Const0>>,<<LengthB>>] + /// CHECK-DAG: <<GetB:i\d+>> ArrayGet [<<NullChB>>,<<BoundsChB>>] + /// CHECK-DAG: <<ZeroCheck:i\d+>> DivZeroCheck [<<IndexParam>>] + /// CHECK-DAG: <<Div:i\d+>> Div [<<GetB>>,<<ZeroCheck>>] + /// CHECK-DAG: <<Xplus1:i\d+>> Add [<<IndexParam>>,<<Const1>>] + /// CHECK-DAG: <<BoundsCh2:i\d+>> BoundsCheck [<<Xplus1>>,<<LengthA>>] + /// CHECK-DAG: ArraySet [<<IntAddr1>>,<<BoundsCh2>>,<<Div>>] + /// CHECK-DAG: TryBoundary + // + /// CHECK-DAG: ClearException + /// CHECK-DAG: <<IntAddr4:i\d+>> IntermediateAddress [<<NullCh1>>,<<Offset>>] + /// CHECK-DAG: ArraySet [<<IntAddr4>>,<<BoundsCh1>>,<<Const1>>] + // + /// CHECK-NOT: NullCheck + /// CHECK-NOT: IntermediateAddress + + // Make sure that BoundsCheck, DivZeroCheck and NullCheck don't stop IntermediateAddress sharing. + public static void boundsCheckAndCatch(int x, int[] a, int[] b) { + a[x] = 1; + try { + a[x] = 2; + a[x + 1] = b[0] / x; + } catch (Exception e) { + a[x] = 1; + } + } + + private static void expectEquals(int expected, int result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } + + public final static int ARRAY_SIZE = 128; + + public static void testBoundsCheckAndCatch() { + int[] a = new int[ARRAY_SIZE]; + int[] b = new int[ARRAY_SIZE]; + + int index = ARRAY_SIZE - 2; + boundsCheckAndCatch(index, a, b); + expectEquals(2, a[index]); + + index = ARRAY_SIZE - 1; + boundsCheckAndCatch(index, a, b); + expectEquals(1, a[index]); + } + public static void testMethod(String method) throws Exception { Class<?> c = Class.forName("Runtime"); Method m = c.getMethod(method, boolean.class, boolean.class); @@ -52,6 +160,14 @@ public class Main { } } + public static void testIntAddressCatch() throws Exception { + int[] a = new int[3]; + + Class<?> c = Class.forName("Runtime"); + Method m = c.getMethod("testIntAddressCatch", int.class, Class.forName("[I")); + m.invoke(null, 0, a); + } + public static void main(String[] args) throws Exception { testMethod("testUseAfterCatch_int"); testMethod("testUseAfterCatch_long"); @@ -64,5 +180,8 @@ public class Main { testMethod("testCatchPhi_double"); testMethod("testCatchPhi_singleSlot"); testMethod("testCatchPhi_doubleSlot"); + + testBoundsCheckAndCatch(); + testIntAddressCatch(); } } diff --git a/test/527-checker-array-access-split/src/Main.java b/test/527-checker-array-access-split/src/Main.java index 935b37858d..f83c924de9 100644 --- a/test/527-checker-array-access-split/src/Main.java +++ b/test/527-checker-array-access-split/src/Main.java @@ -552,6 +552,28 @@ public class Main { return (int)s; } + // + // Check that IntermediateAddress can be shared across BoundsCheck, DivZeroCheck and NullCheck - + // instruction which have fatal slow paths. + // + /// CHECK-START-{ARM,ARM64}: void Main.checkGVNForFatalChecks(int, int, char[], int[]) GVN$after_arch (before) + /// CHECK: IntermediateAddress + /// CHECK: IntermediateAddress + // + /// CHECK-NOT: IntermediateAddress + + /// CHECK-START-{ARM,ARM64}: void Main.checkGVNForFatalChecks(int, int, char[], int[]) GVN$after_arch (after) + /// CHECK: IntermediateAddress + // + /// CHECK-NOT: IntermediateAddress + public final static void checkGVNForFatalChecks(int begin, int end, char[] buf1, int[] buf2) { + buf1[begin] = 'a'; + buf2[0] = begin / end; + buf1[end] = 'n'; + } + + public final static int ARRAY_SIZE = 128; + public static void main(String[] args) { int[] array = {123, 456, 789}; @@ -575,5 +597,10 @@ public class Main { assertIntEquals(2097152, canMergeAfterBCE2()); assertIntEquals(18, checkLongFloatDouble()); + + char[] c1 = new char[ARRAY_SIZE]; + int[] i1 = new int[ARRAY_SIZE]; + checkGVNForFatalChecks(1, 2, c1, i1); + assertIntEquals('n', c1[2]); } } diff --git a/test/706-checker-scheduler/src/Main.java b/test/706-checker-scheduler/src/Main.java index 9f4caecccd..af18193388 100644 --- a/test/706-checker-scheduler/src/Main.java +++ b/test/706-checker-scheduler/src/Main.java @@ -292,8 +292,7 @@ public class Main { /// CHECK: <<ArraySet1:v\d+>> ArraySet [<<Addr1>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: <<ArrayGet2:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: Add loop:<<Loop>> outer_loop:none - /// CHECK: <<Addr2:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none - /// CHECK: <<ArraySet2:v\d+>> ArraySet [<<Addr2>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArraySet2:v\d+>> ArraySet [<<Addr1>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: Add loop:<<Loop>> outer_loop:none /// CHECK-START-ARM64: void Main.CrossOverLoop(int[], int[]) scheduler (after) @@ -303,13 +302,12 @@ public class Main { /// CHECK: <<NullA:l\d+>> NullCheck [<<ParamA>>] loop:none /// CHECK: Phi loop:<<Loop:B\d+>> outer_loop:none /// CHECK: <<ArrayGet1:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none - /// CHECK: Add loop:<<Loop>> outer_loop:none /// CHECK: <<Addr1:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add [<<ArrayGet1>>,{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: <<ArraySet1:v\d+>> ArraySet [<<Addr1>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: <<ArrayGet2:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: Add loop:<<Loop>> outer_loop:none - /// CHECK: <<Addr2:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none - /// CHECK: <<ArraySet2:v\d+>> ArraySet [<<Addr2>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArraySet2:v\d+>> ArraySet [<<Addr1>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none /// CHECK: Add loop:<<Loop>> outer_loop:none private static void CrossOverLoop(int a[], int b[]) { b[20] = 99; |