Unbreak more of the unboxing code -- as far as it can get for now

* This will not run in this form since the unboxing code has been generalized to deal with fixnum / booleans instead of punting on booleans as I had done earlier. * In the current form, branches will also need to be specialized to work on unboxed booleans rather than boxed boolean literals (but which are lightweight and could be made to work without going all the way). To be investigated.
jruby · Jan 15, 2015 · 4142c8d · 4142c8d
1 parent cffeaab
commit 4142c8d
Showing 4 changed files with 32 additions and 30 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRScope.java b/core/src/main/java/org/jruby/ir/IRScope.java
@@ -8,6 +8,7 @@
 import org.jruby.ir.interpreter.InterpreterContext;
 import org.jruby.ir.operands.*;
 import org.jruby.ir.operands.Float;
+import org.jruby.ir.operands.Boolean;
 import org.jruby.ir.passes.*;
 import org.jruby.ir.persistence.IRReaderDecoder;
 import org.jruby.ir.representations.BasicBlock;
@@ -875,9 +876,8 @@ public TemporaryLocalVariable getNewTemporaryVariable(TemporaryVariableType type
                 return new TemporaryFixnumVariable(fixnumVariableIndex);
             }
             case BOOLEAN: {
-                // Shares var index with locals
-                temporaryVariableIndex++;
-                return new TemporaryBooleanVariable(temporaryVariableIndex);
+                booleanVariableIndex++;
+                return new TemporaryBooleanVariable(booleanVariableIndex);
             }
             case LOCAL: {
                 temporaryVariableIndex++;
@@ -909,7 +909,7 @@ public TemporaryLocalVariable getNewUnboxedVariable(Class type) {
             varType = TemporaryVariableType.FLOAT;
         } else if (type == Fixnum.class) {
             varType = TemporaryVariableType.FIXNUM;
-        } else if (type == UnboxedBoolean.class) {
+        } else if (type == Boolean.class) {
             varType = TemporaryVariableType.BOOLEAN;
         } else {
             varType = TemporaryVariableType.LOCAL;

diff --git a/core/src/main/java/org/jruby/ir/Operation.java b/core/src/main/java/org/jruby/ir/Operation.java
@@ -196,7 +196,7 @@ public enum Operation {
     IXOR(OpFlags.f_is_int_op),
     ISHL(OpFlags.f_is_int_op),
     ISHR(OpFlags.f_is_int_op),
-    IEQ(OpFlags.f_is_float_op),
+    IEQ(OpFlags.f_is_int_op),
     FADD(OpFlags.f_is_float_op),
     FSUB(OpFlags.f_is_float_op),
     FMUL(OpFlags.f_is_float_op),

diff --git a/core/src/main/java/org/jruby/ir/dataflow/analyses/UnboxableOpsAnalysisNode.java b/core/src/main/java/org/jruby/ir/dataflow/analyses/UnboxableOpsAnalysisNode.java
@@ -168,10 +168,16 @@ private Class getUnboxedResultType(Class operandType, String name) {
                 case '*' :
                 case '/' : return operandType == Float.class ? Float.class : operandType == Fixnum.class ? Fixnum.class : null;
                 case '>' :
-                case '<' : return operandType == Float.class || operandType == Fixnum.class ? UnboxedBoolean.class : null;
+                case '<' : return operandType == Float.class || operandType == Fixnum.class ? Boolean.class : null;
+                default  : return null;
             }
+        } else if (name.equals(">>") || name.equals("<<")) {
+            return Fixnum.class;
+        } else if (name.equals("==") || name.equals("===")) {
+            return Boolean.class;
+        } else {
+            return null;
         }
-        return null;
     }
 
     private Operation getUnboxedOp(Class unboxedType, String name) {
@@ -201,12 +207,10 @@ private Operation getUnboxedOp(Class unboxedType, String name) {
                     case '&' : return Operation.IAND;
                     case '^' : return Operation.IXOR;
                 }
-            } else if (name.length() == 2) {
+            } else {
                 if (name.equals(">>")) return Operation.ISHR;
                 if (name.equals("<<")) return Operation.ISHL;
-                if (name.equals("==")) return Operation.IEQ;
-            } else if (name.equals("===")) {
-                return Operation.IEQ;
+                if (name.equals("==") || name.equals("===")) return Operation.IEQ;
             }
         }
 
@@ -237,7 +241,7 @@ private void updateUnboxedVarsInfo(Instr i, UnboxState state, Variable dst, bool
             }
 
             // But, the unboxed forms themselves are still usable
-            // after this instruction -- we way just have boxed them
+            // after this instruction -- we may have boxed them
             // needlessly if the exception itself wasn't raised.
         } 
 
@@ -279,7 +283,6 @@ public void initSolution() {
 
     @Override
     public void applyTransferFunction(Instr i) {
-
         Variable dst = null;
         Class    dstType = Object.class; // default worst case assumption
         boolean  unboxedAndDirty = false;
@@ -414,7 +417,7 @@ private boolean matchingTypes(Class c, TemporaryVariableType t) {
         switch (t) {
         case FLOAT: return c == Float.class;
         case FIXNUM: return c == Fixnum.class;
-        case BOOLEAN: return c == UnboxedBoolean.class;
+        case BOOLEAN: return c == Boolean.class;
         default: return c != Float.class && c != Boolean.class && c != Fixnum.class;
         }
     }
@@ -440,7 +443,6 @@ public void boxVar(UnboxState state, Class reqdType, Map<Variable, TemporaryLoca
         TemporaryLocalVariable unboxedV = getUnboxedVar(reqdType, unboxMap, v);
         TemporaryVariableType vType = unboxedV.getType();
         if (vType == TemporaryVariableType.BOOLEAN) {
-            // boolean literals are lightweight enough that they dont need unboxed variants.
             newInstrs.add(new BoxBooleanInstr(v, unboxedV));
         } else if (vType == TemporaryVariableType.FLOAT) { // SSS FIXME: This is broken
             newInstrs.add(new BoxFloatInstr(v, unboxedV));
@@ -453,8 +455,7 @@ public void boxVar(UnboxState state, Class reqdType, Map<Variable, TemporaryLoca
 
     public void unboxVar(UnboxState state, Class reqdType, Map<Variable, TemporaryLocalVariable> unboxMap, Variable v, List<Instr> newInstrs) {
         Variable unboxedV = getUnboxedVar(reqdType, unboxMap, v);
-        if (reqdType == UnboxedBoolean.class) {
-            // boolean literals are lightweight enough that they dont need unboxed variants.
+        if (reqdType == Boolean.class) {
             newInstrs.add(new UnboxBooleanInstr(unboxedV, v));
         } else if (reqdType == Float.class) { // SSS FIXME: This is broken
             newInstrs.add(new UnboxFloatInstr(unboxedV, v));
@@ -482,7 +483,7 @@ private Operand unboxOperand(UnboxState state, Class reqdType, Map<Variable, Tem
                 return new UnboxedFloat(((Float)arg).getValue());
             } else if (arg instanceof Fixnum) {
                 return new UnboxedFixnum(((Fixnum)arg).getValue());
-            } else if (arg instanceof org.jruby.ir.operands.Boolean) {
+            } else if (arg instanceof Boolean) {
                 return new UnboxedBoolean(((Boolean)arg).isTrue());
             }
             // This has to be a known operand like (UnboxedBoolean, etc.)
@@ -684,7 +685,7 @@ public void unbox(Map<Variable, TemporaryLocalVariable> unboxMap) {
                     Operand o = ((ClosureAcceptingInstr)i).getClosureArg();
                     if (i instanceof CallBase && o == null) {
                         CallBase c = (CallBase)i;
-                        String m = c.getName();
+                        String   m = c.getName();
                         Operand  r = c.getReceiver();
                         if (dst != null && c.getArgsCount() == 1 && resemblesALUOp(m)) {
                             Operand a = c.getArg1();

diff --git a/core/src/main/java/org/jruby/ir/interpreter/Interpreter.java b/core/src/main/java/org/jruby/ir/interpreter/Interpreter.java
@@ -503,18 +503,19 @@ private static void processOtherOp(ThreadContext context, Instr instr, Operation
     private static IRubyObject interpret(ThreadContext context, IRubyObject self,
             InterpreterContext interpreterContext, RubyModule implClass,
             String name, IRubyObject[] args, Block block, Block.Type blockType) {
-        Instr[] instrs = interpreterContext.getInstructions();
-        Object[] temp           = interpreterContext.allocateTemporaryVariables();
-        double[] floats         = interpreterContext.allocateTemporaryFloatVariables();
-        long[]   fixnums        = interpreterContext.allocateTemporaryFixnumVariables();
-        boolean[]   booleans    = interpreterContext.allocateTemporaryBooleanVariables();
-        int      n              = instrs.length;
-        int      ipc            = 0;
-        Object   exception      = null;
+        Instr[]   instrs    = interpreterContext.getInstructions();
+        Object[]  temp      = interpreterContext.allocateTemporaryVariables();
+        double[]  floats    = interpreterContext.allocateTemporaryFloatVariables();
+        long[]    fixnums   = interpreterContext.allocateTemporaryFixnumVariables();
+        boolean[] booleans  = interpreterContext.allocateTemporaryBooleanVariables();
+        int       n         = instrs.length;
+        int       ipc       = 0;
+        Object    exception = null;
+
+        StaticScope  currScope = interpreterContext.getStaticScope();
         DynamicScope currDynScope = context.getCurrentScope();
-        StaticScope currScope = interpreterContext.getStaticScope();
-        IRScope scope = currScope.getIRScope();
-        boolean acceptsKeywordArgument = interpreterContext.receivesKeywordArguments();
+        IRScope      scope = currScope.getIRScope();
+        boolean      acceptsKeywordArgument = interpreterContext.receivesKeywordArguments();
 
         // Init profiling this scope
         boolean debug   = IRRuntimeHelpers.isDebug();