# HG changeset patch # User Tom Rodriguez # Date 1399331633 25200 # Node ID 76213c9350ad2e8c02d1f6bea43e01d80076bcfd # Parent 4cdc787681d41a8e085d1bf5f15cc321a5bc8ccd improve annotation error reporting diff -r 4cdc787681d4 -r 76213c9350ad graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/NodeLIRBuilder.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/NodeLIRBuilder.java Mon May 05 16:13:49 2014 -0700 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/NodeLIRBuilder.java Mon May 05 16:13:53 2014 -0700 @@ -23,6 +23,7 @@ package com.oracle.graal.compiler.gen; import static com.oracle.graal.api.code.ValueUtil.*; +import static com.oracle.graal.compiler.GraalDebugConfig.*; import static com.oracle.graal.compiler.common.GraalOptions.*; import static com.oracle.graal.lir.LIR.*; import static com.oracle.graal.nodes.ConstantNode.*; @@ -71,7 +72,9 @@ this.gen = gen; this.nodeOperands = graph.createNodeMap(); this.debugInfoBuilder = createDebugInfoBuilder(nodeOperands); - matchRules = MatchRuleRegistry.lookup(getClass()); + if (MatchExpressions.getValue()) { + matchRules = MatchRuleRegistry.lookup(getClass()); + } } @SuppressWarnings("hiding") @@ -210,11 +213,9 @@ List nodes = blockMap.get(block); - if (MatchExpressions.getValue()) { - // Allow NodeLIRBuilder subclass to specialize code generation of any interesting groups - // of instructions - matchComplexExpressions(nodes); - } + // Allow NodeLIRBuilder subclass to specialize code generation of any interesting groups + // of instructions + matchComplexExpressions(nodes); for (int i = 0; i < nodes.size(); i++) { Node instr = nodes.get(i); @@ -274,6 +275,13 @@ protected void matchComplexExpressions(List nodes) { if (matchRules != null) { try (Scope s = Debug.scope("MatchComplexExpressions")) { + if (LogVerbose.getValue()) { + int i = 0; + for (ScheduledNode node : nodes) { + Debug.log("%d: (%s) %1S", i++, node.usages().count(), node); + } + } + // Match the nodes in backwards order to encourage longer matches. for (int index = nodes.size() - 1; index >= 0; index--) { ScheduledNode snode = nodes.get(index); @@ -281,6 +289,9 @@ continue; } ValueNode node = (ValueNode) snode; + if (getOperand(node) != null) { + continue; + } // See if this node is the root of any MatchStatements List statements = matchRules.get(node.getClass()); if (statements != null) { diff -r 4cdc787681d4 -r 76213c9350ad graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchContext.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchContext.java Mon May 05 16:13:49 2014 -0700 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchContext.java Mon May 05 16:13:53 2014 -0700 @@ -104,12 +104,12 @@ // these can be evaluated lazily so don't worry about them. This should probably be // captured by some interface that indicates that their generate method is empty. continue; - } else if (consumed == null || !consumed.contains(node) && node != root) { + } else if ((consumed == null || !consumed.contains(node)) && node != root) { if (LogVerbose.getValue()) { Debug.log("unexpected node %s", node); for (int j = startIndex; j <= endIndex; j++) { ScheduledNode theNode = nodes.get(j); - Debug.log("%s(%s) %1s", (consumed.contains(theNode) || theNode == root) ? "*" : " ", theNode.usages().count(), theNode); + Debug.log("%s(%s) %1s", (consumed != null && consumed.contains(theNode) || theNode == root) ? "*" : " ", theNode.usages().count(), theNode); } } return Result.NOT_SAFE(node, rule.getPattern()); @@ -147,14 +147,16 @@ public Result consume(ValueNode node) { assert node.usages().count() <= 1 : "should have already been checked"; + // Check NOT_IN_BLOCK first since that usually implies ALREADY_USED + int index = nodes.indexOf(node); + if (index == -1) { + return Result.NOT_IN_BLOCK(node, rule.getPattern()); + } + if (builder.hasOperand(node)) { return Result.ALREADY_USED(node, rule.getPattern()); } - int index = nodes.indexOf(node); - if (index == -1) { - return Result.NOT_IN_BLOCK(node, rule.getPattern()); - } startIndex = Math.min(startIndex, index); if (consumed == null) { consumed = new ArrayList<>(2); diff -r 4cdc787681d4 -r 76213c9350ad graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchProcessor.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchProcessor.java Mon May 05 16:13:49 2014 -0700 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchProcessor.java Mon May 05 16:13:53 2014 -0700 @@ -53,6 +53,9 @@ @SupportedAnnotationTypes({"com.oracle.graal.compiler.match.MatchRule", "com.oracle.graal.compiler.match.MatchRules", "com.oracle.graal.compiler.match.MatchableNode"}) public class MatchProcessor extends AbstractProcessor { + public MatchProcessor() { + } + @Override public SourceVersion getSupportedSourceVersion() { return SourceVersion.latest(); @@ -142,6 +145,7 @@ next(); return descriptor; } + throw new RuleParseError("Too many arguments to " + descriptor.nodeType.nodeClass); } throw new RuleParseError("Extra tokens following match pattern: " + peek(null)); } @@ -265,11 +269,16 @@ List requiredPackages = new ArrayList<>(); /** - * The automatically generated wrapper class for a method based MatchRule. + * The java.lang.reflect.Method for invoking a method based MatchRule. */ private Map invokers = new LinkedHashMap<>(); + private TypeDescriptor valueType; + private TypeMirror matchRulesTypeMirror; + + private TypeMirror matchRuleTypeMirror; + private void declareType(TypeMirror mirror, String shortName, String nodeClass, String nodePackage, int inputs, String adapter, boolean commutative, boolean shareable, Element element) { TypeDescriptor descriptor = new TypeDescriptor(mirror, shortName, nodeClass, nodePackage, inputs, adapter, commutative, shareable); descriptor.originatingElements.add(element); @@ -279,13 +288,10 @@ } } - private static String findPackage(Element type) { - Element enclosing = type.getEnclosingElement(); - while (enclosing != null && enclosing.getKind() != ElementKind.PACKAGE) { - enclosing = enclosing.getEnclosingElement(); - } - if (enclosing != null && enclosing.getKind() == ElementKind.PACKAGE) { - return ((PackageElement) enclosing).getQualifiedName().toString(); + private String findPackage(Element type) { + PackageElement p = processingEnv.getElementUtils().getPackageOf(type); + if (p != null) { + return p.getQualifiedName().toString(); } throw new GraalInternalError("can't find package for %s", type); } @@ -373,7 +379,7 @@ /** * Strip the package off a class name leaving the full class name including any outer classes. */ - static String fullClassName(Element element) { + private String fullClassName(Element element) { assert element.getKind() == ElementKind.CLASS || element.getKind() == ElementKind.INTERFACE : element; String pkg = findPackage(element); return ((TypeElement) element).getQualifiedName().toString().substring(pkg.length() + 1); @@ -386,7 +392,7 @@ String matchStatementClassName = topDeclaringClass + "_" + MatchStatementSet.class.getSimpleName(); Element[] originatingElements = info.originatingElements.toArray(new Element[info.originatingElements.size()]); - Types typeUtils = processingEnv.getTypeUtils(); + Types typeUtils = typeUtils(); Filer filer = processingEnv.getFiler(); try (PrintWriter out = createSourceFile(pkg, matchStatementClassName, filer, originatingElements)) { @@ -560,17 +566,28 @@ return topDeclaringType(enclosing); } + private AnnotationMirror findAnnotationMirror(Element element, TypeMirror typeMirror) { + for (AnnotationMirror mirror : element.getAnnotationMirrors()) { + if (typeUtils().isSameType(mirror.getAnnotationType(), typeMirror)) { + return mirror; + } + } + return null; + } + @Override public boolean process(Set annotations, RoundEnvironment roundEnv) { if (roundEnv.processingOver()) { return true; } + matchRulesTypeMirror = processingEnv.getElementUtils().getTypeElement(MatchRules.class.getCanonicalName()).asType(); + matchRuleTypeMirror = processingEnv.getElementUtils().getTypeElement(MatchRule.class.getCanonicalName()).asType(); try { // Define a TypeDescriptor the generic node but don't enter it into the nodeTypes table // since it shouldn't mentioned in match rules. - TypeMirror mirror = processingEnv.getElementUtils().getTypeElement(ValueNode.class.getName()).asType(); - valueType = new TypeDescriptor(mirror, "Value", ValueNode.class.getSimpleName(), ValueNode.class.getPackage().getName(), 0, null, false, false); + TypeMirror valueTypeMirror = processingEnv.getElementUtils().getTypeElement(ValueNode.class.getName()).asType(); + valueType = new TypeDescriptor(valueTypeMirror, "Value", ValueNode.class.getSimpleName(), ValueNode.class.getPackage().getName(), 0, null, false, false); // Import default definitions processMatchableNode(processingEnv.getElementUtils().getTypeElement(GraalMatchableNodes.class.getName())); @@ -589,11 +606,12 @@ } Map map = new HashMap<>(); + for (Element element : roundEnv.getElementsAnnotatedWith(MatchRule.class)) { - processMatchRule(map, element); + processMatchRule(map, element, findAnnotationMirror(element, matchRuleTypeMirror)); } for (Element element : roundEnv.getElementsAnnotatedWith(MatchRules.class)) { - processMatchRule(map, element); + processMatchRule(map, element, findAnnotationMirror(element, matchRulesTypeMirror)); } for (MatchRuleDescriptor info : map.values()) { @@ -665,7 +683,7 @@ processingEnv.getMessager().printMessage(Kind.ERROR, "Exception throw during processing: " + t.toString() + " " + Arrays.toString(Arrays.copyOf(t.getStackTrace(), 2)), element); } - private void processMatchRule(Map map, Element element) { + private void processMatchRule(Map map, Element element, AnnotationMirror mirror) { if (!processedMatchRule.contains(element)) { try { processedMatchRule.add(element); @@ -679,9 +697,16 @@ info = new MatchRuleDescriptor(topDeclaringType); map.put(topDeclaringType, info); } - for (MatchRule matchRule : element.getAnnotationsByType(MatchRule.class)) { - // System.err.println(matchRule); - processMethodMatchRule((ExecutableElement) element, info, matchRule); + if (typeUtils().isSameType(mirror.getAnnotationType(), matchRulesTypeMirror)) { + List value = getAnnotationValueList(AnnotationMirror.class, mirror, "value"); + int i = 0; + for (MatchRule matchRule : element.getAnnotationsByType(MatchRule.class)) { + processMethodMatchRule((ExecutableElement) element, info, matchRule, value.get(i++)); + } + } else { + for (MatchRule matchRule : element.getAnnotationsByType(MatchRule.class)) { + processMethodMatchRule((ExecutableElement) element, info, matchRule, mirror); + } } } catch (Throwable t) { reportExceptionThrow(element, t); @@ -689,8 +714,12 @@ } } - private void processMethodMatchRule(ExecutableElement method, MatchRuleDescriptor info, MatchRule matchRule) { - Types typeUtils = processingEnv.getTypeUtils(); + private Types typeUtils() { + return processingEnv.getTypeUtils(); + } + + private void processMethodMatchRule(ExecutableElement method, MatchRuleDescriptor info, MatchRule matchRule, AnnotationMirror mirror) { + Types typeUtils = typeUtils(); if (!method.getModifiers().contains(Modifier.PUBLIC)) { String msg = String.format("MatchRule method %s must be public", method.getSimpleName()); @@ -775,7 +804,136 @@ info.matchRules.add(new MatchRuleItem(match, invoker)); } } catch (RuleParseError e) { - processingEnv.getMessager().printMessage(Kind.ERROR, e.getMessage(), method); + processingEnv.getMessager().printMessage(Kind.ERROR, e.getMessage(), method, mirror); } } + + // TODO borrowed from com.oracle.truffle.dsl.processor.Utils + @SuppressWarnings("unchecked") + private static List getAnnotationValueList(Class expectedListType, AnnotationMirror mirror, String name) { + List values = getAnnotationValue(List.class, mirror, name); + List result = new ArrayList<>(); + + if (values != null) { + for (AnnotationValue value : values) { + T annotationValue = resolveAnnotationValue(expectedListType, value); + if (annotationValue != null) { + result.add(annotationValue); + } + } + } + return result; + } + + private static T getAnnotationValue(Class expectedType, AnnotationMirror mirror, String name) { + return resolveAnnotationValue(expectedType, getAnnotationValue(mirror, name)); + } + + @SuppressWarnings({"unchecked"}) + private static T resolveAnnotationValue(Class expectedType, AnnotationValue value) { + if (value == null) { + return null; + } + + Object unboxedValue = value.accept(new AnnotationValueVisitorImpl(), null); + if (unboxedValue != null) { + if (expectedType == TypeMirror.class && unboxedValue instanceof String) { + return null; + } + if (!expectedType.isAssignableFrom(unboxedValue.getClass())) { + throw new ClassCastException(unboxedValue.getClass().getName() + " not assignable from " + expectedType.getName()); + } + } + return (T) unboxedValue; + } + + private static AnnotationValue getAnnotationValue(AnnotationMirror mirror, String name) { + ExecutableElement valueMethod = null; + for (ExecutableElement method : ElementFilter.methodsIn(mirror.getAnnotationType().asElement().getEnclosedElements())) { + if (method.getSimpleName().toString().equals(name)) { + valueMethod = method; + break; + } + } + + if (valueMethod == null) { + return null; + } + + AnnotationValue value = mirror.getElementValues().get(valueMethod); + if (value == null) { + value = valueMethod.getDefaultValue(); + } + + return value; + } + + private static class AnnotationValueVisitorImpl extends AbstractAnnotationValueVisitor7 { + + @Override + public Object visitBoolean(boolean b, Void p) { + return Boolean.valueOf(b); + } + + @Override + public Object visitByte(byte b, Void p) { + return Byte.valueOf(b); + } + + @Override + public Object visitChar(char c, Void p) { + return c; + } + + @Override + public Object visitDouble(double d, Void p) { + return d; + } + + @Override + public Object visitFloat(float f, Void p) { + return f; + } + + @Override + public Object visitInt(int i, Void p) { + return i; + } + + @Override + public Object visitLong(long i, Void p) { + return i; + } + + @Override + public Object visitShort(short s, Void p) { + return s; + } + + @Override + public Object visitString(String s, Void p) { + return s; + } + + @Override + public Object visitType(TypeMirror t, Void p) { + return t; + } + + @Override + public Object visitEnumConstant(VariableElement c, Void p) { + return c; + } + + @Override + public Object visitAnnotation(AnnotationMirror a, Void p) { + return a; + } + + @Override + public Object visitArray(List vals, Void p) { + return vals; + } + + } } diff -r 4cdc787681d4 -r 76213c9350ad graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchRuleRegistry.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchRuleRegistry.java Mon May 05 16:13:49 2014 -0700 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchRuleRegistry.java Mon May 05 16:13:53 2014 -0700 @@ -29,7 +29,7 @@ import com.oracle.graal.compiler.gen.*; import com.oracle.graal.debug.*; -import com.oracle.graal.debug.Debug.*; +import com.oracle.graal.debug.Debug.Scope; import com.oracle.graal.nodes.*; public class MatchRuleRegistry { diff -r 4cdc787681d4 -r 76213c9350ad graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchStatement.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchStatement.java Mon May 05 16:13:49 2014 -0700 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/match/MatchStatement.java Mon May 05 16:13:53 2014 -0700 @@ -30,8 +30,7 @@ import com.oracle.graal.api.meta.*; import com.oracle.graal.compiler.common.*; import com.oracle.graal.compiler.gen.*; -import com.oracle.graal.compiler.match.MatchPattern.MatchResultCode; -import com.oracle.graal.compiler.match.MatchPattern.Result; +import com.oracle.graal.compiler.match.MatchPattern.*; import com.oracle.graal.debug.*; import com.oracle.graal.graph.*; import com.oracle.graal.graph.Node.Verbosity; @@ -99,8 +98,15 @@ if (value != null) { context.setResult(value); MatchStatementSuccess.increment(); + Debug.metric("MatchStatement[%s]", getName()).increment(); return true; } + // The pattern matched but some other code generation constraint disallowed code + // generation for the pattern. + if (LogVerbose.getValue()) { + Debug.log("while matching %s|%s %s %s returned null", context.getRoot().toString(Verbosity.Id), context.getRoot().getClass().getSimpleName(), getName(), generatorMethod.getName()); + Debug.log("with nodes %s", formatMatch(node)); + } } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { throw new GraalInternalError(e); }