Problem adding match rule for (ReadNode address)

[Andrew Dinn]

I have been trying to add a match rule for a ReadNode to the AArch64
graal LIRGenerator and was foiled by the following bug (which has been
accidentally side-stepped by the x86 implementation -- details below).

If I add a match rule/handler method to the end of class
AArch64NodeMatchRules as follows

graal/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeMatchRules.java
@@ -33,6 +33,8 @@ import org.graalvm.compiler.nodes.DeoptimizingNode;
 import org.graalvm.compiler.nodes.calc.SignExtendNode;
 import org.graalvm.compiler.nodes.calc.ZeroExtendNode;
 import org.graalvm.compiler.nodes.memory.Access;
+import org.graalvm.compiler.nodes.memory.ReadNode;
+import org.graalvm.compiler.nodes.ValueNode;

 import jdk.vm.ci.aarch64.AArch64Kind;

@@ -75,4 +77,9 @@ public class AArch64NodeMatchRules extends
NodeMatchRules {
         AArch64Kind memoryKind = getMemoryKind(access);
         return builder ->
getArithmeticLIRGenerator().emitExtendMemory(true, memoryKind,
root.getResultBits(), (AArch64AddressValue)
operand(access.getAddress()), getState(access));
     }
+
+    @MatchRule("(Read address)")
+    public ComplexMatchResult checkRead(ReadNode root, ValueNode address) {
+        return null;
+    }
 }

then the build process fails as follows.

Compiling org.graalvm.compiler.replacements.amd64 with javac-daemon...
[dependency GRAAL_NODEINFO_PROCESSOR updated]
/home/adinn/openjdk/graal/graal-core/mxbuild/graal/org.graalvm.compiler.core.aarch64/src_gen/org/graalvm/compiler/core/aarch64/AArch64NodeMatchRules_MatchStatementSet.java:23:
error: cannot find symbol
            return ((AArch64NodeMatchRules)
nodeMatchRules).checkRead((ReadNode) args[0], (ValueNode) args[1]);

                  ^
  symbol:   class ValueNode
  location: class MatchGenerator_checkRead
1 error

The error happens because the generated matching engine code in
AArch64NodeMatchRules_MatchStatementSet.java does not include an import
declaration for class ValueNode. Imports are only generated for the
packages to which root nodes of matched terms and subterms belong but
not for packages containing Value arguments like address.

Note that the corresponding x86 code successfully employs match rules
which refer to ValueNode arguments because it includes rules (e.g. the
ones for IfNode) whose root nodes lie in the same package as ValueNode.

The fix is to modify class MatchProcessor so it always generates an
import for ValueNode

 +++
b/graal/org.graalvm.compiler.core.match.processor/src/org/graalvm/compiler/core/match/processor/MatchProcessor.java
@@ -521,6 +521,7 @@ public class MatchProcessor extends AbstractProcessor {
             out.println("import " + NodeMatchRules.class.getName() + ";");
             out.println("import " + Position.class.getName() + ";");
             out.println("import " + ServiceProvider.class.getName() + ";");
+            out.println("import " + ValueNode.class.getName() + ";");
             for (String p : info.requiredPackages) {
                 out.println("import " + p + ".*;");
             }

Do you need this patch to be submitted via a github pull request? Do I
need to raise an associated issue?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander