[GR-31289] Avoid unnecessary VM call and exception when canonicalizing array length read.

PullRequest: graal/8918

Author: woess

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/memory/ReadNode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -114,7 +114,15 @@ public static ValueNode canonicalizeRead(ValueNode read, AddressNode address, Lo
         if (tool.canonicalizeReads() && address instanceof OffsetAddressNode) {
             OffsetAddressNode objAddress = (OffsetAddressNode) address;
             ValueNode object = objAddress.getBase();
-            if (metaAccess != null && object.isConstant() && !object.isNullConstant() && objAddress.getOffset().isConstant()) {
+            // Note: readConstant cannot be used to read the array length, so in order to avoid an
+            // unnecessary CompilerToVM.readFieldValue call ending in an IllegalArgumentException,
+            // check if we are reading the array length location first.
+            if (locationIdentity.equals(ARRAY_LENGTH_LOCATION)) {
+                ValueNode length = GraphUtil.arrayLength(object, ArrayLengthProvider.FindLengthMode.CANONICALIZE_READ, tool.getConstantReflection());
+                if (length != null) {
+                    return length;
+                }
+            } else if (metaAccess != null && object.isConstant() && !object.isNullConstant() && objAddress.getOffset().isConstant()) {
                 long displacement = objAddress.getOffset().asJavaConstant().asLong();
                 int stableDimension = ((ConstantNode) object).getStableDimension();
                 if (locationIdentity.isImmutable() || stableDimension > 0) {
@@ -125,12 +133,6 @@ public static ValueNode canonicalizeRead(ValueNode read, AddressNode address, Lo
                     }
                 }
             }
-            if (locationIdentity.equals(ARRAY_LENGTH_LOCATION)) {
-                ValueNode length = GraphUtil.arrayLength(object, ArrayLengthProvider.FindLengthMode.CANONICALIZE_READ, tool.getConstantReflection());
-                if (length != null) {
-                    return length;
-                }
-            }
             if (locationIdentity instanceof CanonicalizableLocation) {
                 CanonicalizableLocation canonicalize = (CanonicalizableLocation) locationIdentity;
                 ValueNode result = canonicalize.canonicalizeRead(read, address, object, tool);

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/spi/ArrayLengthProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -44,7 +44,7 @@ enum FindLengthMode {
          *
          * Values that are defined inside a loop and flow out the loop need to be proxied by
          * {@link ValueProxyNode}. When this mode is used, new necessary proxy nodes are created
-         * base on the proxies that were found while traversing the path to the length node. In
+         * based on the proxies that were found while traversing the path to the length node. In
          * addition, new {@link ValuePhiNode phi nodes} can be created. The caller is responsible
          * for adding these nodes to the graph, i.e., the return value can be a node that is not yet
          * added to the graph.