jruby · Oct 6, 2014 · Oct 6, 2014 · Oct 6, 2014 · Oct 6, 2014
diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/InterpretedIRMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/InterpretedIRMethod.java
@@ -140,7 +140,7 @@ public void ensureInstrsReady() {
         // Prepare method if not yet done so we know if the method has an explicit/implicit call protocol
         if (method.getInstrsForInterpretation() == null) {
             method.prepareForInterpretation(false);
-            this.pushScope = method.getFlags().contains(IRFlags.REQUIRES_DYNSCOPE) || !method.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
+            this.pushScope = !method.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
         }
     }
 

diff --git a/core/src/main/java/org/jruby/ir/IRFlags.java b/core/src/main/java/org/jruby/ir/IRFlags.java
@@ -55,4 +55,5 @@ public enum IRFlags {
     REQUIRES_VISIBILITY,          // callee may read/write caller's visibility
 
     DYNSCOPE_ELIMINATED,          // local var load/stores have been converted to tmp var accesses
+    REUSE_PARENT_DYNSCOPE,        // for clsoures -- reuse parent's dynscope
 }
diff --git a/core/src/main/java/org/jruby/ir/IRScope.java b/core/src/main/java/org/jruby/ir/IRScope.java
@@ -11,6 +11,8 @@
 import org.jruby.ir.passes.CompilerPass;
 import org.jruby.ir.passes.CompilerPassScheduler;
 import org.jruby.ir.passes.DeadCodeElimination;
+import org.jruby.ir.dataflow.analyses.StoreLocalVarPlacementProblem;
+import org.jruby.ir.dataflow.analyses.LiveVariablesProblem;
 import org.jruby.ir.passes.UnboxingPass;
 import org.jruby.ir.persistence.IRReaderDecoder;
 import org.jruby.ir.representations.BasicBlock;
@@ -451,7 +453,7 @@ public CFG buildCFG() {
     }
 
     protected void setCFG(CFG cfg) {
-       this.cfg = cfg;
+        this.cfg = cfg;
     }
 
     public CFG getCFG() {
@@ -517,19 +519,7 @@ public void setupRescueMap() {
         }
     }
 
-    private void runCompilerPasses(List<CompilerPass> passes) {
-        // SSS FIXME: Why is this again?  Document this weirdness!
-        // Forcibly clear out the shared eval-scope variable allocator each time this method executes
-        initEvalScopeVariableAllocator(true);
-
-        // SSS FIXME: We should configure different optimization levels
-        // and run different kinds of analysis depending on time budget.
-        // Accordingly, we need to set IR levels/states (basic, optimized, etc.)
-        // ENEBO: If we use a MT optimization mechanism we cannot mutate CFG
-        // while another thread is using it.  This may need to happen on a clone()
-        // and we may need to update the method to return the new method.  Also,
-        // if this scope is held in multiple locations how do we update all references?
-
+    private boolean isUnsafeScope() {
         boolean unsafeScope = false;
         if (flags.contains(HAS_END_BLOCKS) || this.isBeginEndBlock()) {
             unsafeScope = true;
@@ -545,12 +535,27 @@ private void runCompilerPasses(List<CompilerPass> passes) {
                 unsafeScope = true;
             }
         }
+        return unsafeScope;
+    }
+
+    private void runCompilerPasses(List<CompilerPass> passes) {
+        // SSS FIXME: Why is this again?  Document this weirdness!
+        // Forcibly clear out the shared eval-scope variable allocator each time this method executes
+        initEvalScopeVariableAllocator(true);
+
+        // SSS FIXME: We should configure different optimization levels
+        // and run different kinds of analysis depending on time budget.
+        // Accordingly, we need to set IR levels/states (basic, optimized, etc.)
+        // ENEBO: If we use a MT optimization mechanism we cannot mutate CFG
+        // while another thread is using it.  This may need to happen on a clone()
+        // and we may need to update the method to return the new method.  Also,
+        // if this scope is held in multiple locations how do we update all references?
 
         // All passes are disabled in scopes where BEGIN and END scopes might
         // screw around with escaped variables. Optimizing for them is not
         // worth the effort. It is simpler to just go fully safe in scopes
         // influenced by their presence.
-        if (unsafeScope) {
+        if (isUnsafeScope()) {
             passes = getManager().getSafePasses(this);
         }
 
@@ -567,17 +572,23 @@ private void runCompilerPasses(List<CompilerPass> passes) {
     }
 
     private void runDeadCodeAndVarLoadStorePasses() {
-        // For methods that don't require a dynamic scope,
+        // For scopes that don't require a dynamic scope,
         // inline-add lvar loads/store to tmp-var loads/stores.
-        if (!flags.contains(HAS_END_BLOCKS) && !flags.contains(REQUIRES_DYNSCOPE)) {
+        if (!isUnsafeScope() && !flags.contains(REQUIRES_DYNSCOPE)) {
             CompilerPass pass;
             pass = new DeadCodeElimination();
             if (pass.previouslyRun(this) == null) {
                 pass.run(this);
             }
+
+            // This will run the simplified version of the pass
+            // that doesn't require dataflow analysis and hence
+            // can run on closures independent of enclosing scopes.
             pass = new AddLocalVarLoadStoreInstructions();
             if (pass.previouslyRun(this) == null) {
-                pass.run(this);
+                ((AddLocalVarLoadStoreInstructions)pass).eliminateLocalVars(this);
+                setDataFlowSolution(StoreLocalVarPlacementProblem.NAME, new StoreLocalVarPlacementProblem());
+                setDataFlowSolution(LiveVariablesProblem.NAME, null);
             }
         }
     }
@@ -730,7 +741,7 @@ public void computeScopeFlags() {
         }
 
         // Compute flags for nested closures (recursively) and set derived flags.
-        for (IRClosure cl : getClosures()) {
+        for (IRClosure cl: getClosures()) {
             cl.computeScopeFlags();
             if (cl.usesEval()) {
                 flags.add(CAN_RECEIVE_BREAKS);
@@ -749,8 +760,8 @@ public void computeScopeFlags() {
             }
         }
 
-        if (!(this instanceof IRMethod)
-            || flags.contains(CAN_RECEIVE_BREAKS)
+        if (flags.contains(CAN_RECEIVE_BREAKS)
+            || flags.contains(HAS_NONLOCAL_RETURNS)
             || flags.contains(CAN_RECEIVE_NONLOCAL_RETURNS)
             || flags.contains(BINDING_HAS_ESCAPED)
                // SSS FIXME: checkArity for keyword args
@@ -1087,6 +1098,7 @@ protected void depends(Object obj) {
                 "up wrong.  Use depends(build()) not depends(build).";
     }
 
+    // SSS FIXME: Why do we have cfg() with this assertion and a getCFG() without an assertion??
     public CFG cfg() {
         assert cfg != null: "Trying to access build before build started";
         return cfg;

diff --git a/core/src/main/java/org/jruby/ir/IRScriptBody.java b/core/src/main/java/org/jruby/ir/IRScriptBody.java
@@ -104,7 +104,9 @@ public IRubyObject interpret(ThreadContext context, IRubyObject self) {
         IRubyObject retVal;
 
         scope.setModule(currModule);
-        context.preMethodScopeOnly(currModule, scope);
+        if (!this.flags.contains(IRFlags.DYNSCOPE_ELIMINATED)) {
+            context.preMethodScopeOnly(currModule, scope);
+        }
         context.setCurrentVisibility(Visibility.PRIVATE);
 
         try {
@@ -116,7 +118,9 @@ public IRubyObject interpret(ThreadContext context, IRubyObject self) {
         } catch (IRBreakJump bj) {
             throw IRException.BREAK_LocalJumpError.getException(context.runtime);
         } finally {
-            context.popScope();
+            if (!this.flags.contains(IRFlags.DYNSCOPE_ELIMINATED)) {
+                context.popScope();
+            }
         }
 
         return retVal;

diff --git a/core/src/main/java/org/jruby/ir/dataflow/DataFlowProblem.java b/core/src/main/java/org/jruby/ir/dataflow/DataFlowProblem.java
@@ -141,7 +141,7 @@ private void buildFlowGraph() {
         // Since LVA runs analyses on nested closures, and dependencies aren't
         // checked on nested scopes, it can happen that for closures,
         // the cfg hasn't been built yet.
-        if (scope.cfg() == null) {
+        if (scope.getCFG() == null) {
             scope.buildCFG();
         }
 

diff --git a/core/src/main/java/org/jruby/ir/dataflow/analyses/LiveVariableNode.java b/core/src/main/java/org/jruby/ir/dataflow/analyses/LiveVariableNode.java
@@ -108,9 +108,12 @@ public void applyTransferFunction(Instr i) {
             if (o != null && o instanceof WrappedIRClosure) {
                 IRClosure cl = ((WrappedIRClosure)o).getClosure();
                 LiveVariablesProblem cl_lvp = (LiveVariablesProblem) cl.getDataFlowSolution(DataFlowConstants.LVP_NAME);
+                boolean needsInit = false;
                 if (cl_lvp == null) {
                     cl_lvp = new LiveVariablesProblem(cl, problem.getNonSelfLocalVars());
                     cl.setDataFlowSolution(cl_lvp.getName(), cl_lvp);
+                    cl.computeScopeFlags();
+                    needsInit = true;
                 }
 
                 // Add all living local variables.
@@ -143,6 +146,12 @@ public void applyTransferFunction(Instr i) {
                 // SSS FIXME: Think through this .. Is there any way out of having
                 // to recompute the entire lva for the closure each time through?
                 cl_lvp.setVarsLiveOnScopeExit(liveVars);
+                if (needsInit) {
+                    // Init DF vars from this set
+                    for (Variable v: liveVars) {
+                        cl_lvp.addDFVar(v);
+                    }
+                }
                 cl_lvp.compute_MOP_Solution();
 
                 // Check if liveOnScopeEntry added new vars -- if so, rerun.

diff --git a/core/src/main/java/org/jruby/ir/dataflow/analyses/StoreLocalVarPlacementNode.java b/core/src/main/java/org/jruby/ir/dataflow/analyses/StoreLocalVarPlacementNode.java
@@ -243,6 +243,9 @@ public boolean addStores(Map<Operand, Operand> varRenameMap, Set<LocalVariable>
 
                 if (!basicBlock.isExitBB()) {
                     LiveVariablesProblem lvp = (LiveVariablesProblem)scope.getDataFlowSolution(DataFlowConstants.LVP_NAME);
+                    if (lvp == null) {
+                        System.out.println("LVP missing for " + scope);
+                    }
                     java.util.Collection<LocalVariable> liveVars = lvp.getVarsLiveOnScopeExit();
                     if (liveVars != null) {
                         dirtyVars.retainAll(liveVars); // Intersection with variables live on exit from the scope

diff --git a/core/src/main/java/org/jruby/ir/instructions/BreakInstr.java b/core/src/main/java/org/jruby/ir/instructions/BreakInstr.java
@@ -56,6 +56,7 @@ public Operand[] getOperands() {
     @Override
     public boolean computeScopeFlags(IRScope scope) {
         scope.getFlags().add(IRFlags.HAS_BREAK_INSTRS);
+        scope.getFlags().add(IRFlags.REQUIRES_DYNSCOPE);
         return true;
     }
 

diff --git a/core/src/main/java/org/jruby/ir/instructions/LoadLocalVarInstr.java b/core/src/main/java/org/jruby/ir/instructions/LoadLocalVarInstr.java
@@ -18,7 +18,7 @@ public class LoadLocalVarInstr extends Instr implements ResultInstr, FixedArityI
     /** This is the variable that is being loaded from the scope.  This variable
      * doesn't participate in the computation itself.  We just use it as a proxy for
      * its (a) name (b) offset (c) scope-depth. */
-    private final LocalVariable lvar;
+    private LocalVariable lvar;
 
     public LoadLocalVarInstr(IRScope scope, TemporaryLocalVariable result, LocalVariable lvar) {
         super(Operation.BINDING_LOAD);
@@ -49,6 +49,11 @@ public void updateResult(Variable v) {
         this.result = (TemporaryLocalVariable)v;
     }
 
+    // SSS FIXME: This feels dirty
+    public void decrementLVarScopeDepth() {
+        this.lvar = lvar.cloneForDepth(lvar.getScopeDepth()-1);
+    }
+
     @Override
     public String toString() {
         return result + " = load_lvar(" + scope.getName() + ", " + lvar + ")";

diff --git a/core/src/main/java/org/jruby/ir/instructions/StoreLocalVarInstr.java b/core/src/main/java/org/jruby/ir/instructions/StoreLocalVarInstr.java
@@ -21,7 +21,7 @@ public class StoreLocalVarInstr extends Instr implements FixedArityInstr {
     /** This is the variable that is being stored into in this scope.  This variable
      * doesn't participate in the computation itself.  We just use it as a proxy for
      * its (a) name (b) offset (c) scope-depth. */
-    private final LocalVariable lvar;
+    private LocalVariable lvar;
 
     public StoreLocalVarInstr(Operand value, IRScope scope, LocalVariable lvar) {
         super(Operation.BINDING_STORE);
@@ -54,6 +54,11 @@ public LocalVariable getLocalVar() {
         return lvar;
     }
 
+    // SSS FIXME: This feels dirty
+    public void decrementLVarScopeDepth() {
+        this.lvar = lvar.cloneForDepth(lvar.getScopeDepth()-1);
+    }
+
     @Override
     public Instr cloneForInlining(InlinerInfo ii) {
         // SSS FIXME: Do we need to rename lvar really?  It is just a name-proxy!

diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java
@@ -91,8 +91,7 @@ public Object execute(IRScope scope, Object... data) {
                 }
             }
 
-            boolean requireBinding = bindingHasEscaped || scopeHasLocalVarStores
-                || (scope.getFlags().contains(IRFlags.REQUIRES_DYNSCOPE) || !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED));
+            boolean requireBinding = bindingHasEscaped || scopeHasLocalVarStores || !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
 
             // FIXME: Why do we need a push/pop for frame & binding for scopes with unrescued exceptions??
             // 1. I think we need a different check for frames -- it is NOT scopeHasUnrescuedExceptions

diff --git a/core/src/main/java/org/jruby/ir/passes/AddLocalVarLoadStoreInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddLocalVarLoadStoreInstructions.java
@@ -8,6 +8,8 @@
 import org.jruby.ir.dataflow.analyses.StoreLocalVarPlacementProblem;
 import org.jruby.ir.instructions.Instr;
 import org.jruby.ir.instructions.ResultInstr;
+import org.jruby.ir.instructions.LoadLocalVarInstr;
+import org.jruby.ir.instructions.StoreLocalVarInstr;
 import org.jruby.ir.operands.LocalVariable;
 import org.jruby.ir.operands.Operand;
 import org.jruby.ir.operands.Variable;
@@ -35,11 +37,85 @@ private void setupLocalVarReplacement(LocalVariable v, IRScope s, Map<Operand, O
          if (varRenameMap.get(v) == null) varRenameMap.put(v, s.getNewTemporaryVariableFor(v));
     }
 
+    private void decrementScopeDepth(LocalVariable v, IRScope s, Map<Operand, Operand> varRenameMap) {
+         if (varRenameMap.get(v) == null) varRenameMap.put(v, v.cloneForDepth(v.getScopeDepth()-1));
+    }
+
+    public void eliminateLocalVars(IRScope s) {
+        Map<Operand, Operand> varRenameMap = new HashMap<Operand, Operand>();
+        // Record the fact that we eliminated the scope
+        s.getFlags().add(IRFlags.DYNSCOPE_ELIMINATED);
+
+        // Since the scope does not require a binding, no need to do
+        // any analysis. It is sufficient to rename all local var uses
+        // with a temporary variable.
+        boolean parentScopeNeeded = false;
+        for (BasicBlock b: s.getCFG().getBasicBlocks()) {
+            for (Instr i: b.getInstrs()) {
+                if (i instanceof ResultInstr) {
+                    Variable v = ((ResultInstr) i).getResult();
+                    // %self is local to every scope and never crosses scope boundaries and need not be spilled/refilled
+                    if (v instanceof LocalVariable && !v.isSelf()) {
+                        LocalVariable lv = (LocalVariable)v;
+                        if (lv.getScopeDepth() == 0) {
+                            // Make sure there is a replacement tmp-var allocated for lv
+                            setupLocalVarReplacement(lv, s, varRenameMap);
+                        } else {
+                            parentScopeNeeded = true;
+                            decrementScopeDepth(lv, s, varRenameMap);
+                        }
+                    }
+                }
+
+                for (Variable v : i.getUsedVariables()) {
+                    if (v instanceof LocalVariable && !v.isSelf()) {
+                        LocalVariable lv = (LocalVariable)v;
+                        if (lv.getScopeDepth() == 0) {
+                            // Make sure there is a replacement tmp-var allocated for lv
+                            setupLocalVarReplacement(lv, s, varRenameMap);
+                        } else {
+                            // SSS FIXME: Ugly/Dirty! Some abstraction is broken.
+                            if (i instanceof LoadLocalVarInstr) { 
+                                LoadLocalVarInstr llvi = (LoadLocalVarInstr)i;
+                                if (llvi.getLocalVar() == lv) {
+                                    llvi.decrementLVarScopeDepth();
+                                }
+                            } else if (i instanceof StoreLocalVarInstr) {
+                                StoreLocalVarInstr slvi = (StoreLocalVarInstr)i;
+                                if (slvi.getLocalVar() == lv) {
+                                    slvi.decrementLVarScopeDepth();
+                                }
+                            }
+
+                            parentScopeNeeded = true;
+                            decrementScopeDepth(lv, s, varRenameMap);
+                        }
+                    }
+                }
+            }
+        }
+
+        if (parentScopeNeeded) {
+            s.getFlags().add(IRFlags.REUSE_PARENT_DYNSCOPE);
+        }
+
+        // Rename all local var uses with their tmp-var stand-ins
+        for (BasicBlock b: s.getCFG().getBasicBlocks()) {
+            for (Instr i: b.getInstrs()) i.renameVars(varRenameMap);
+        }
+    }
+
     @Override
     public Object execute(IRScope s, Object... data) {
         StoreLocalVarPlacementProblem slvp = new StoreLocalVarPlacementProblem();
-        Map<Operand, Operand> varRenameMap = new HashMap<Operand, Operand>();
-        if (s.bindingHasEscaped() || s instanceof IRClosure) {
+
+        // Make sure flags are computed
+        s.computeScopeFlags();
+
+        if (!s.getFlags().contains(IRFlags.REQUIRES_DYNSCOPE) && (data.length == 0 || data[0] == false)) {
+            eliminateLocalVars(s);
+        } else {
+            Map<Operand, Operand> varRenameMap = new HashMap<Operand, Operand>();
             // 1. Figure out required stores
             // 2. Add stores
             // 3. Figure out required loads
@@ -57,43 +133,21 @@ public Object execute(IRScope s, Object... data) {
             llvp.setup(s);
             llvp.compute_MOP_Solution();
 
-            // Add loads,
+            // Add loads
             llvp.addLoads(varRenameMap);
-        } else {
-            // Record the fact that we eliminated the scope
-            s.getFlags().add(IRFlags.DYNSCOPE_ELIMINATED);
 
-            // Since the scope does not require a binding, no need to do
-            // any analysis. It is sufficient to rename all local var uses
-            // with a temporary variable.
+            // Rename all local var uses with their tmp-var stand-ins
             for (BasicBlock b: s.getCFG().getBasicBlocks()) {
-                for (Instr i: b.getInstrs()) {
-                    if (i instanceof ResultInstr) {
-                        Variable v = ((ResultInstr) i).getResult();
-                        // %self is local to every scope and never crosses scope boundaries and need not be spilled/refilled
-                        if (v instanceof LocalVariable && !v.isSelf()) {
-                            // Make sure there is a replacement tmp-var allocated for lv
-                            setupLocalVarReplacement((LocalVariable)v, s, varRenameMap);
-                        }
-                    }
-                    for (Variable v : i.getUsedVariables()) {
-                        if (v instanceof LocalVariable && !v.isSelf()) {
-                            setupLocalVarReplacement((LocalVariable)v, s, varRenameMap);
-                        }
-                    }
-                }
+                for (Instr i: b.getInstrs()) i.renameVars(varRenameMap);
             }
-        }
 
-        // Rename all local var uses with their tmp-var stand-ins
-        for (BasicBlock b: s.getCFG().getBasicBlocks()) {
-            for (Instr i: b.getInstrs()) i.renameVars(varRenameMap);
+            // Run on all nested closures.
+            //
+            // In the current implementation, nested scopes are processed independently (unlike Live Variable Analysis)
+            // However, since this pass requires LVA information, in reality, we cannot run
+            for (IRClosure c: s.getClosures()) execute(c, true);
         }
 
-        // Run on all nested closures.
-        // In the current implementation, nested scopes are processed independently (unlike Live Variable Analysis)
-        for (IRClosure c: s.getClosures()) execute(c);
-
         s.setDataFlowSolution(StoreLocalVarPlacementProblem.NAME, slvp);
 
         // LVA information is no longer valid after the pass

diff --git a/core/src/main/java/org/jruby/ir/passes/DeadCodeElimination.java b/core/src/main/java/org/jruby/ir/passes/DeadCodeElimination.java
@@ -27,10 +27,18 @@ public Object execute(IRScope scope, Object... data) {
             run(cl, true);
         }
 
+        scope.setDataFlowSolution("DCE", scope.getDataFlowSolution(LiveVariablesProblem.NAME));
+
         return true;
     }
 
+    @Override
+    public Object previouslyRun(IRScope scope) {
+        return scope.getDataFlowSolution("DCE");
+    }
+
+    @Override
     public void invalidate(IRScope scope) {
-        // FIXME: Can we reset this?
+        scope.setDataFlowSolution("DCE", null);
     }
 }
diff --git a/core/src/main/java/org/jruby/ir/passes/LiveVariableAnalysis.java b/core/src/main/java/org/jruby/ir/passes/LiveVariableAnalysis.java
@@ -1,9 +1,22 @@
 package org.jruby.ir.passes;
 
+import org.jruby.ir.IRClosure;
+import org.jruby.ir.IREvalScript;
 import org.jruby.ir.IRScope;
+import org.jruby.ir.instructions.ClosureAcceptingInstr;
+import org.jruby.ir.instructions.Instr;
+import org.jruby.ir.instructions.ResultInstr;
+import org.jruby.ir.operands.LocalVariable;
+import org.jruby.ir.operands.Operand;
+import org.jruby.ir.operands.Variable;
+import org.jruby.ir.operands.WrappedIRClosure;
+import org.jruby.ir.representations.BasicBlock;
 import org.jruby.ir.dataflow.analyses.LiveVariablesProblem;
 
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.List;
 
 public class LiveVariableAnalysis extends CompilerPass {
@@ -24,11 +37,60 @@ public Object previouslyRun(IRScope scope) {
         return scope.getDataFlowSolution(LiveVariablesProblem.NAME);
     }
 
+    private void collectNonLocalDirtyVars(IRClosure cl, Set<LocalVariable> vars, int minDepth) {
+        for (BasicBlock bb: cl.cfg().getBasicBlocks()) {
+            for (Instr i: bb.getInstrs()) {
+                // Collect local vars belonging to an outer scope dirtied here
+                if (i instanceof ResultInstr) {
+                    Variable res = ((ResultInstr)i).getResult();
+                    if (res instanceof LocalVariable && ((LocalVariable)res).getScopeDepth() > minDepth) {
+                        vars.add((LocalVariable)res);
+                    }
+                }
+
+                // When encountering nested closures, increase minDepth by 1
+                // so that we continue to collect vars belong to outer scopes.
+                if (i instanceof ClosureAcceptingInstr) {
+                    Operand clArg = ((ClosureAcceptingInstr)i).getClosureArg();
+                    if (clArg instanceof WrappedIRClosure) {
+                        collectNonLocalDirtyVars(((WrappedIRClosure)clArg).getClosure(), vars, minDepth+1);
+                    }
+                }
+            }
+        }
+    }
+
     @Override
     public Object execute(IRScope scope, Object... data) {
+        // Should never be invoked directly on IRClosures
+        // if (scope instanceof IRClosure && !(scope instanceof IREvalScript)) {
+        //     System.out.println("Hmm .. should not run lvp directly on closure scope: " + scope);
+        // }
+
+        // Make sure flags are computed
+        scope.computeScopeFlags();
+
         LiveVariablesProblem lvp = new LiveVariablesProblem(scope);
-        lvp.compute_MOP_Solution();
 
+        if (scope instanceof IRClosure) {
+            // Go conservative! Normally, closure scopes are analyzed
+            // in the context of their outermost method scopes where we
+            // have better knowledge of aliveness in that global context.
+            //
+            // But, if we are analyzing closures standalone, we have to
+            // conservatively assume that any dirtied variables that
+            // belong to an outer scope are live on exit.
+            Set<LocalVariable> nlVars = new HashSet<LocalVariable>();
+            collectNonLocalDirtyVars((IRClosure)scope, nlVars, 0);
+
+            // Init DF vars from this set
+            for (Variable v: nlVars) {
+                lvp.addDFVar(v);
+            }
+            lvp.setVarsLiveOnScopeExit(nlVars);
+        }
+
+        lvp.compute_MOP_Solution();
         scope.setDataFlowSolution(LiveVariablesProblem.NAME, lvp);
 
         return lvp;

diff --git a/core/src/main/java/org/jruby/ir/passes/LocalOptimizationPass.java b/core/src/main/java/org/jruby/ir/passes/LocalOptimizationPass.java
@@ -39,7 +39,9 @@ public Object execute(IRScope s, Object... data) {
             runLocalOptsOnInstrList(s, b.getInstrs().listIterator(), false);
         }
 
-        // Only after running local opts, compute various execution scope flags
+        // SSS FIXME: What is this about? 
+        // Why 'Only after running local opts'?  Figure out and document.
+        // Only after running local opts, compute various execution scope flags.
         s.computeScopeFlags();
 
         // Mark done

diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -701,7 +701,7 @@ public static RubyModule findInstanceMethodContainer(ThreadContext context, Dyna
                                 return (RubyModule) self;
 
                             default:
-                                throw new RuntimeException("Should not get here!");
+                                throw new RuntimeException("Should not get here! scopeType is " + scopeType);
                         }
                     }
                     break;

diff --git a/core/src/main/java/org/jruby/runtime/InterpretedIRBlockBody.java b/core/src/main/java/org/jruby/runtime/InterpretedIRBlockBody.java
@@ -6,19 +6,47 @@
 import org.jruby.runtime.Binding;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.ir.IRClosure;
+import org.jruby.ir.IRFlags;
 import org.jruby.ir.interpreter.Interpreter;
+import org.jruby.ir.representations.CFG;
+import org.jruby.ir.runtime.IRRuntimeHelpers;
 import org.jruby.runtime.Block.Type;
 import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.util.log.Logger;
+import org.jruby.util.log.LoggerFactory;
 
 public class InterpretedIRBlockBody extends IRBlockBody {
+    private static final Logger LOG = LoggerFactory.getLogger("InterpretedIRBlockBody");
     protected final IRClosure closure;
+    protected boolean pushScope;
+    protected boolean reuseParentScope;
 
     public InterpretedIRBlockBody(IRClosure closure, Arity arity, int argumentType) {
         super(closure.getStaticScope(), closure.getParameterList(), closure.getFileName(), closure.getLineNumber(), arity);
         this.closure = closure;
+        this.pushScope = true;
+        this.reuseParentScope = false;
+    }
+
+    public void ensureInstrsReady(Block.Type blockType) {
+        // Prepare closure if not yet done so we know if the method requires a dynscope or not
+        if (closure.getInstrsForInterpretation() == null) {
+            closure.prepareForInterpretation(blockType == Block.Type.LAMBDA);
+            this.pushScope = !closure.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
+            this.reuseParentScope = closure.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE);
+            if (IRRuntimeHelpers.isDebug()) {
+                LOG.info("Executing '" + closure + "'");
+                // The base IR may not have been processed yet
+                CFG cfg = closure.getCFG();
+                LOG.info("Graph:\n" + cfg.toStringGraph());
+                LOG.info("CFG:\n" + cfg.toStringInstrs());
+            }
+        }
     }
 
     protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args, IRubyObject self, Binding binding, Type type, Block block) {
+        ensureInstrsReady(type);
+
         // SSS: Important!  Use getStaticScope() to use a copy of the static-scope stored in the block-body.
         // Do not use 'closure.getStaticScope()' -- that returns the original copy of the static scope.
         // This matters because blocks created for Thread bodies modify the static-scope field of the block-body
@@ -34,17 +62,21 @@ protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args,
             self = useBindingSelf(binding);
         }
 
-        DynamicScope newScope = null;
-        DynamicScope prevScope = binding.getDynamicScope();
 
         // SSS FIXME: Maybe, we should allocate a NoVarsScope/DummyScope for for-loop bodies because the static-scope here
         // probably points to the parent scope? To be verified and fixed if necessary. There is no harm as it is now. It
         // is just wasteful allocation since the scope is not used at all.
 
         // Pass on eval state info to the dynamic scope and clear it on the block-body
-        newScope  = DynamicScope.newDynamicScope(getStaticScope(), prevScope, this.evalType.get());
+        DynamicScope prevScope = binding.getDynamicScope();
+        if (this.pushScope) {
+            context.pushScope(DynamicScope.newDynamicScope(getStaticScope(), prevScope, this.evalType.get()));
+        } else if (this.reuseParentScope) {
+            // Reuse!
+            // We can avoid the push only if surrounding vars aren't referenced!
+            context.pushScope(prevScope);
+        }
         this.evalType.set(EvalType.NONE);
-        context.pushScope(newScope);
 
         try {
             return Interpreter.INTERPRET_BLOCK(context, self, closure, args, binding.getMethod(), block, type);
@@ -54,7 +86,11 @@ protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args,
             // Ex: eval("...", foo.instance_eval { binding })
             // The dyn-scope used for binding needs to have its eval-type set to INSTANCE_EVAL
             binding.getFrame().setVisibility(oldVis);
-            context.postYield(binding, prevFrame);
+            if (this.pushScope || this.reuseParentScope) {
+                context.postYield(binding, prevFrame);
+            } else {
+                context.postYieldNoScope(prevFrame);
+            }
         }
     }
 }