Fixes #4425. Code behavior changes after being JITted at runtime

Revert "Address FIXMEs in LocalOptimizationPass" This reverts commit d41fac5. lvars were getting constant propagated via valueMap...
jruby · Jan 9, 2017 · e136ac1 · e136ac1
1 parent 2d090fb
commit e136ac1
Showing 1 changed file with 29 additions and 17 deletions.
diff --git a/core/src/main/java/org/jruby/ir/passes/LocalOptimizationPass.java b/core/src/main/java/org/jruby/ir/passes/LocalOptimizationPass.java
@@ -8,8 +8,6 @@
 import org.jruby.ir.operands.Operand;
 import org.jruby.ir.operands.Variable;
 import org.jruby.ir.representations.BasicBlock;
-import org.jruby.ir.instructions.ClosureAcceptingInstr;
-import org.jruby.ir.operands.WrappedIRClosure;
 
 import java.util.*;
 
@@ -107,16 +105,6 @@ public static Instr optInstr(IRScope s, Instr instr, Map<Operand,Operand> valueM
         return newInstr;
     }
 
-    private static boolean isDataflowBarrier(Instr i, IRScope s) {
-        boolean reset = false;
-        if (!i.isDead() && i instanceof ClosureAcceptingInstr) {
-            Operand o = ((ClosureAcceptingInstr)i).getClosureArg();
-            reset = s.bindingHasEscaped() || (o != null && o instanceof WrappedIRClosure);
-        }
-
-        return reset;
-    }
-
     public static void runLocalOptsOnInstrArray(IRScope s, Instr[] instrs) {
         // Reset value map if this instruction is the start/end of a basic block
         Map<Operand,Operand> valueMap = new HashMap<>();
@@ -128,10 +116,21 @@ public static void runLocalOptsOnInstrArray(IRScope s, Instr[] instrs) {
                 instrs[i] = newInstr;
             }
 
-            // Reset simplification info if this starts/ends a basic block
-            // or if the instr is a dataflow barrier.
+            // If the call has been optimized away in the previous step, it is no longer a hard boundary for opts!
+            //
+            // Right now, calls are considered hard boundaries for optimization and
+            // information cannot be propagated across them!
+            //
+            // SSS FIXME: Rather than treat all calls with a broad brush, what we need
+            // is to capture different attributes about a call :
+            //   - uses closures
+            //   - known call target
+            //   - can modify scope,
+            //   - etc.
+            //
+            // This information is present in instruction flags on CallBase. Use it!
             Operation iop = instr.getOperation();
-            if (iop.startsBasicBlock() || iop.endsBasicBlock() || isDataflowBarrier(instr, s)) {
+            if (iop.startsBasicBlock() || iop.endsBasicBlock() || (iop.isCall() && !instr.isDead())) {
                 valueMap = new HashMap<>();
                 simplificationMap = new HashMap<>();
             }
@@ -152,8 +151,21 @@ public static void runLocalOptsOnBasicBlock(IRScope s, BasicBlock b) {
                 instrs.set(newInstr);
             }
 
-            // Reset simplification info if this is a dataflow barrier.
-            if (isDataflowBarrier(instr, s)) {
+            // If the call has been optimized away in the previous step, it is no longer a hard boundary for opts!
+            //
+            // Right now, calls are considered hard boundaries for optimization and
+            // information cannot be propagated across them!
+            //
+            // SSS FIXME: Rather than treat all calls with a broad brush, what we need
+            // is to capture different attributes about a call :
+            //   - uses closures
+            //   - known call target
+            //   - can modify scope,
+            //   - etc.
+            //
+            // This information is present in instruction flags on CallBase. Use it!
+            Operation iop = instr.getOperation();
+            if (iop.isCall() && !instr.isDead()) {
                 valueMap = new HashMap<>();
                 simplificationMap = new HashMap<>();
             }