Merge pull request jruby#8202 from enebo/igv

do not emit labels which are not used while constructing loops + IGV enhancements

core/src/main/java/org/jruby/RubyInstanceConfig.java

@@ -1771,6 +1771,7 @@ public boolean shouldPrecompileAll() {
 
     public static boolean IR_DEBUG = Options.IR_DEBUG.load(); // ast tool can toggle this
     public static final String IR_DEBUG_IGV = Options.IR_DEBUG_IGV.load();
+    public static final boolean IR_DEBUG_IGV_STDOUT = Options.IR_DEBUG_IGV_STDOUT.load();
     public static final boolean IR_PROFILE = Options.IR_PROFILE.load();
     public static boolean IR_COMPILER_DEBUG = Options.IR_COMPILER_DEBUG.load(); // ast tool can toggle this
     public static final boolean IR_WRITING = Options.IR_WRITING.load();

core/src/main/java/org/jruby/ir/IRScope.java

@@ -671,7 +671,7 @@ public IGVDumper dumpToIGV() {
 
             if (spec.contains(":") && spec.equals(getFileName() + ":" + getLineNumber()) ||
                     spec.equals(getFileName())) {
-                return new IGVDumper(getFullyQualifiedName() + "; line " + getLineNumber());
+                return new IGVDumper(getFullyQualifiedName() + "; line " + getLineNumber(), RubyInstanceConfig.IR_DEBUG_IGV_STDOUT);
             }
         }
 

core/src/main/java/org/jruby/ir/builder/EnsureBlockInfo.java

@@ -65,10 +65,12 @@ class EnsureBlockInfo {
     final List<Instr> instrs;
 
     public EnsureBlockInfo(IRScope s, IRLoop l, Label bodyRescuer) {
-        regionStart = s.getNewLabel();
-        start       = s.getNewLabel();
-        end         = s.getNewLabel();
-        dummyRescueBlockLabel = s.getNewLabel();
+        // this technically may be any block and not specifically rescue but for the sake of looking at the CFG
+        // it is more or less a begin block with exception handling around it.
+        regionStart = s.getNewLabel("BEGIN");
+        start       = s.getNewLabel("RESC_START");
+        end         = s.getNewLabel("AFTER_RESC");
+        dummyRescueBlockLabel = s.getNewLabel("RESC_DUMMY");
         instrs = new ArrayList<>();
         savedGlobalException = null;
         innermostLoop = l;

core/src/main/java/org/jruby/ir/builder/IRBuilder.java

@@ -1077,6 +1077,7 @@ protected Operand buildBreak(CodeBlock value, int line) {
             // If we have ensure blocks, have to run those first!
             if (!activeEnsureBlockStack.isEmpty()) emitEnsureBlocks(currLoop);
 
+            currLoop.hasBreak = true;
             addInstr(new CopyInstr(currLoop.loopResult, value.run()));
             addInstr(new JumpInstr(currLoop.loopEndLabel));
         } else {
@@ -1666,7 +1667,7 @@ protected Operand buildConditionalLoop(U conditionNode, U bodyNode, boolean isWh
             if (bodyNode != null) build(bodyNode);
 
             // Next jumps here
-            addInstr(new LabelInstr(loop.iterEndLabel));
+            if (loop.hasNext) addInstr(new LabelInstr(loop.iterEndLabel));
             if (isLoopHeadCondition) {
                 addInstr(new JumpInstr(loop.loopStartLabel));
             } else {
@@ -1679,7 +1680,7 @@ protected Operand buildConditionalLoop(U conditionNode, U bodyNode, boolean isWh
             addInstr(new CopyInstr(loopResult, nil()));
 
             // Loop end -- breaks jump here bypassing the result set up above
-            addInstr(new LabelInstr(loop.loopEndLabel));
+            if (loop.hasBreak) addInstr(new LabelInstr(loop.loopEndLabel));
 
             // Done with loop
             loopStack.pop();
@@ -1750,6 +1751,7 @@ protected Operand buildNext(final Operand rv, int line) {
         if (!activeEnsureBlockStack.isEmpty()) emitEnsureBlocks(currLoop);
 
         if (currLoop != null) {
+            currLoop.hasNext = true;
             // If a regular loop, the next is simply a jump to the end of the iteration
             addInstr(new JumpInstr(currLoop.iterEndLabel));
         } else {
@@ -2448,8 +2450,8 @@ protected Operand buildRedo(int line) {
     protected void buildRescueBodyInternal(U[] exceptions, U body, X consequent, Variable rv, Variable exc, Label endLabel,
                                  U reference) {
         // Compare and branch as necessary!
-        Label uncaughtLabel = getNewLabel();
-        Label caughtLabel = getNewLabel();
+        Label uncaughtLabel = getNewLabel("MISSED");
+        Label caughtLabel = getNewLabel("RESCUE");
         if (exceptions == null || exceptions.length == 0) {
             outputExceptionCheck(getManager().getStandardError(), exc, caughtLabel);
         } else {
@@ -2527,7 +2529,7 @@ protected Operand buildRescueInternal(U bodyNode, U elseNode, U[] exceptions, U
         // Labels marking start, else, end of the begin-rescue(-ensure)-end block
         Label rBeginLabel = getNewLabel();
         Label rEndLabel   = ensure.end;
-        Label rescueLabel = getNewLabel(); // Label marking start of the first rescue code.
+        Label rescueLabel = getNewLabel("RESC_TEST"); // Label marking start of the first rescue code.
         ensure.needsBacktrace = needsBacktrace;
 
         addInstr(new LabelInstr(rBeginLabel));

core/src/main/java/org/jruby/ir/builder/IRLoop.java

@@ -8,11 +8,14 @@ class IRLoop {
     public final IRScope container;
     public final IRLoop   parentLoop;
     public final Label loopStartLabel;
-    public final Label    loopEndLabel;
+    public final Label    loopEndLabel; // for `break`
     public final Label    iterStartLabel;
-    public final Label    iterEndLabel;
+    public final Label    iterEndLabel; // for `next`
     public final Variable loopResult;
 
+    public boolean hasNext = false;
+    public boolean hasBreak = false;
+
     public IRLoop(IRScope s, IRLoop outerLoop, Variable result) {
         container = s;
         parentLoop = outerLoop;

core/src/main/java/org/jruby/ir/representations/IGVCFGVisitor.java

@@ -5,7 +5,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import jnr.ffi.annotations.In;
 import org.jruby.ir.Tuple;
 import org.jruby.ir.instructions.BranchInstr;
 import org.jruby.ir.instructions.Instr;
@@ -25,10 +24,21 @@
  * visitor altogether not accessed via CFG.
  */
 public class IGVCFGVisitor {
+    private class Edge {
+        final int from;
+        final Object to;
+        final String name;
+
+        public Edge(int from, Object to, String name) {
+            this.from = from;
+            this.to = to;
+            this.name = name;
+        }
+    }
     final PrintStream writer;
     final Map<BasicBlock, Integer> indexOffsets = new HashMap();
-    final List<Tuple<Integer, Integer>> instrEdges = new ArrayList();
-    final List<Tuple<Integer, JumpTargetInstr>> extraInstrEdges = new ArrayList();
+    final List<Edge> instrEdges = new ArrayList();
+    final List<Edge> extraInstrEdges = new ArrayList();
     Instr lastInstr = null; // Last instr from the previous BB.
     final IGVInstrListener listener;
 
@@ -78,12 +88,12 @@ protected void visitInstrs(BasicBlock basicBlock) {
 
             // Last BB processed needs to hook up to first in next one if not a Jump (fallthrough)
             if (lastInstr != null && !(lastInstr instanceof JumpInstr)) {
-                instrEdges.add(new Tuple(System.identityHashCode(lastInstr), lastIPC));
+                instrEdges.add(new Edge(System.identityHashCode(lastInstr), lastIPC, "fallthrough"));
             }
 
             for (int i = 1; i < size; i++) {
                 int ipc = Instr(instrs.get(i));
-                instrEdges.add(new Tuple(lastIPC, ipc));
+                instrEdges.add(new Edge(lastIPC, ipc, "exception"));
                 lastIPC = ipc;
             }
 
@@ -111,12 +121,14 @@ public void CFG(CFG cfg, String name) {
 
         startTag(writer, "edges");
 
-        for (Tuple<Integer, Integer> edge: instrEdges) {
-            emptyTag(writer, "edge", "from", edge.a, "to", edge.b);
+        for (Edge edge: instrEdges) {
+            emptyTag(writer, "edge", "from", edge.from, "to", edge.to, "label", edge.name, "name", edge.name);
         }
 
-        for (Tuple<Integer, JumpTargetInstr> edge: extraInstrEdges) {
-            emptyTag(writer, "edge", "from", edge.a, "to", indexOffsets.get(cfg.getBBForLabel(edge.b.getJumpTarget())));
+        for (Edge edge: extraInstrEdges) {
+            emptyTag(writer, "edge", "from", edge.from,
+                    "to", indexOffsets.get(cfg.getBBForLabel((((JumpTargetInstr) edge.to).getJumpTarget()))),
+                    "label", edge.name, "name", edge.name);
         }
 
         for (Tuple<Instr, Instr> edge: listener.getRemovedEdges()) {
@@ -141,7 +153,7 @@ public int Instr(Instr instr) {
         property(writer, "name", instr);
 
         // We have not processed all BBs yet so we cannot resolve ipc locations of the jumps destinations.
-        if (instr instanceof BranchInstr) extraInstrEdges.add(new Tuple(ipc, (JumpTargetInstr) instr));
+        if (instr instanceof BranchInstr) extraInstrEdges.add(new Edge(ipc, instr, "branch"));
 
         endTag(writer, "properties");
         endTag(writer, "node");

core/src/main/java/org/jruby/ir/util/IGVDumper.java

@@ -20,12 +20,16 @@ public class IGVDumper {
     PrintStream writer;
     final String baseLabel;
 
-    public IGVDumper(String baseLabel) {
+    public IGVDumper(String baseLabel, boolean saveToFile) {
         this.baseLabel = baseLabel;
 
         try {
-            socket = new Socket(HOST, PORT);
-            writer = new PrintStream(socket.getOutputStream());
+            if (saveToFile) {
+                writer = System.out;
+            } else {
+                socket = new Socket(HOST, PORT);
+                writer = new PrintStream(socket.getOutputStream());
+            }
             startTag(writer, "graphDocument");
             startTag(writer, "group");
             startTag(writer, "properties");
@@ -44,8 +48,10 @@ public void close() {
         endTag(writer, "group");
         endTag(writer, "graphDocument");
         try {
-            writer.close();
-            socket.close();
+            if (socket != null) {
+                writer.close();
+                socket.close();
+            }
         } catch (IOException e) {
         }
     }

core/src/main/java/org/jruby/util/cli/Options.java

@@ -99,6 +99,7 @@ public class Options {
     public static final Option<ClassLoaderMode> JIT_LOADER_MODE = enumeration(JIT, "jit.loader.mode", ClassLoaderMode.class, ClassLoaderMode.UNIQUE, "Set JIT class loader to use. UNIQUE class loader per class; SHARED loader for all classes");
 
     public static final Option<String> IR_DEBUG_IGV          = string(IR, "ir.debug.igv", (String) null, "Specify file:line of scope to jump to IGV");
+    public static final Option<Boolean> IR_DEBUG_IGV_STDOUT = bool(IR, "ir.debug.igv.stdout", false, "Save IGV generated XML to stdout");
     public static final Option<Boolean> IR_DEBUG             = bool(IR, "ir.debug", false, "Debug generation of JRuby IR.");
     public static final Option<Boolean> IR_PROFILE           = bool(IR, "ir.profile", false, "[EXPT]: Profile IR code during interpretation.");
     public static final Option<Boolean> IR_COMPILER_DEBUG    = bool(IR, "ir.compiler.debug", false, "Debug compilation of JRuby IR.");