jruby · Dec 6, 2015 · Dec 6, 2015
Showing with 137 additions and 72 deletions.

+112 −71 core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java

+24 −0 core/src/main/java/org/jruby/runtime/MixedModeIRBlockBody.java

+1 −1 core/src/main/java/org/jruby/runtime/Signature.java
diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java
@@ -3,9 +3,11 @@
 import org.jruby.ir.*;
 import org.jruby.ir.dataflow.analyses.StoreLocalVarPlacementProblem;
 import org.jruby.ir.instructions.*;
+import org.jruby.runtime.Signature;
 import org.jruby.ir.operands.ImmutableLiteral;
 import org.jruby.ir.operands.Label;
 import org.jruby.ir.operands.Operand;
+import org.jruby.ir.operands.Self;
 import org.jruby.ir.operands.TemporaryVariable;
 import org.jruby.ir.operands.Variable;
 import org.jruby.ir.representations.BasicBlock;
@@ -20,7 +22,10 @@ public String getLabel() {
     }
 
     private boolean explicitCallProtocolSupported(IRScope scope) {
-        return scope instanceof IRMethod || (scope instanceof IRModuleBody && !(scope instanceof IRMetaClassBody));
+        return scope instanceof IRMethod
+            // SSS: Turning this off till this is fully debugged
+            // || (scope instanceof IRClosure && !(scope instanceof IREvalScript))
+            || (scope instanceof IRModuleBody && !(scope instanceof IRMetaClassBody));
     }
 
     /*
@@ -41,6 +46,16 @@ private void fixReturn(IRScope scope, ReturnBase i, ListIterator<Instr> instrs)
         }
     }
 
+    private void popSavedState(IRScope scope, boolean requireBinding, boolean requireFrame, Variable savedViz, Variable savedFrame, ListIterator<Instr> instrs) {
+        if (requireBinding) instrs.add(new PopBindingInstr());
+        if (scope instanceof IRClosure) {
+            instrs.add(new PopBlockFrameInstr(savedFrame));
+            instrs.add(new RestoreBindingVisibilityInstr(savedViz));
+        } else {
+            if (requireFrame) instrs.add(new PopMethodFrameInstr());
+        }
+    }
+
     @Override
     public Object execute(IRScope scope, Object... data) {
         // IRScriptBody do not get explicit call protocol instructions right now.
@@ -50,95 +65,121 @@ public Object execute(IRScope scope, Object... data) {
         // Add explicit frame and binding push/pop instrs ONLY for methods -- we cannot handle this in closures and evals yet
         // If the scope uses $_ or $~ family of vars, has local load/stores, or if its binding has escaped, we have
         // to allocate a dynamic scope for it and add binding push/pop instructions.
-        if (explicitCallProtocolSupported(scope)) {
-            StoreLocalVarPlacementProblem slvpp = scope.getStoreLocalVarPlacementProblem();
-            boolean scopeHasLocalVarStores = false;
-            boolean bindingHasEscaped      = scope.bindingHasEscaped();
+        if (!explicitCallProtocolSupported(scope)) return null;
 
-            CFG cfg = scope.getCFG();
+        StoreLocalVarPlacementProblem slvpp = scope.getStoreLocalVarPlacementProblem();
+        boolean scopeHasLocalVarStores = false;
+        boolean bindingHasEscaped = scope.bindingHasEscaped();
 
-            if (slvpp != null && bindingHasEscaped) {
-                scopeHasLocalVarStores = slvpp.scopeHasLocalVarStores();
-            } else {
-                // We dont require local-var load/stores to have been run.
-                // If it is not run, we go conservative and add push/pop binding instrs. everywhere
-                scopeHasLocalVarStores = bindingHasEscaped;
-            }
+        CFG cfg = scope.getCFG();
 
-            boolean requireFrame = doesItRequireFrame(scope, bindingHasEscaped);
-            boolean requireBinding = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
+        if (slvpp != null && bindingHasEscaped) {
+            scopeHasLocalVarStores = slvpp.scopeHasLocalVarStores();
+        } else {
+            // We dont require local-var load/stores to have been run.
+            // If it is not run, we go conservative and add push/pop binding instrs. everywhere
+            scopeHasLocalVarStores = bindingHasEscaped;
+        }
 
-            if (requireBinding || requireFrame) {
-                BasicBlock entryBB = cfg.getEntryBB();
-                // Push
+        // For now, we always require frame for closures
+        boolean requireFrame = doesItRequireFrame(scope, bindingHasEscaped);
+        boolean requireBinding = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
+
+        if (scope instanceof IRClosure || requireBinding || requireFrame) {
+            BasicBlock entryBB = cfg.getEntryBB();
+            Variable savedViz = null, savedFrame = null;
+            if (scope instanceof IRClosure) {
+                savedViz = scope.createTemporaryVariable();
+                savedFrame = scope.createTemporaryVariable();
+                entryBB.addInstr(new SaveBindingVisibilityInstr(savedViz));
+                entryBB.addInstr(new PushBlockFrameInstr(savedFrame, scope.getName()));
+                if (requireBinding) entryBB.addInstr(new PushBlockBindingInstr());
+                entryBB.addInstr(new UpdateBlockExecutionStateInstr(Self.SELF));
+                Signature sig = ((IRClosure)scope).getSignature();
+
+                // If it doesn't need any args, no arg preparation involved!
+                int arityValue = sig.arityValue();
+                if (arityValue != 0) {
+                    // Add the right kind of arg preparation instruction
+                    if (sig.isFixed()) {
+                        if (arityValue == 1) {
+                            entryBB.addInstr(new PrepareSingleBlockArgInstr());
+                        } else {
+                            entryBB.addInstr(new PrepareFixedBlockArgsInstr());
+                        }
+                    } else {
+                        entryBB.addInstr(new PrepareBlockArgsInstr(Operation.PREPARE_BLOCK_ARGS));
+                    }
+                }
+            } else {
                 if (requireFrame) entryBB.addInstr(new PushMethodFrameInstr(scope.getName()));
                 if (requireBinding) entryBB.addInstr(new PushMethodBindingInstr());
+            }
 
-                // SSS FIXME: We are doing this conservatively.
-                // Only scopes that have unrescued exceptions need a GEB.
-                //
-                // Allocate GEB if necessary for popping
-                BasicBlock geb = cfg.getGlobalEnsureBB();
-                if (geb == null) {
-                    Variable exc = scope.createTemporaryVariable();
-                    geb = new BasicBlock(cfg, Label.getGlobalEnsureBlockLabel());
-                    geb.addInstr(new ReceiveJRubyExceptionInstr(exc)); // JRuby Implementation exception handling
-                    geb.addInstr(new ThrowExceptionInstr(exc));
-                    cfg.addGlobalEnsureBB(geb);
-                }
+            // SSS FIXME: We are doing this conservatively.
+            // Only scopes that have unrescued exceptions need a GEB.
+            //
+            // Allocate GEB if necessary for popping
+            BasicBlock geb = cfg.getGlobalEnsureBB();
+            boolean gebProcessed = false;
+            if (geb == null) {
+                Variable exc = scope.createTemporaryVariable();
+                geb = new BasicBlock(cfg, Label.getGlobalEnsureBlockLabel());
+                geb.addInstr(new ReceiveJRubyExceptionInstr(exc)); // JRuby Implementation exception handling
+                geb.addInstr(new ThrowExceptionInstr(exc));
+                cfg.addGlobalEnsureBB(geb);
+            }
 
-                // Pop on all scope-exit paths
-                for (BasicBlock bb: cfg.getBasicBlocks()) {
-                    Instr i = null;
-                    ListIterator<Instr> instrs = bb.getInstrs().listIterator();
-                    while (instrs.hasNext()) {
-                        i = instrs.next();
-                        // Right now, we only support explicit call protocol on methods.
-                        // So, non-local returns and breaks don't get here.
-                        // Non-local-returns and breaks are tricky since they almost always
-                        // throw an exception and we don't multiple pops (once before the
-                        // return/break, and once when the exception is caught).
-                        if (!bb.isExitBB() && i instanceof ReturnBase) {
-                            if (requireBinding || requireFrame) {
-                                fixReturn(scope, (ReturnBase)i, instrs);
-                            }
-                            // Add before the break/return
-                            instrs.previous();
-                            if (requireBinding) instrs.add(new PopBindingInstr());
-                            if (requireFrame) instrs.add(new PopMethodFrameInstr());
-                            break;
+            // Pop on all scope-exit paths
+            for (BasicBlock bb: cfg.getBasicBlocks()) {
+                Instr i = null;
+                ListIterator<Instr> instrs = bb.getInstrs().listIterator();
+                while (instrs.hasNext()) {
+                    i = instrs.next();
+                    // Right now, we only support explicit call protocol on methods.
+                    // So, non-local returns and breaks don't get here.
+                    // Non-local-returns and breaks are tricky since they almost always
+                    // throw an exception and we don't multiple pops (once before the
+                    // return/break, and once when the exception is caught).
+                    if (!bb.isExitBB() && i instanceof ReturnBase) {
+                        if (requireBinding || requireFrame) {
+                            fixReturn(scope, (ReturnBase)i, instrs);
                         }
+                        // Add before the break/return
+                        instrs.previous();
+                        popSavedState(scope, requireBinding, requireFrame, savedViz, savedFrame, instrs);
+                        if (bb == geb) gebProcessed = true;
+                        break;
                     }
+                }
 
-                    if (bb.isExitBB() && !bb.isEmpty()) {
-                        // Last instr could be a return -- so, move iterator one position back
-                        if (i != null && i instanceof ReturnBase) {
-                            if (requireBinding || requireFrame) {
-                                fixReturn(scope, (ReturnBase)i, instrs);
-                            }
-                            instrs.previous();
+                if (bb.isExitBB() && !bb.isEmpty()) {
+                    // Last instr could be a return -- so, move iterator one position back
+                    if (i != null && i instanceof ReturnBase) {
+                        if (requireBinding || requireFrame) {
+                            fixReturn(scope, (ReturnBase)i, instrs);
                         }
-                        if (requireBinding) instrs.add(new PopBindingInstr());
-                        if (requireFrame) instrs.add(new PopMethodFrameInstr());
+                        instrs.previous();
                     }
+                    popSavedState(scope, requireBinding, requireFrame, savedViz, savedFrame, instrs);
+                    if (bb == geb) gebProcessed = true;
+                }
 
-                    if (bb == geb) {
-                        // Add before throw-exception-instr which would be the last instr
-                        if (i != null) {
-                            // Assumption: Last instr should always be a control-transfer instruction
-                            assert i.getOperation().transfersControl(): "Last instruction of GEB in scope: " + scope + " is " + i + ", not a control-xfer instruction";
-                            instrs.previous();
-                        }
-                        if (requireBinding) instrs.add(new PopBindingInstr());
-                        if (requireFrame) instrs.add(new PopMethodFrameInstr());
+                if (!gebProcessed && bb == geb) {
+                    // Add before throw-exception-instr which would be the last instr
+                    if (i != null) {
+                        // Assumption: Last instr should always be a control-transfer instruction
+                        assert i.getOperation().transfersControl(): "Last instruction of GEB in scope: " + scope + " is " + i + ", not a control-xfer instruction";
+                        instrs.previous();
                     }
+                    popSavedState(scope, requireBinding, requireFrame, savedViz, savedFrame, instrs);
                 }
             }
-
-            // This scope has an explicit call protocol flag now
-            scope.setExplicitCallProtocolFlag();
         }
 
+        // This scope has an explicit call protocol flag now
+        scope.setExplicitCallProtocolFlag();
+
         // LVA information is no longer valid after the pass
         // FIXME: Grrr ... this seems broken to have to create a new object to invalidate
         (new LiveVariableAnalysis()).invalidate(scope);

diff --git a/core/src/main/java/org/jruby/runtime/MixedModeIRBlockBody.java b/core/src/main/java/org/jruby/runtime/MixedModeIRBlockBody.java
@@ -89,6 +89,30 @@ public String getName() {
         return closure.getName();
     }
 
+    @Override
+    protected IRubyObject callDirect(ThreadContext context, Block block, IRubyObject[] args, Block blockArg) {
+        if (callCount >= 0) promoteToFullBuild(context);
+        CompiledIRBlockBody jittedBody = this.jittedBody;
+        if (jittedBody != null) {
+            return jittedBody.callDirect(context, block, args, blockArg);
+        }
+
+        context.setCurrentBlockType(Block.Type.PROC);
+        return Interpreter.INTERPRET_BLOCK(context, block, null, interpreterContext, args, block.getBinding().getMethod(), blockArg);
+    }
+
+    @Override
+    protected IRubyObject yieldDirect(ThreadContext context, Block block, IRubyObject[] args, IRubyObject self) {
+        if (callCount >= 0) promoteToFullBuild(context);
+        CompiledIRBlockBody jittedBody = this.jittedBody;
+        if (jittedBody != null) {
+            return jittedBody.yieldDirect(context, block, args, self);
+        }
+
+        context.setCurrentBlockType(Block.Type.NORMAL);
+        return Interpreter.INTERPRET_BLOCK(context, block, self, interpreterContext, args, block.getBinding().getMethod(), Block.NULL_BLOCK);
+    }
+
     protected IRubyObject commonYieldPath(ThreadContext context, Block block, IRubyObject[] args, IRubyObject self, Block blockArg) {
         if (callCount >= 0) promoteToFullBuild(context);
 

diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java
@@ -74,7 +74,7 @@ public boolean restKwargs() {
     public boolean hasRest() { return rest != Rest.NONE; }
 
     /**
-     * Are their an exact (fixed) number of parameters to this signature?
+     * Are there an exact (fixed) number of parameters to this signature?
      */
     public boolean isFixed() {
         return arityValue() >= 0;