Skip to content

Commit

Permalink
Fix #2409: Splat --> BuildSplatInstr + Splat (for use in call args)
Browse files Browse the repository at this point in the history
* Splat operand is now a BuildSplatInstr to represent splats seen
  in ruby code. These can throw exceptions.

* Splat operand is now only used in call args and will always
  extract its args and is guaranteed to have a RubyArray for its
  operand. These cannot throw exceptions.

* This will ungreen master since my jit fixes seem to be incomplete --
  Charlie will take over those fixes.
  • Loading branch information
subbuss committed Jan 5, 2015
1 parent 8ced6d2 commit 29371d9
Show file tree
Hide file tree
Showing 11 changed files with 127 additions and 27 deletions.
14 changes: 7 additions & 7 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -577,11 +577,11 @@ protected Operand buildAttrAssignCallArgs(List<Operand> argsList, Node args, IRS
Operand rhs = build(argsPushNode.getSecondNode(), s);
Variable res = s.createTemporaryVariable();
addInstr(s, new BuildCompoundArrayInstr(res, lhs, rhs, true));
argsList.add(new Splat(res, true));
argsList.add(new Splat(res));
return rhs;
}
case SPLATNODE: { // a[1] = *b
Splat rhs = new Splat(build(((SplatNode)args).getValue(), s), true);
Splat rhs = new Splat(buildSplat((SplatNode)args, s));
argsList.add(rhs);
return rhs;
}
Expand All @@ -594,7 +594,7 @@ protected Operand[] buildCallArgs(Node args, IRScope s) {
switch (args.getNodeType()) {
case ARGSCATNODE:
case ARGSPUSHNODE:
return new Operand[] { new Splat(build(args, s), true) };
return new Operand[] { new Splat(build(args, s)) };
case ARRAYNODE: {
List<Node> children = args.childNodes();
int numberOfArgs = children.size();
Expand All @@ -606,7 +606,7 @@ protected Operand[] buildCallArgs(Node args, IRScope s) {
return builtArgs;
}
case SPLATNODE:
return new Operand[] { new Splat(build(((SplatNode) args).getValue(), s), true) };
return new Operand[] { new Splat(buildSplat((SplatNode)args, s)) };
}

throw new NotCompilableException("Invalid node for call args: " + args.getClass().getSimpleName() + ":" + args.getPosition());
Expand Down Expand Up @@ -3269,9 +3269,9 @@ public Operand buildSelf(IRScope s) {
}

public Operand buildSplat(SplatNode splatNode, IRScope s) {
// SSS: Since splats can only occur in call argument lists, no need to copyAndReturnValue(...)
// Verify with Tom / Charlie
return new Splat(build(splatNode.getValue(), s));
Variable res = s.createTemporaryVariable();
addInstr(s, new BuildSplatInstr(res, build(splatNode.getValue(), s)));
return res;
}

public Operand buildStr(StrNode strNode, IRScope s) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/IRClosure.java
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void addInstr(Instr i) {
// FIXME: This lost encoding information when name was converted to string earlier in IRBuilder
keywordArgs.add(new KeyValuePair<Operand, Operand>(new Symbol(rkai.argName, USASCIIEncoding.INSTANCE), rkai.getResult()));
} else if (i instanceof ReceiveRestArgInstr) {
blockArgs.add(new Splat(((ReceiveRestArgInstr)i).getResult(), true));
blockArgs.add(new Splat(((ReceiveRestArgInstr)i).getResult()));
} else if (i instanceof ReceiveArgBase) {
blockArgs.add(((ReceiveArgBase) i).getResult());
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/IRMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void addInstr(Instr i) {
// FIXME: This lost encoding information when name was converted to string earlier in IRBuilder
keywordArgs.add(new KeyValuePair<Operand, Operand>(new Symbol(rkai.argName, USASCIIEncoding.INSTANCE), rkai.getResult()));
} else if (i instanceof ReceiveRestArgInstr) {
callArgs.add(new Splat(((ReceiveRestArgInstr)i).getResult(), true));
callArgs.add(new Splat(((ReceiveRestArgInstr)i).getResult()));
} else if (i instanceof ReceiveArgBase) {
callArgs.add(((ReceiveArgBase) i).getResult());
}
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/IRVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ private void error(Object object) {
public void BuildCompoundStringInstr(BuildCompoundStringInstr instr) { error(instr); }
public void BuildDynRegExpInstr(BuildDynRegExpInstr instr) { error(instr); }
public void BuildRangeInstr(BuildRangeInstr instr) { error(instr); }
public void BuildSplatInstr(BuildSplatInstr instr) { error(instr); }
public void CallInstr(CallInstr callinstr) { error(callinstr); }
public void CheckArgsArrayArityInstr(CheckArgsArrayArityInstr checkargsarrayarityinstr) { error(checkargsarrayarityinstr); }
public void CheckArityInstr(CheckArityInstr checkarityinstr) { error(checkarityinstr); }
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/Operation.java
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public enum Operation {
BUILD_COMPOUND_STRING(OpFlags.f_can_raise_exception),
BUILD_DREGEXP(OpFlags.f_can_raise_exception),
BUILD_RANGE(OpFlags.f_can_raise_exception),
BUILD_SPLAT(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),
Expand Down
71 changes: 71 additions & 0 deletions core/src/main/java/org/jruby/ir/instructions/BuildSplatInstr.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.jruby.ir.instructions;

import org.jruby.ir.IRVisitor;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.runtime.IRRuntimeHelpers;
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.Map;

// Represents a splat value in Ruby code: *array
public class BuildSplatInstr extends Instr implements ResultInstr {
private Variable result;
private Operand array;

public BuildSplatInstr(Variable result, Operand array) {
super(Operation.BUILD_SPLAT);
this.result = result;
this.array = array;
}

@Override
public String toString() {
return result + " = *" + array;
}

@Override
public Variable getResult() {
return result;
}

@Override
public void updateResult(Variable v) {
this.result = v;
}

public Operand getArray() {
return array;
}

@Override
public Operand[] getOperands() {
return new Operand[] { array };
}

@Override
public void simplifyOperands(Map<Operand, Operand> valueMap, boolean force) {
array = array.getSimplifiedOperand(valueMap, force);
}

@Override
public Instr clone(CloneInfo ii) {
return new BuildSplatInstr(ii.getRenamedVariable(result), array.cloneForInlining(ii));
}

@Override
public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
IRubyObject arrayVal = (IRubyObject) array.retrieve(context, self, currScope, currDynScope, temp);
return IRRuntimeHelpers.irSplat(context, arrayVal);
}

@Override
public void visit(IRVisitor visitor) {
visitor.BuildSplatInstr(this);
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/instructions/CallBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public String toString() {

public static boolean containsArgSplat(Operand[] arguments) {
for (Operand argument : arguments) {
if (argument instanceof Splat && ((Splat)argument).unsplatArgs) return true;
if (argument instanceof Splat) return true;
}

return false;
Expand Down
34 changes: 20 additions & 14 deletions core/src/main/java/org/jruby/ir/operands/Splat.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,15 @@
// Further down the line, it could get converted to calls that implement splat semantics
public class Splat extends Operand implements DepthCloneable {
final private Operand array;
final public boolean unsplatArgs;

public Splat(Operand array, boolean unsplatArgs) {
public Splat(Operand array) {
super(OperandType.SPLAT);
this.array = array;
this.unsplatArgs = unsplatArgs;
}

public Splat(Operand array) {
this(array, false);
}

@Override
public String toString() {
return (unsplatArgs ? "*(unsplat)" : "*") + array;
return "*(unsplat)" + array;
}

@Override
Expand All @@ -46,7 +40,7 @@ public Operand getArray() {
@Override
public Operand getSimplifiedOperand(Map<Operand, Operand> valueMap, boolean force) {
Operand newArray = array.getSimplifiedOperand(valueMap, force);
return (newArray == array) ? this : new Splat(newArray, unsplatArgs);
return (newArray == array) ? this : new Splat(newArray);
}

/** Append the list of variables used in this operand to the input list */
Expand All @@ -57,19 +51,31 @@ public void addUsedVariables(List<Variable> l) {

/** When fixing up splats in nested closure we need to tweak the operand if it is a LocalVariable */
public Operand cloneForDepth(int n) {
return array instanceof LocalVariable ? new Splat(((LocalVariable) array).cloneForDepth(n), unsplatArgs) : this;
return array instanceof LocalVariable ? new Splat(((LocalVariable) array).cloneForDepth(n)) : this;
}

@Override
public Operand cloneForInlining(CloneInfo ii) {
return hasKnownValue() ? this : new Splat(array.cloneForInlining(ii), unsplatArgs);
return hasKnownValue() ? this : new Splat(array.cloneForInlining(ii));
}

@Override
public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
IRubyObject arrayVal = (IRubyObject) array.retrieve(context, self, currScope, currDynScope, temp);
// SSS FIXME: Some way to specialize this code?
return IRRuntimeHelpers.irSplat(context, arrayVal);
// Splat is now only used in call arg lists where it is guaranteed that
// the splat-arg is an array.
//
// It is:
// - either a result of a args-cat/args-push (which generate an array),
// - or a result of a BuildSplatInstr (which also generates an array),
// - or a rest-arg that has been received (which also generates an array)
// and is being passed via zsuper.
//
// In addition, since this only shows up in call args, the array itself is
// never modified. The array elements are extracted out and inserted into
// a java array. So, a dup is not required either.
//
// So, besides retrieving the array, nothing more to be done here!
return (IRubyObject) array.retrieve(context, self, currScope, currDynScope, temp);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public Operand decode(OperandType type) {
case REGEXP: return decodeRegexp();
case SCOPE_MODULE: return new ScopeModule(d.decodeInt());
case SELF: return Self.SELF;
case SPLAT: return new Splat(d.decodeOperand(), d.decodeBoolean());
case SPLAT: return new Splat(d.decodeOperand());
case STANDARD_ERROR: return new StandardError();
case STRING_LITERAL: return new StringLiteral(d.decodeString());
case SVALUE: return new SValue(d.decodeOperand());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void encode(Operand operand) {

@Override public void Self(Self self) {} // No data

@Override public void Splat(Splat splat) { encode(splat.getArray()); encoder.encode(splat.unsplatArgs); }
@Override public void Splat(Splat splat) { encode(splat.getArray()); }

@Override public void StandardError(StandardError standarderror) {} // No data

Expand Down
23 changes: 22 additions & 1 deletion core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,14 @@ public void BuildRangeInstr(BuildRangeInstr instr) {
jvmStoreLocal(instr.getResult());
}

@Override
public void BuildSplatInstr(BuildSplatInstr instr) {
jvmMethod().loadContext();
visit(instr.getArray());
jvmMethod().invokeIRHelper("irSplat", sig(RubyArray.class, ThreadContext.class, IRubyObject.class));
jvmStoreLocal(instr.getResult());
}

@Override
public void CallInstr(CallInstr callInstr) {
IRBytecodeAdapter m = jvmMethod();
Expand Down Expand Up @@ -2115,7 +2123,20 @@ public void Self(Self self) {
public void Splat(Splat splat) {
jvmMethod().loadContext();
visit(splat.getArray());
jvmMethod().invokeIRHelper("irSplat", sig(RubyArray.class, ThreadContext.class, IRubyObject.class));
// Splat is now only used in call arg lists where it is guaranteed that
// the splat-arg is an array.
//
// It is:
// - either a result of a args-cat/args-push (which generate an array),
// - or a result of a BuildSplatInstr (which also generates an array),
// - or a rest-arg that has been received (which also generates an array)
// and is being passed via zsuper.
//
// In addition, since this only shows up in call args, the array itself is
// never modified. The array elements are extracted out and inserted into
// a java array. So, a dup is not required either.
//
// So, besides retrieving the array, nothing more to be done here!
}

@Override
Expand Down

0 comments on commit 29371d9

Please sign in to comment.