Convert Range operand into a BuildRangeInstr

* Ranges can throw exceptions during constructions and hence cannot be operands (which are meant to always succeed and be exception-free). * Eliminates the interpreter warning previously seen during rubyspec runs. * Persistence code needs update to reflect the changes.
jruby · Oct 30, 2014 · 816e3e2 · 816e3e2
1 parent 7e5e119
commit 816e3e2
Showing 9 changed files with 54 additions and 53 deletions.
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -2074,7 +2074,9 @@ public String buildType(Node typeNode) {
     }
 
     public Operand buildDot(final DotNode dotNode, IRScope s) {
-        return copyAndReturnValue(s, new Range(build(dotNode.getBeginNode(), s), build(dotNode.getEndNode(), s), dotNode.isExclusive()));
+        Variable res = s.createTemporaryVariable();
+        addInstr(s, new BuildRangeInstr(res, build(dotNode.getBeginNode(), s), build(dotNode.getEndNode(), s), dotNode.isExclusive()));
+        return res;
     }
 
     private Operand dynamicPiece(Node pieceNode, IRScope s) {

diff --git a/core/src/main/java/org/jruby/ir/IRVisitor.java b/core/src/main/java/org/jruby/ir/IRVisitor.java
@@ -40,6 +40,7 @@ private void error(Object object) {
     public void BuildCompoundArrayInstr(BuildCompoundArrayInstr instr) { error(instr); }
     public void BuildCompoundStringInstr(BuildCompoundStringInstr instr) { error(instr); }
     public void BuildDynRegExpInstr(BuildDynRegExpInstr instr) { error(instr); }
+    public void BuildRangeInstr(BuildRangeInstr instr) { error(instr); }
     public void CallInstr(CallInstr callinstr) { error(callinstr); }
     public void CheckArgsArrayArityInstr(CheckArgsArrayArityInstr checkargsarrayarityinstr) { error(checkargsarrayarityinstr); }
     public void CheckArityInstr(CheckArityInstr checkarityinstr) { error(checkarityinstr); }
@@ -156,7 +157,6 @@ private void error(Object object) {
     public void Nil(Nil nil) { error(nil); }
     public void NthRef(NthRef nthref) { error(nthref); }
     public void ObjectClass(ObjectClass objectclass) { error(objectclass); }
-    public void Range(Range range) { error(range); }
     public void Rational(Rational rational) { error(rational); }
     public void Regexp(Regexp regexp) { error(regexp); }
     public void ScopeModule(ScopeModule scopemodule) { error(scopemodule); }

diff --git a/core/src/main/java/org/jruby/ir/Operation.java b/core/src/main/java/org/jruby/ir/Operation.java
@@ -143,6 +143,7 @@ public enum Operation {
     BUILD_COMPOUND_ARRAY(OpFlags.f_can_raise_exception),
     BUILD_COMPOUND_STRING(OpFlags.f_can_raise_exception),
     BUILD_DREGEXP(OpFlags.f_can_raise_exception),
+    BUILD_RANGE(OpFlags.f_can_raise_exception),
     BACKTICK_STRING(OpFlags.f_can_raise_exception),
     CHECK_ARGS_ARRAY_ARITY(OpFlags.f_can_raise_exception),
     CHECK_ARITY(OpFlags.f_is_book_keeping_op | OpFlags.f_can_raise_exception),

diff --git a/...ain/java/org/jruby/ir/operands/Range.java → ...ruby/ir/instructions/BuildRangeInstr.java b/...ain/java/org/jruby/ir/operands/Range.java → ...ruby/ir/instructions/BuildRangeInstr.java
@@ -1,32 +1,51 @@
-package org.jruby.ir.operands;
+package org.jruby.ir.instructions;
 
 import org.jruby.RubyRange;
 import org.jruby.ir.IRVisitor;
+import org.jruby.ir.Operation;
+import org.jruby.ir.operands.Variable;
+import org.jruby.ir.operands.Operand;
 import org.jruby.ir.transformations.inlining.CloneInfo;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.DynamicScope;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
 
-import java.util.List;
 import java.util.Map;
 
 // Represents a range (1..5) or (a..b) in ruby code
 //
 // NOTE: This operand is only used in the initial stages of optimization
 // Further down the line, this range operand could get converted to calls
-// that actually build the Range object
-public class Range extends Operand {
-    final private Operand begin;
-    final private Operand end;
+// that actually build the BuildRangeInstr object
+public class BuildRangeInstr extends Instr implements ResultInstr {
+    private Variable result;
+    private Operand begin;
+    private Operand end;
     private boolean exclusive;
 
-    public Range(Operand begin, Operand end, boolean exclusive) {
-        super(OperandType.RANGE);
+    public BuildRangeInstr(Variable result, Operand begin, Operand end, boolean exclusive) {
+        super(Operation.BUILD_RANGE);
 
         this.begin = begin;
         this.end = end;
         this.exclusive = exclusive;
+        this.result = result;
+    }
+
+    @Override
+    public Variable getResult() {
+        return result;
+    }
+
+    @Override
+    public void updateResult(Variable v) {
+        this.result = v;
+    }
+
+    @Override
+    public Operand[] getOperands() {
+        return new Operand[] { begin, end };
     }
 
     public Operand getBegin() {
@@ -43,44 +62,30 @@ public boolean isExclusive() {
 
     @Override
     public String toString() {
-        return begin + (exclusive ? ".." : "...") + end;
+        return result + " = " + begin + (exclusive ? ".." : "...") + end;
     }
 
-
 // ---------- These methods below are used during compile-time optimizations -------
     @Override
-    public boolean hasKnownValue() {
-        return begin.hasKnownValue() && end.hasKnownValue();
-    }
-
-    @Override
-    public Operand getSimplifiedOperand(Map<Operand, Operand> valueMap, boolean force) {
-        Operand newBegin = begin.getSimplifiedOperand(valueMap, force);
-        Operand newEnd = end.getSimplifiedOperand(valueMap, force);
-        return (newBegin == begin && newEnd == end) ? this : new Range(newBegin, newEnd, exclusive);
-    }
-
-    /** Append the list of variables used in this operand to the input list */
-    @Override
-    public void addUsedVariables(List<Variable> l) {
-        begin.addUsedVariables(l);
-        end.addUsedVariables(l);
+    public void simplifyOperands(Map<Operand, Operand> valueMap, boolean force) {
+        begin = begin.getSimplifiedOperand(valueMap, force);
+        end= end.getSimplifiedOperand(valueMap, force);
     }
 
     @Override
-    public Operand cloneForInlining(CloneInfo ii) {
-        return hasKnownValue() ? this : new Range(begin.cloneForInlining(ii), end.cloneForInlining(ii), exclusive);
+    public Instr clone(CloneInfo ii) {
+        return new BuildRangeInstr(ii.getRenamedVariable(result), begin.cloneForInlining(ii), end.cloneForInlining(ii), exclusive);
     }
 
     @Override
-    public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
+    public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
         return RubyRange.newRange(context.runtime, context,
                 (IRubyObject) begin.retrieve(context, self, currScope, currDynScope, temp),
                 (IRubyObject) end.retrieve(context, self, currScope, currDynScope, temp), exclusive);
     }
 
     @Override
     public void visit(IRVisitor visitor) {
-        visitor.Range(this);
+        visitor.BuildRangeInstr(this);
     }
 }
diff --git a/core/src/main/java/org/jruby/ir/persistence/InstrDecoderMap.java b/core/src/main/java/org/jruby/ir/persistence/InstrDecoderMap.java
@@ -53,10 +53,10 @@ public Instr decodeInner(Operation operation) {
             case CHECK_ARITY: return new CheckArityInstr(d.decodeInt(), d.decodeInt(), d.decodeInt(), d.decodeBoolean(), d.decodeInt());
             case CLASS_VAR_MODULE: return new GetClassVarContainerModuleInstr(d.decodeVariable(), d.decodeOperand(), d.decodeVariable());
             case CONST_MISSING: return decodeConstMissingInstr();
-            // SSS FIXME: Might need fixing
-            // case BUILD_COMPOUND_INSTR: return decodeBuildCompoundStringInstr();
             // SSS FIXME: TODO
+            // case BUILD_COMPOUND_INSTR: return decodeBuildCompoundStringInstr();
             // case BUILD_DREGEXP: return decodeBuildDynRegExpInstr();
+            // case BUILD_RANGE: return new Range(d.decodeOperand(), d.decodeOperand(), d.decodeBoolean());
             case COPY: return decodeCopy();
             case DEF_CLASS: return new DefineClassInstr((d.decodeVariable()), (IRClassBody) d.decodeScope(), d.decodeOperand(), d.decodeOperand());
             case DEF_CLASS_METH: return new DefineClassMethodInstr(d.decodeOperand(), (IRMethod) d.decodeScope());

diff --git a/core/src/main/java/org/jruby/ir/persistence/InstrEncoderMap.java b/core/src/main/java/org/jruby/ir/persistence/InstrEncoderMap.java
@@ -47,10 +47,9 @@ public void encode(Instr instr) {
             case CHECK_ARGS_ARRAY_ARITY: encodeCheckArgsArrayArityInstr((CheckArgsArrayArityInstr) instr); break;
             case CHECK_ARITY: encodeCheckArityInstr((CheckArityInstr) instr); break;
             case CLASS_VAR_MODULE: encodeGetClassVarContainerModuleInstr((GetClassVarContainerModuleInstr) instr); break;
-            // SSS FIXME: Needs fixing
             // case BUILD_COMPOUND_STRING: encodeBuildCompoundStringInstr((BuildCompoundStringInstr) instr); break;
-            // SSS FIXME: TODO
             // case BUILD_DREGEXP: return encodeBuildDynRegExpInstr();
+            // case BUILD_RANGE: return encodeBuildRangeInstr();
             case CONST_MISSING: encodeConstMissingInstr((ConstMissingInstr) instr); break;
             case COPY: encodeCopyInstr((CopyInstr) instr); break;
             case DEF_CLASS: encodeDefineClassInstr((DefineClassInstr) instr); break;

diff --git a/core/src/main/java/org/jruby/ir/persistence/OperandDecoderMap.java b/core/src/main/java/org/jruby/ir/persistence/OperandDecoderMap.java
@@ -50,7 +50,6 @@ public Operand decode(OperandType type) {
             case NIL: return manager.getNil();
             case NTH_REF: return new NthRef(d.decodeInt());
             case OBJECT_CLASS: return new ObjectClass();
-            case RANGE: return new Range(d.decodeOperand(), d.decodeOperand(), d.decodeBoolean());
             case REGEXP: return decodeRegexp();
             case SCOPE_MODULE: return new ScopeModule(d.decodeInt());
             case SELF: return Self.SELF;

diff --git a/core/src/main/java/org/jruby/ir/persistence/OperandEncoderMap.java b/core/src/main/java/org/jruby/ir/persistence/OperandEncoderMap.java
@@ -93,12 +93,6 @@ public void encode(Operand operand) {
 
     @Override public void ObjectClass(ObjectClass objectclass) {} // No data
 
-    @Override public void Range(Range range) {
-        encoder.encode(range.getBegin());
-        encoder.encode(range.getEnd());
-        encoder.encode(range.isExclusive());
-    }
-
     @Override public void Regexp(Regexp regexp) {
         encode(regexp.getRegexp());
         encoder.encode(regexp.options.isEncodingNone());

diff --git a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
@@ -767,6 +767,17 @@ public void run() {
         jvmStoreLocal(instr.getResult());
     }
 
+    @Override
+    public void BuildRangeInstr(BuildRangeInstr instr) {
+        jvmMethod().loadRuntime();
+        jvmMethod().loadContext();
+        visit(instr.getBegin());
+        visit(instr.getEnd());
+        jvmAdapter().ldc(instr.isExclusive());
+        jvmAdapter().invokestatic(p(RubyRange.class), "newRange", sig(RubyRange.class, Ruby.class, ThreadContext.class, IRubyObject.class, IRubyObject.class, boolean.class));
+        jvmStoreLocal(instr.getResult());
+    }
+
     @Override
     public void CallInstr(CallInstr callInstr) {
         IRBytecodeAdapter m = jvmMethod();
@@ -2017,16 +2028,6 @@ public void ObjectClass(ObjectClass objectclass) {
         jvmMethod().pushObjectClass();
     }
 
-    @Override
-    public void Range(Range range) {
-        jvmMethod().loadRuntime();
-        jvmMethod().loadContext();
-        visit(range.getBegin());
-        visit(range.getEnd());
-        jvmAdapter().ldc(range.isExclusive());
-        jvmAdapter().invokestatic(p(RubyRange.class), "newRange", sig(RubyRange.class, Ruby.class, ThreadContext.class, IRubyObject.class, IRubyObject.class, boolean.class));
-    }
-
     @Override
     public void Regexp(Regexp regexp) {
         if (!regexp.hasKnownValue() && !regexp.options.isOnce()) {