OptimizeTempVarsPass is now just a collection of static methods and not inheriting from CompilerPass.

clonedInstrs is no longer a field in IRScope and is a local in prepareFullBuildCommon.

Author: enebo

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

@@ -2,6 +2,7 @@
 
 import java.util.EnumSet;
 import org.jruby.RubyInstanceConfig;
+import org.jruby.ir.instructions.Instr;
 import org.jruby.ir.listeners.IRScopeListener;
 import org.jruby.ir.listeners.InstructionsListener;
 import org.jruby.ir.operands.*;
@@ -31,7 +32,6 @@ public class IRManager {
 
     private final CompilerPass deadCodeEliminationPass = new DeadCodeElimination();
     private final CompilerPass optimizeDynScopesPass = new OptimizeDynScopesPass();
-    private final CompilerPass optimizeTempVarsPass = new OptimizeTempVarsPass();
     private final CompilerPass optimizeDelegationPass = new OptimizeDelegationPass();
 
     private int dummyMetaClassCount = 0;
@@ -219,9 +219,9 @@ public TemporaryLocalVariable newTemporaryLocalVariable(int index) {
         return tempVar;
     }
 
-    public void optimizeTemporaryVariablesIfEnabled(IRScope scope) {
+    public Instr[] optimizeTemporaryVariablesIfEnabled(IRScope scope, Instr[] instrs) {
         // FIXME: Make this check ir.passes and not run if ir.passes is set and does not contain opttempvars.
-        optimizeTempVarsPass.run(scope);
+        return OptimizeTempVarsPass.optimizeTmpVars(scope, instrs);
     }
 
     /**

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

@@ -96,8 +96,6 @@ public abstract class IRScope implements ParseResult {
     /** -X-C full interpretation OR JIT depends on this */
     protected FullInterpreterContext fullInterpreterContext;
 
-    private Instr[] clonedInstrs = null;
-
     protected int temporaryVariableIndex;
     protected int floatVariableIndex;
     protected int fixnumVariableIndex;
@@ -447,40 +445,31 @@ public InterpreterContext allocateInterpreterContext(List<Instr> instructions) {
         return interpreterContext;
     }
 
-    protected void cloneInstrs() {
-        cloneInstrs(new SimpleCloneInfo(this, false));
-    }
+    private Instr[] cloneInstrs() {
+        SimpleCloneInfo cloneInfo = new SimpleCloneInfo(this, false);
 
-    public void cloneInstrs(SimpleCloneInfo cloneInfo) {
         Instr[] instructions = interpreterContext.getInstructions();
         int length = instructions.length;
-        setClonedInstrs(new Instr[length]);
+        Instr[] newInstructions = new Instr[length];
 
         for (int i = 0; i < length; i++) {
-            clonedInstrs[i] = instructions[i].clone(cloneInfo);
+            newInstructions[i] = instructions[i].clone(cloneInfo);
         }
 
-        for (IRClosure cl: getClosures()) {
-            // Closure may have independently been promoted to full build already
-            if (cl.fullInterpreterContext == null) {
-                cl.cloneInstrs(cloneInfo.cloneForCloningClosure(cl));
-            }
-        }
+        return newInstructions;
     }
 
     protected void prepareFullBuildCommon() {
         // Clone instrs from startup interpreter so we do not swap out instrs out from under the
         // startup interpreter as we are building the full interpreter.
-        cloneInstrs();
+        Instr[] instrs = cloneInstrs();
 
         // This is a complicating pseudo-pass which needs to be run before CFG is generated.  This
         // necessitates us needing a clonedInstrs field on IRScope.  If we can rewrite this to a full
         // CFG using pass we can eliminate this intermediate save and field.
-        getManager().optimizeTemporaryVariablesIfEnabled(this);
+        instrs = getManager().optimizeTemporaryVariablesIfEnabled(this, instrs);
 
-        fullInterpreterContext = new FullInterpreterContext(this, clonedInstrs);
-
-        setClonedInstrs(null); // boo. vestigial temporal field...because of opt tmp vars pass.
+        fullInterpreterContext = new FullInterpreterContext(this, instrs);
     }
     /**
      * This initializes a more complete(full) InterpreterContext which if used in mixed mode will be
@@ -1036,12 +1025,4 @@ public void savePersistenceInfo(int offset, IRReaderDecoder file) {
 //        instructionsOffsetInfoPersistenceBuffer = offset;
 //        persistenceStore = file;
     }
-
-    public Instr[] getClonedInstrs() {
-        return clonedInstrs;
-    }
-
-    public void setClonedInstrs(Instr[] clonedInstrs) {
-        this.clonedInstrs = clonedInstrs;
-    }
 }

core/src/main/java/org/jruby/ir/passes/OptimizeTempVarsPass.java

@@ -1,35 +1,28 @@
 package org.jruby.ir.passes;
 
-import org.jruby.ir.IRClosure;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.ListIterator;
+import java.util.Map;
 import org.jruby.ir.IRScope;
 import org.jruby.ir.instructions.*;
 import org.jruby.ir.operands.ImmutableLiteral;
 import org.jruby.ir.operands.Operand;
 import org.jruby.ir.operands.TemporaryVariable;
 import org.jruby.ir.operands.Variable;
 
-import java.util.*;
-
-public class OptimizeTempVarsPass extends CompilerPass {
-    @Override
-    public String getLabel() {
-        return "Temporary Variable Reduction";
-    }
-
-    @Override
-    public Object execute(IRScope s, Object... data) {
-        optimizeTmpVars(s);
-
-        return null;
-    }
-
-    @Override
-    public boolean invalidate(IRScope s) {
-        // This runs after IR is built and before CFG is built.
-        // Not reversible in the form it is written right now.
-        return false;
-    }
 
+/**
+ * Takes multiple single def-use temporary variables and reduces them to share the same temp variable.
+ * This ends up reducing the amount of allocation and most likely helps hotspot warm up in some way quicker.
+ *
+ * This traditionally was a compiler pass (extends CompilerPass) but it is special in that it is the only
+ * pass which does not require any supplementary datastructures.  In fact, it cannot be run by the time
+ * a CFG is created.  So it was de-CompilerPassed and called directly.
+ */
+public class OptimizeTempVarsPass {
     private static void allocVar(Operand oldVar, IRScope s, List<TemporaryVariable> freeVarsList, Map<Operand, Operand> newVarMap) {
         // If we dont have a var mapping, get a new var -- try the free list first
         // and if none available, allocate a fresh one
@@ -43,8 +36,8 @@ private static void freeVar(TemporaryVariable newVar, List<TemporaryVariable> fr
         if (!freeVarsList.contains(newVar)) freeVarsList.add(0, newVar);
     }
 
-    private static void optimizeTmpVars(IRScope s) {
-        List<Instr> instructions = new ArrayList<>(Arrays.asList(s.getClonedInstrs()));
+    public static Instr[] optimizeTmpVars(IRScope s, Instr[] initialInstrs) {
+        List<Instr> instructions = new ArrayList<>(Arrays.asList(initialInstrs));
 
         // Pass 1: Analyze instructions and find use and def count of temporary variables
         Map<TemporaryVariable, Instr> tmpVarUses = new HashMap<>();
@@ -208,7 +201,7 @@ else if (i instanceof CopyInstr) {
         //
         // NOTE: It is sufficient to just track last use for renaming purposes.
         // At the first definition, we allocate a variable which then starts the live range
-        Map<TemporaryVariable, Integer> lastVarUseOrDef = new HashMap<TemporaryVariable, Integer>();
+        Map<TemporaryVariable, Integer> lastVarUseOrDef = new HashMap<>();
         int iCount = -1;
         for (Instr i: instructions) {
             iCount++;
@@ -238,8 +231,8 @@ else if (i instanceof CopyInstr) {
         // Pass 4: Reallocate temporaries based on last uses to minimize # of unique vars.
         // Replace all single use operands with constants they were assigned to.
         // Using operand -> operand signature because simplifyOperands works on operands
-        Map<Operand, Operand>   newVarMap    = new HashMap<Operand, Operand>();
-        List<TemporaryVariable> freeVarsList = new ArrayList<TemporaryVariable>();
+        Map<Operand, Operand>   newVarMap    = new HashMap<>();
+        List<TemporaryVariable> freeVarsList = new ArrayList<>();
         iCount = -1;
         s.resetTemporaryVariables();
 
@@ -273,6 +266,6 @@ else if (i instanceof CopyInstr) {
 
         Instr[] newInstrs = new Instr[instructions.size()];
         instructions.toArray(newInstrs);
-        s.setClonedInstrs(newInstrs);
+        return newInstrs;
     }
 }