[JDK-8337983] Always insert null check before is array check.

PullRequest: graal/18560
oracle · Aug 13, 2024 · 1ff744c · 1ff744c
2 parents 5a6081a + c3536d7
commit 1ff744c
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 16 deletions.
diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/GraalOSRTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/GraalOSRTest.java
@@ -290,4 +290,28 @@ static ReturnValue testArrayBottom(byte[] aB, int[] aI, int i) {
     public void testOSR07() {
         testOSR(getInitialOptions(), "testArrayBottom", null, new byte[100], new int[100], 10);
     }
+
+    static ReturnValue testNonNullArrayBottom(int i) {
+        Object a;
+        long base;
+        if (i % 2 == 0) {
+            a = new byte[100];
+            base = Unsafe.ARRAY_BYTE_BASE_OFFSET;
+        } else {
+            a = new int[100];
+            base = Unsafe.ARRAY_INT_BASE_OFFSET;
+        }
+        int res = 0;
+        for (int j = 0; j < 10000; j++) {
+            res += UNSAFE.getByte(a, base + j);
+        }
+        GraalDirectives.sideEffect(res);
+
+        return ReturnValue.SUCCESS;
+    }
+
+    @Test
+    public void testOSR08() {
+        testOSR(getInitialOptions(), "testNonNullArrayBottom", null, 10);
+    }
 }
diff --git a/...src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/phases/OnStackReplacementPhase.java b/...src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/phases/OnStackReplacementPhase.java
@@ -311,22 +311,17 @@ private static ValueNode narrowOsrLocal(StructuredGraph graph, Stamp narrowedSta
              * test against a type, it is not an instance of operation but a null check and an
              * isArray check
              */
-            if (checkedStamp.isObjectStamp() && ((ObjectStamp) checkedStamp).nonNull()) {
-                // without a null check
-                check = graph.addOrUniqueWithInputs(ObjectIsArrayNode.create(effectiveOsrLocal));
-            } else {
-                // add a preceding null check with the same speculation reason
-                check = graph.addOrUniqueWithInputs(LogicNegationNode.create(IsNullNode.create(effectiveOsrLocal)));
-                SpeculationLog.Speculation constant = graph.getSpeculationLog().speculate(reason);
-                FixedGuardNode guard = graph.add(new FixedGuardNode(check, DeoptimizationReason.OptimizedTypeCheckViolated, DeoptimizationAction.InvalidateRecompile, constant, false));
-                graph.addAfterFixed(osrStart, guard);
-                PiNode nonNullPi = graph.addOrUnique(new PiNode(effectiveOsrLocal, ((ObjectStamp) checkedStamp).asNonNull(), guard));
-
-                insertionPoint = guard;
-
-                // with a null check
-                check = graph.addOrUnique(ObjectIsArrayNode.create(nonNullPi));
-            }
+            // add a preceding null check with the same speculation reason
+            check = graph.addOrUniqueWithInputs(LogicNegationNode.create(IsNullNode.create(effectiveOsrLocal)));
+            SpeculationLog.Speculation constant = graph.getSpeculationLog().speculate(reason);
+            FixedGuardNode guard = graph.add(new FixedGuardNode(check, DeoptimizationReason.OptimizedTypeCheckViolated, DeoptimizationAction.InvalidateRecompile, constant, false));
+            graph.addAfterFixed(osrStart, guard);
+            PiNode nonNullPi = graph.addOrUnique(new PiNode(effectiveOsrLocal, ((ObjectStamp) checkedStamp).asNonNull(), guard));
+
+            insertionPoint = guard;
+
+            // with a null check
+            check = graph.addOrUnique(ObjectIsArrayNode.create(nonNullPi));
             checkedStamp = new ObjectStamp(null, false, true, false, true);
         } else {
             check = graph.addOrUniqueWithInputs(InstanceOfNode.createHelper((ObjectStamp) checkedStamp, effectiveOsrLocal, null, null));