Skip to content

Commit

Permalink
Merge pull request #4812 from jruby/jruby-9.1-with-lje-and-eqq
Browse files Browse the repository at this point in the history
Backport LocalJumpError and EQQ fixes from 9.2
  • Loading branch information
headius committed Oct 10, 2017
2 parents 697c3cf + d455523 commit 210e637
Show file tree
Hide file tree
Showing 17 changed files with 195 additions and 93 deletions.
2 changes: 1 addition & 1 deletion core/pom.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
jar 'org.jruby.jcodings:jcodings:1.0.18'
jar 'org.jruby:dirgra:0.3'

jar 'com.headius:invokebinder:1.7'
jar 'com.headius:invokebinder:1.8'
jar 'com.headius:options:1.4'
jar 'com.headius:unsafe-fences:1.0'

Expand Down
2 changes: 1 addition & 1 deletion core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ DO NOT MODIFIY - GENERATED CODE
<dependency>
<groupId>com.headius</groupId>
<artifactId>invokebinder</artifactId>
<version>1.7</version>
<version>1.8-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>com.headius</groupId>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -1321,7 +1321,7 @@ public int compare(Map.Entry<Integer, Label> o1, Map.Entry<Integer, Label> o2) {
expression = ((StringLiteral) expression).frozenString;
}

addInstr(new EQQInstr(eqqResult, expression, value, true));
addInstr(new EQQInstr(eqqResult, expression, value, false));
v1 = eqqResult;
v2 = manager.getTrue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public Operand getClosureArg() {
return getOperand1();
}

public boolean hasLiteralClosure() { return getClosureArg() instanceof WrappedIRClosure; }

@Override
public void encode(IRWriterEncoder e) {
super.encode(e);
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/java/org/jruby/ir/instructions/CallBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,14 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco
IRubyObject[] values = prepareArguments(context, self, currScope, dynamicScope, temp);
Block preparedBlock = prepareBlock(context, self, currScope, dynamicScope, temp);

if (hasLiteralClosure()) {
try {
return callSite.call(context, self, object, values, preparedBlock);
} finally {
preparedBlock.escape();
}
}

return callSite.call(context, self, object, values, preparedBlock);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
*/
public interface ClosureAcceptingInstr {
public Operand getClosureArg();
public boolean hasLiteralClosure();
}
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ protected static void processCall(ThreadContext context, Instr instr, Operation
IRubyObject r = (IRubyObject)retrieveOp(call.getReceiver(), context, self, currDynScope, currScope, temp);
IRubyObject o = (IRubyObject)call.getArg1().retrieve(context, self, currScope, currDynScope, temp);
Block preparedBlock = call.prepareBlock(context, self, currScope, currDynScope, temp);
result = call.getCallSite().call(context, self, r, o, preparedBlock);
result = call.getCallSite().callIter(context, self, r, o, preparedBlock);
setResult(temp, currDynScope, call.getResult(), result);
break;
}
Expand Down Expand Up @@ -419,7 +419,6 @@ protected static void processBookKeepingOp(ThreadContext context, Block block, I
protected static IRubyObject processReturnOp(ThreadContext context, Block block, Instr instr, Operation operation,
DynamicScope currDynScope, Object[] temp, IRubyObject self,
StaticScope currScope) {
Block.Type blockType = block == null ? null : block.type;
switch(operation) {
// --------- Return flavored instructions --------
case RETURN: {
Expand All @@ -433,12 +432,12 @@ protected static IRubyObject processReturnOp(ThreadContext context, Block block,
// This assumes that scopes with break instr. have a frame / dynamic scope
// pushed so that we can get to its static scope. For-loops now always have
// a dyn-scope pushed onto stack which makes this work in all scenarios.
return IRRuntimeHelpers.initiateBreak(context, currDynScope, rv, blockType);
return IRRuntimeHelpers.initiateBreak(context, currDynScope, rv, block);
}
case NONLOCAL_RETURN: {
NonlocalReturnInstr ri = (NonlocalReturnInstr)instr;
IRubyObject rv = (IRubyObject)retrieveOp(ri.getReturnValue(), context, self, currDynScope, currScope, temp);
return IRRuntimeHelpers.initiateNonLocalReturn(context, currDynScope, blockType, rv);
return IRRuntimeHelpers.initiateNonLocalReturn(context, currDynScope, block, rv);
}
case RETURN_OR_RETHROW_SAVED_EXC: {
IRubyObject retVal = (IRubyObject) retrieveOp(((ReturnBase) instr).getReturnValue(), context, self, currDynScope, currScope, temp);
Expand Down
21 changes: 11 additions & 10 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ public static void checkForLJE(ThreadContext context, DynamicScope dynScope, boo
}

// Create a jump for a non-local return which will return from nearest lambda (which may be itself) or method.
public static IRubyObject initiateNonLocalReturn(ThreadContext context, DynamicScope dynScope, Block.Type blockType, IRubyObject returnValue) {
if (IRRuntimeHelpers.inLambda(blockType)) throw new IRWrappedLambdaReturnValue(returnValue);
public static IRubyObject initiateNonLocalReturn(ThreadContext context, DynamicScope dynScope, Block block, IRubyObject returnValue) {
if (block != null && IRRuntimeHelpers.inLambda(block.type)) throw new IRWrappedLambdaReturnValue(returnValue);

throw IRReturnJump.create(getContainingMethodOrLambdasDynamicScope(dynScope), returnValue);
}
Expand Down Expand Up @@ -159,15 +159,21 @@ private static IRScopeType ensureScopeIsClosure(ThreadContext context, DynamicSc
}

// FIXME: When we recompile lambdas we can eliminate this binary code path and we can emit as a NONLOCALRETURN directly.
public static IRubyObject initiateBreak(ThreadContext context, DynamicScope dynScope, IRubyObject breakValue, Block.Type blockType) throws RuntimeException {
public static IRubyObject initiateBreak(ThreadContext context, DynamicScope dynScope, IRubyObject breakValue, Block block) throws RuntimeException {
// Wrap the return value in an exception object and push it through the break exception
// paths so that ensures are run, frames/scopes are popped from runtime stacks, etc.
if (inLambda(blockType)) throw new IRWrappedLambdaReturnValue(breakValue);
if (inLambda(block.type)) throw new IRWrappedLambdaReturnValue(breakValue);

IRScopeType scopeType = ensureScopeIsClosure(context, dynScope);

DynamicScope parentScope = dynScope.getParentScope();

if (block.isEscaped()) {
throw context.runtime.newLocalJumpError(RubyLocalJumpError.Reason.BREAK, breakValue, "unexpected break");
}

// Raise a break jump so we can bubble back down the stack to the appropriate place to break from.
throw IRBreakJump.create(dynScope.getParentScope(), breakValue, scopeType.isEval()); // weirdly evals are impld as closures...yes yes.
throw IRBreakJump.create(parentScope, breakValue, scopeType.isEval()); // weirdly evals are impld as closures...yes yes.
}

// Are we within the scope where we want to return the value we are passing down the stack?
Expand Down Expand Up @@ -402,11 +408,6 @@ public static IRubyObject isEQQ(ThreadContext context, IRubyObject receiver, IRu
return isUndefValue ? receiver : callSite.call(context, receiver, receiver, value);
}

@Deprecated
public static IRubyObject isEQQ(ThreadContext context, IRubyObject receiver, IRubyObject value, CallSite callSite) {
return isEQQ(context, receiver, value, callSite, true);
}

public static IRubyObject newProc(Ruby runtime, Block block) {
return (block == Block.NULL_BLOCK) ? runtime.getNil() : runtime.newProc(Block.Type.PROC, block);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*/
public class ArrayDerefInvokeSite extends NormalInvokeSite {
public ArrayDerefInvokeSite(MethodType type, String file, int line) {
super(type, "[]", file, line);
super(type, "[]", false, file, line);
}

public static final Handle BOOTSTRAP = new Handle(Opcodes.H_INVOKESTATIC, p(ArrayDerefInvokeSite.class), "bootstrap", sig(CallSite.class, MethodHandles.Lookup.class, String.class, MethodType.class, String.class, int.class));
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/targets/Bootstrap.java
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ static MethodHandle buildJittedHandle(InvokeSite site, DynamicMethod method, boo
// Temporary fix for missing kwargs dup+splitting logic from frobnicate, called by CompiledIRMethod but
// skipped by indy's direct binding.
if (compiledIRMethod.hasKwargs()) return null;

// attempt IR direct binding
// TODO: this will have to expand when we start specializing arities

Expand Down
41 changes: 37 additions & 4 deletions core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.compiler.impl.SkinnyMethodAdapter;
import org.jruby.ir.instructions.ClosureAcceptingInstr;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.UndefinedValue;
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.runtime.CallType;
Expand Down Expand Up @@ -361,9 +363,9 @@ public org.objectweb.asm.Label newLabel() {
*
* @param name name of the method to invoke
* @param arity arity of the call
* @param hasClosure whether a closure will be on the stack for passing
* @param blockPassType what type of closure is passed
*/
public abstract void invokeOther(String file, int line, String name, int arity, boolean hasClosure, boolean isPotentiallyRefined);
public abstract void invokeOther(String file, int line, String name, int arity, BlockPassType blockPassType, boolean isPotentiallyRefined);

/**
* Invoke the array dereferencing method ([]) on an object other than self.
Expand Down Expand Up @@ -394,6 +396,30 @@ public org.objectweb.asm.Label newLabel() {
*/
public abstract void invokeOtherOneFloat(String file, int line, String name, double flote, CallType callType);

public enum BlockPassType {
NONE(false, false),
GIVEN(true, false),
LITERAL(true, true);

private final boolean given;
private final boolean literal;

BlockPassType(boolean given, boolean literal) {
this.given = given;
this.literal = literal;
}

public boolean given() {
return given;
}
public boolean literal() {
return literal;
}
public static BlockPassType fromIR(ClosureAcceptingInstr callInstr) {
Operand closure = callInstr.getClosureArg();
return closure != null ? ( callInstr.hasLiteralClosure() ? BlockPassType.LITERAL : BlockPassType.GIVEN) : BlockPassType.NONE;
}
}

/**
* Invoke a method on self.
Expand All @@ -404,10 +430,10 @@ public org.objectweb.asm.Label newLabel() {
* @param line the line number where this call appears
* @param name name of the method to invoke
* @param arity arity of the call
* @param hasClosure whether a closure will be on the stack for passing
* @param blockPassType what type of closure is passed
* @param callType
*/
public abstract void invokeSelf(String file, int line, String name, int arity, boolean hasClosure, CallType callType, boolean isPotentiallyRefined);
public abstract void invokeSelf(String file, int line, String name, int arity, BlockPassType blockPassType, CallType callType, boolean isPotentiallyRefined);

/**
* Invoke a superclass method from an instance context.
Expand Down Expand Up @@ -626,6 +652,13 @@ public org.objectweb.asm.Label newLabel() {
*/
public abstract void prepareBlock(Handle handle, org.jruby.runtime.Signature signature, String className);

/**
* Perform a === call appropriate for a case/when statement.
*
* Stack required: context, case value, when value
*/
public abstract void callEqq(boolean isSplattedValue);

public SkinnyMethodAdapter adapter;
private int variableCount = 0;
private Map<Integer, Type> variableTypes = new HashMap<Integer, Type>();
Expand Down
42 changes: 26 additions & 16 deletions core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,9 @@ public void run() {
});
}

public void invokeOther(String file, int line, String name, int arity, boolean hasClosure, boolean isPotentiallyRefined) {
invoke(file, line, name, arity, hasClosure, CallType.NORMAL, isPotentiallyRefined);
@Override
public void invokeOther(String file, int line, String name, int arity, BlockPassType blockPassType, boolean isPotentiallyRefined) {
invoke(file, line, name, arity, blockPassType, CallType.NORMAL, isPotentiallyRefined);
}

public void invokeArrayDeref(String file, int line) {
Expand All @@ -410,14 +411,15 @@ public void invokeArrayDeref(String file, int line) {
adapter.invokestatic(getClassData().clsName, methodName, incomingSig);
}

public void invoke(String file, int lineNumber, String name, int arity, boolean hasClosure, CallType callType, boolean isPotentiallyRefined) {
public void invoke(String file, int lineNumber, String name, int arity, BlockPassType blockPassType, CallType callType, boolean isPotentiallyRefined) {
if (arity > MAX_ARGUMENTS) throw new NotCompilableException("call to `" + name + "' has more than " + MAX_ARGUMENTS + " arguments");

SkinnyMethodAdapter adapter2;
String incomingSig;
String outgoingSig;

if (hasClosure) {
boolean blockGiven = blockPassType.given();
if (blockGiven) {
switch (arity) {
case -1:
incomingSig = sig(JVM.OBJECT, params(ThreadContext.class, JVM.OBJECT, JVM.OBJECT, JVM.OBJECT_ARRAY, Block.class));
Expand Down Expand Up @@ -478,29 +480,29 @@ public void invoke(String file, int lineNumber, String name, int arity, boolean
case -1:
case 1:
adapter2.aload(3);
if (hasClosure) adapter2.aload(4);
if (blockGiven) adapter2.aload(4);
break;
case 0:
if (hasClosure) adapter2.aload(3);
if (blockGiven) adapter2.aload(3);
break;
case 2:
adapter2.aload(3);
adapter2.aload(4);
if (hasClosure) adapter2.aload(5);
if (blockGiven) adapter2.aload(5);
break;
case 3:
adapter2.aload(3);
adapter2.aload(4);
adapter2.aload(5);
if (hasClosure) adapter2.aload(6);
if (blockGiven) adapter2.aload(6);
break;
default:
buildArrayFromLocals(adapter2, 3, arity);
if (hasClosure) adapter2.aload(3 + arity);
if (blockGiven) adapter2.aload(3 + arity);
break;
}

adapter2.invokevirtual(p(CachingCallSite.class), "call", outgoingSig);
adapter2.invokevirtual(p(CachingCallSite.class), blockPassType.literal() ? "callIter" : "call", outgoingSig);
adapter2.areturn();
adapter2.end();

Expand Down Expand Up @@ -533,9 +535,9 @@ public void invokeOtherOneFixnum(String file, int line, String name, long fixnum
if (!MethodIndex.hasFastFixnumOps(name)) {
pushFixnum(fixnum);
if (callType == CallType.NORMAL) {
invokeOther(file, line, name, 1, false, false);
invokeOther(file, line, name, 1, BlockPassType.NONE, false);
} else {
invokeSelf(file, line, name, 1, false, callType, false);
invokeSelf(file, line, name, 1, BlockPassType.NONE, callType, false);
}
return;
}
Expand Down Expand Up @@ -589,9 +591,9 @@ public void invokeOtherOneFloat(String file, int line, String name, double flote
if (!MethodIndex.hasFastFloatOps(name)) {
pushFloat(flote);
if (callType == CallType.NORMAL) {
invokeOther(file, line, name, 1, false, false);
invokeOther(file, line, name, 1, BlockPassType.NONE, false);
} else {
invokeSelf(file, line, name, 1, false, callType, false);
invokeSelf(file, line, name, 1, BlockPassType.NONE, callType, false);
}
return;
}
Expand Down Expand Up @@ -641,10 +643,10 @@ public void invokeOtherOneFloat(String file, int line, String name, double flote
adapter.invokestatic(getClassData().clsName, methodName, incomingSig);
}

public void invokeSelf(String file, int line, String name, int arity, boolean hasClosure, CallType callType, boolean isPotentiallyRefined) {
public void invokeSelf(String file, int line, String name, int arity, BlockPassType blockPassType, CallType callType, boolean isPotentiallyRefined) {
if (arity > MAX_ARGUMENTS) throw new NotCompilableException("call to `" + name + "' has more than " + MAX_ARGUMENTS + " arguments");

invoke(file, line, name, arity, hasClosure, callType, isPotentiallyRefined);
invoke(file, line, name, arity, blockPassType, callType, isPotentiallyRefined);
}

public void invokeInstanceSuper(String file, int line, String name, int arity, boolean hasClosure, boolean[] splatmap) {
Expand Down Expand Up @@ -1015,5 +1017,13 @@ public void prepareBlock(Handle handle, org.jruby.runtime.Signature signature, S
invokeIRHelper("prepareBlock", sig(Block.class, ThreadContext.class, IRubyObject.class, DynamicScope.class, BlockBody.class));
}

@Override
public void callEqq(boolean isSplattedValue) {
String siteName = getUniqueSiteName("===");
IRBytecodeAdapter.cacheCallSite(adapter, getClassData().clsName, siteName, "===", CallType.FUNCTIONAL, false);
adapter.ldc(isSplattedValue);
invokeIRHelper("isEQQ", sig(IRubyObject.class, ThreadContext.class, IRubyObject.class, IRubyObject.class, CallSite.class, boolean.class));
}

private final Map<Object, String> cacheFieldNames = new HashMap<>();
}
Loading

0 comments on commit 210e637

Please sign in to comment.