changeset 3013:5ee0f57bb18c

added message to verification error, InliningPhase uses TTY
author Lukas Stadler <lukas.stadler@jku.at>
date Fri, 17 Jun 2011 14:58:03 +0200
parents 4d03919746d4
children 681a227c332b
files graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/GraalCompiler.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/GraalOptions.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/graph/IR.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/AccessField.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/EndNode.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/phases/GraphBuilderPhase.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/phases/InliningPhase.java graal/com.oracle.max.graal.graph/src/com/oracle/max/graal/graph/Node.java graal/com.oracle.max.graal.graph/src/com/oracle/max/graal/graph/VerificationListener.java src/share/vm/runtime/sharedRuntime.cpp
diffstat 10 files changed, 62 insertions(+), 57 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/GraalCompiler.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/GraalCompiler.java	Fri Jun 17 14:58:03 2011 +0200
@@ -71,12 +71,12 @@
 
         Graph.verificationListeners.add(new VerificationListener() {
             @Override
-            public void verificationFailed(Node n) {
+            public void verificationFailed(Node n, String message) {
                 GraalCompiler.this.fireCompilationEvent(new CompilationEvent(currentCompilation, "Verification Error on Node " + n.id(), currentCompilation.graph, true, false));
                 for (Node p : n.predecessors()) {
                     TTY.println("predecessor: " + p);
                 }
-                assert false : "Verification of node " + n + " failed";
+                assert false : "Verification of node " + n + " failed: " + message;
             }
         });
     }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/GraalOptions.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/GraalOptions.java	Fri Jun 17 14:58:03 2011 +0200
@@ -42,6 +42,8 @@
     public static int     MaximumInstructionCount            = 37000;
     public static float   MaximumInlineRatio                 = 0.90f;
     public static int     MaximumInlineSize                  = 35;
+    public static int     MaximumFreqInlineSize              = 200;
+    public static int     FreqInlineRatio                    = 20;
     public static int     MaximumTrivialSize                 = 6;
     public static int     MaximumInlineLevel                 = 9;
     public static int     MaximumRecursiveInlineLevel        = 2;
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/graph/IR.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/graph/IR.java	Fri Jun 17 14:58:03 2011 +0200
@@ -69,7 +69,7 @@
      */
     public void build() {
 
-//        Object stored = compilation.method.compilerStorage().get(CompilerGraph.class);
+//        Object stored = GraphBuilderPhase.cachedGraphs.get(compilation.method);
 //        if (stored != null) {
 //            Map<Node, Node> replacements = new HashMap<Node, Node>();
 //            CompilerGraph duplicate = (CompilerGraph) stored;
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/AccessField.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/AccessField.java	Fri Jun 17 14:58:03 2011 +0200
@@ -23,6 +23,7 @@
 package com.oracle.max.graal.compiler.ir;
 
 import java.lang.reflect.*;
+import java.util.*;
 
 import com.oracle.max.graal.graph.*;
 import com.sun.cri.ci.*;
@@ -101,4 +102,11 @@
     public boolean isVolatile() {
         return Modifier.isVolatile(field.accessFlags());
     }
+
+    @Override
+    public Map<Object, Object> getDebugProperties() {
+        Map<Object, Object> properties = super.getDebugProperties();
+        properties.put("field", field);
+        return properties;
+    }
 }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/EndNode.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/EndNode.java	Fri Jun 17 14:58:03 2011 +0200
@@ -60,10 +60,10 @@
 
     @Override
     public boolean verify() {
-        assertTrue(inputs().size() == 0);
-        assertTrue(successors().size() == 0);
-        assertTrue(usages().size() <= 1);
-        assertTrue(predecessors().size() <= 1);
+        assertTrue(inputs().size() == 0, "inputs empty");
+        assertTrue(successors().size() == 0, "successors empty");
+        assertTrue(usages().size() <= 1, "at most one usage");
+        assertTrue(predecessors().size() <= 1, "at most one predecessor");
         return true;
     }
 }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/phases/GraphBuilderPhase.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/phases/GraphBuilderPhase.java	Fri Jun 17 14:58:03 2011 +0200
@@ -104,7 +104,7 @@
 
     private final boolean createUnwind;
 
-    public static HashMap<String, Integer> methodCount = new HashMap<String, Integer>();
+    public static final Map<RiMethod, CompilerGraph> cachedGraphs = new WeakHashMap<RiMethod, CompilerGraph>();
 
     /**
      * Creates a new, initialized, {@code GraphBuilder} instance for a given compilation.
@@ -126,23 +126,6 @@
         this.constantPool = runtime.getConstantPool(method);
         this.createUnwind = createUnwind;
         this.storeResultGraph = true;
-
-//        String name = method.toString().intern();
-//        if (methodCount.get(name) == null) {
-//            methodCount.put(name, 1);
-//        } else {
-//            methodCount.put(name, methodCount.get(name) + 1);
-//        }
-//
-//        int inlined = 0;
-//        int duplicate = 0;
-//        for (Map.Entry<String, Integer> entry : methodCount.entrySet()) {
-//            inlined += entry.getValue();
-//            duplicate += entry.getValue() - 1;
-//        }
-//        if (inlined > 0) {
-//            System.out.printf("GraphBuilder overhead: %d (%5.3f %%)\n", duplicate, duplicate * 100.0 / inlined);
-//        }
     }
 
     @Override
@@ -234,7 +217,7 @@
             replacements.put(graph.start(), duplicate.start());
             duplicate.addDuplicate(graph.getNodes(), replacements);
 
-            method.compilerStorage().put(CompilerGraph.class, duplicate);
+            cachedGraphs.put(method, duplicate);
         }
     }
 
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/phases/InliningPhase.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/phases/InliningPhase.java	Fri Jun 17 14:58:03 2011 +0200
@@ -26,6 +26,7 @@
 import java.util.*;
 
 import com.oracle.max.graal.compiler.*;
+import com.oracle.max.graal.compiler.debug.*;
 import com.oracle.max.graal.compiler.graph.*;
 import com.oracle.max.graal.compiler.ir.*;
 import com.oracle.max.graal.compiler.value.*;
@@ -79,7 +80,7 @@
                         if (trace) {
                             String targetName = CiUtil.format("%H.%n(%p):%r", invoke.target, false);
                             String concreteName = CiUtil.format("%H.%n(%p):%r", concrete, false);
-                            System.out.printf("recording concrete method assumption: %s -> %s\n", targetName, concreteName);
+                            TTY.println("recording concrete method assumption: %s -> %s", targetName, concreteName);
                         }
                         compilation.assumptions.recordConcreteMethod(invoke.target, concrete);
                         addToQueue(invoke, concrete);
@@ -98,7 +99,7 @@
                             guard.setNext(invoke);
 
                             if (trace) {
-                                System.out.println("inlining with type check, type probability: " + profile.probabilities[0]);
+                                TTY.println("inlining with type check, type probability: %5.3f", profile.probabilities[0]);
                             }
                             addToQueue(invoke, concrete);
 //                            System.out.println("inlining with type check " + profile.probabilities[0] + " " + profile.morphism + " " + profile.count);
@@ -132,7 +133,7 @@
 
             if (inliningSize > GraalOptions.MaximumInstructionCount) {
                 if (trace) {
-                    System.out.println("inlining stopped: MaximumInstructionCount reached");
+                    TTY.println("inlining stopped: MaximumInstructionCount reached");
                 }
                 break;
             }
@@ -147,7 +148,7 @@
                 duplicate += entry.getValue() - 1;
             }
             if (inlined > 0) {
-                System.out.printf("overhead_: %d (%5.3f %%)\n", duplicate, duplicate * 100.0 / inlined);
+                TTY.println("overhead: %d (%5.3f %%)", duplicate, duplicate * 100.0 / inlined);
             }
         }
     }
@@ -157,74 +158,82 @@
 
         if (invoke.stateAfter() == null) {
             if (trace) {
-                System.out.println("not inlining " + name + " because the invoke has no after state");
+                TTY.println("not inlining %s because the invoke has no after state", name);
             }
             return false;
         }
         if (invoke.stateAfter().locksSize() > 0) {
             if (trace) {
-                System.out.println("not inlining " + name + " because of locks");
+                TTY.println("not inlining %s because of locks", name);
             }
             return false;
         }
         if (!invoke.target.isResolved()) {
             if (trace) {
-                System.out.println("not inlining " + name + " because the invoke target is unresolved");
+                TTY.println("not inlining %s because the invoke target is unresolved", name);
             }
             return false;
         }
         if (invoke.predecessors().size() == 0) {
             if (trace) {
-                System.out.println("not inlining " + name + " because the invoke is dead code");
+                TTY.println("not inlining %s because the invoke is dead code", name);
             }
             return false;
         }
         if (invoke.stateAfter() == null) {
             if (trace) {
-                System.out.println("not inlining " + name + " because of missing frame state");
+                TTY.println("not inlining %s because of missing frame state", name);
             }
         }
         return true;
     }
 
-    private boolean checkInliningConditions(RiMethod method, int iterations, Invoke invoke, RiTypeProfile profile, float ratio) {
+    private boolean checkInliningConditions(RiMethod method, int iterations, Invoke invoke, RiTypeProfile profile, float adjustedRatio) {
         String name = !trace ? null : CiUtil.format("%H.%n(%p):%r", method, false) + " (" + method.codeSize() + " bytes)";
         if (Modifier.isNative(method.accessFlags())) {
             if (trace) {
-                System.out.println("not inlining " + name + " because it is a native method");
+                TTY.println("not inlining %s because it is a native method", name);
             }
             return false;
         }
-        if (method.code().length > GraalOptions.MaximumInlineSize) {
+        if (Modifier.isAbstract(method.accessFlags())) {
             if (trace) {
-                System.out.println("not inlining " + name + " because of code size");
+                TTY.println("not inlining %s because it is an abstract method", name);
             }
             return false;
         }
         if (!method.holder().isInitialized()) {
             if (trace) {
-                System.out.println("not inlining " + name + " because of non-initialized class");
+                TTY.println("not inlining %s because of non-initialized class", name);
             }
             return false;
         }
         if (method == compilation.method && iterations > GraalOptions.MaximumRecursiveInlineLevel) {
             if (trace) {
-                System.out.println("not inlining " + name + " because of recursive inlining limit");
+                TTY.println("not inlining %s because of recursive inlining limit", name);
             }
             return false;
         }
-        if (method.code().length > GraalOptions.MaximumTrivialSize) {
+        int maximumSize = GraalOptions.MaximumTrivialSize;
+        float ratio = 0;
+        if (profile != null && profile.count > 0) {
             RiMethod parent = parentMethod.get(invoke);
             if (parent == null) {
                 parent = compilation.method;
             }
-            if (profile == null || profile.count < parent.invocationCount() * (1 - ratio)) {
-                if (trace) {
-                    System.out.println("not inlining " + name + " because the invocation counter is too low");
-                }
-                return false;
+            ratio = profile.count / (float) parent.invocationCount();
+            if (ratio >= GraalOptions.FreqInlineRatio) {
+                maximumSize = GraalOptions.MaximumFreqInlineSize;
+            } else if (ratio >= (1 - adjustedRatio)) {
+                maximumSize = GraalOptions.MaximumInlineSize;
             }
         }
+        if (method.codeSize() > maximumSize) {
+            if (trace) {
+                TTY.println("not inlining %s because of code size (size: %d, max size: %d, ratio %5.3f)", name, method.codeSize(), maximumSize, ratio);
+            }
+            return false;
+        }
         return true;
     }
 
@@ -234,15 +243,15 @@
         Instruction exceptionEdge = invoke.exceptionEdge();
 
         CompilerGraph graph;
-        Object stored = method.compilerStorage().get(CompilerGraph.class);
+        Object stored = GraphBuilderPhase.cachedGraphs.get(method);
         if (stored != null) {
             if (trace) {
-                System.out.printf("Reusing graph for %s, locals: %d, stack: %d\n", name, method.maxLocals(), method.maxStackSize());
+                TTY.println("Reusing graph for %s, locals: %d, stack: %d", name, method.maxLocals(), method.maxStackSize());
             }
             graph = (CompilerGraph) stored;
         } else {
             if (trace) {
-                System.out.printf("Building graph for %s, locals: %d, stack: %d\n", name, method.maxLocals(), method.maxStackSize());
+                TTY.println("Building graph for %s, locals: %d, stack: %d", name, method.maxLocals(), method.maxStackSize());
             }
             graph = new CompilerGraph(null);
             new GraphBuilderPhase(compilation, method, true, true).apply(graph);
@@ -290,7 +299,7 @@
         }
 
         if (trace) {
-            System.out.println("inlining " + name + ": " + frameStates.size() + " frame states, " + nodes.size() + " nodes");
+            TTY.println("inlining %s: %d frame states, %d nodes", name, frameStates.size(), nodes.size());
         }
 
         assert invoke.successors().get(0) != null : invoke;
@@ -318,7 +327,6 @@
         if (monitorIndexDelta > 0) {
             for (Map.Entry<Node, Node> entry : duplicates.entrySet()) {
                 if (entry.getValue() instanceof MonitorAddress) {
-                    System.out.println("Adjusting monitor index");
                     MonitorAddress address = (MonitorAddress) entry.getValue();
                     address.setMonitorIndex(address.monitorIndex() + monitorIndexDelta);
                 }
--- a/graal/com.oracle.max.graal.graph/src/com/oracle/max/graal/graph/Node.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.graph/src/com/oracle/max/graal/graph/Node.java	Fri Jun 17 14:58:03 2011 +0200
@@ -210,12 +210,16 @@
     }
 
     public final void assertTrue(boolean cond) {
-        assert cond || assertionFailure();
+        assert cond || assertionFailure("");
     }
 
-    public final boolean assertionFailure() {
+    public final void assertTrue(boolean cond, String message) {
+        assert cond || assertionFailure(message);
+    }
+
+    public final boolean assertionFailure(String message) {
         for (VerificationListener l : Graph.verificationListeners) {
-            l.verificationFailed(this);
+            l.verificationFailed(this, message);
         }
         return true;
     }
--- a/graal/com.oracle.max.graal.graph/src/com/oracle/max/graal/graph/VerificationListener.java	Thu Jun 16 20:43:51 2011 +0200
+++ b/graal/com.oracle.max.graal.graph/src/com/oracle/max/graal/graph/VerificationListener.java	Fri Jun 17 14:58:03 2011 +0200
@@ -24,5 +24,5 @@
 
 
 public interface VerificationListener {
-    void verificationFailed(Node n);
+    void verificationFailed(Node n, String message);
 }
--- a/src/share/vm/runtime/sharedRuntime.cpp	Thu Jun 16 20:43:51 2011 +0200
+++ b/src/share/vm/runtime/sharedRuntime.cpp	Fri Jun 17 14:58:03 2011 +0200
@@ -2674,7 +2674,7 @@
   // ResourceObject, so do not put any ResourceMarks in here.
   char *s = sig->as_C_string();
   int len = (int)strlen(s);
-  *s++; len--;                  // Skip opening paren
+  s++; len--;                  // Skip opening paren
   char *t = s+len;
   while( *(--t) != ')' ) ;      // Find close paren