Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 0ab806add94f
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: a13e521a9379
Choose a head ref
  • 2 commits
  • 3 files changed
  • 1 contributor

Commits on Dec 6, 2015

  1. Update ACP pass to add explicit invocation protocol for blocks

    * This is not yet enabled.
    * This needs thorough testing in interpreter mode (and I am running
      into some lifecycle issues that I'll have to figure out).
    * Also, JIT needs to be updated to generate code for these new
      instructions before this is turned on there.
    subbuss committed Dec 6, 2015
    Copy the full SHA
    9674fa6 View commit details
  2. Add direct yield/call methods to MixedModeIRBlockBody

    * Also fixed a typo in signature
    subbuss committed Dec 6, 2015
    Copy the full SHA
    a13e521 View commit details
183 changes: 112 additions & 71 deletions core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java
Original file line number Diff line number Diff line change
@@ -3,9 +3,11 @@
import org.jruby.ir.*;
import org.jruby.ir.dataflow.analyses.StoreLocalVarPlacementProblem;
import org.jruby.ir.instructions.*;
import org.jruby.runtime.Signature;
import org.jruby.ir.operands.ImmutableLiteral;
import org.jruby.ir.operands.Label;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Self;
import org.jruby.ir.operands.TemporaryVariable;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.representations.BasicBlock;
@@ -20,7 +22,10 @@ public String getLabel() {
}

private boolean explicitCallProtocolSupported(IRScope scope) {
return scope instanceof IRMethod || (scope instanceof IRModuleBody && !(scope instanceof IRMetaClassBody));
return scope instanceof IRMethod
// SSS: Turning this off till this is fully debugged
// || (scope instanceof IRClosure && !(scope instanceof IREvalScript))
|| (scope instanceof IRModuleBody && !(scope instanceof IRMetaClassBody));
}

/*
@@ -41,6 +46,16 @@ private void fixReturn(IRScope scope, ReturnBase i, ListIterator<Instr> instrs)
}
}

private void popSavedState(IRScope scope, boolean requireBinding, boolean requireFrame, Variable savedViz, Variable savedFrame, ListIterator<Instr> instrs) {
if (requireBinding) instrs.add(new PopBindingInstr());
if (scope instanceof IRClosure) {
instrs.add(new PopBlockFrameInstr(savedFrame));
instrs.add(new RestoreBindingVisibilityInstr(savedViz));
} else {
if (requireFrame) instrs.add(new PopMethodFrameInstr());
}
}

@Override
public Object execute(IRScope scope, Object... data) {
// IRScriptBody do not get explicit call protocol instructions right now.
@@ -50,95 +65,121 @@ public Object execute(IRScope scope, Object... data) {
// Add explicit frame and binding push/pop instrs ONLY for methods -- we cannot handle this in closures and evals yet
// If the scope uses $_ or $~ family of vars, has local load/stores, or if its binding has escaped, we have
// to allocate a dynamic scope for it and add binding push/pop instructions.
if (explicitCallProtocolSupported(scope)) {
StoreLocalVarPlacementProblem slvpp = scope.getStoreLocalVarPlacementProblem();
boolean scopeHasLocalVarStores = false;
boolean bindingHasEscaped = scope.bindingHasEscaped();
if (!explicitCallProtocolSupported(scope)) return null;

CFG cfg = scope.getCFG();
StoreLocalVarPlacementProblem slvpp = scope.getStoreLocalVarPlacementProblem();
boolean scopeHasLocalVarStores = false;
boolean bindingHasEscaped = scope.bindingHasEscaped();

if (slvpp != null && bindingHasEscaped) {
scopeHasLocalVarStores = slvpp.scopeHasLocalVarStores();
} else {
// We dont require local-var load/stores to have been run.
// If it is not run, we go conservative and add push/pop binding instrs. everywhere
scopeHasLocalVarStores = bindingHasEscaped;
}
CFG cfg = scope.getCFG();

boolean requireFrame = doesItRequireFrame(scope, bindingHasEscaped);
boolean requireBinding = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);
if (slvpp != null && bindingHasEscaped) {
scopeHasLocalVarStores = slvpp.scopeHasLocalVarStores();
} else {
// We dont require local-var load/stores to have been run.
// If it is not run, we go conservative and add push/pop binding instrs. everywhere
scopeHasLocalVarStores = bindingHasEscaped;
}

if (requireBinding || requireFrame) {
BasicBlock entryBB = cfg.getEntryBB();
// Push
// For now, we always require frame for closures
boolean requireFrame = doesItRequireFrame(scope, bindingHasEscaped);
boolean requireBinding = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED);

if (scope instanceof IRClosure || requireBinding || requireFrame) {
BasicBlock entryBB = cfg.getEntryBB();
Variable savedViz = null, savedFrame = null;
if (scope instanceof IRClosure) {
savedViz = scope.createTemporaryVariable();
savedFrame = scope.createTemporaryVariable();
entryBB.addInstr(new SaveBindingVisibilityInstr(savedViz));
entryBB.addInstr(new PushBlockFrameInstr(savedFrame, scope.getName()));
if (requireBinding) entryBB.addInstr(new PushBlockBindingInstr());
entryBB.addInstr(new UpdateBlockExecutionStateInstr(Self.SELF));
Signature sig = ((IRClosure)scope).getSignature();

// If it doesn't need any args, no arg preparation involved!
int arityValue = sig.arityValue();
if (arityValue != 0) {
// Add the right kind of arg preparation instruction
if (sig.isFixed()) {
if (arityValue == 1) {
entryBB.addInstr(new PrepareSingleBlockArgInstr());
} else {
entryBB.addInstr(new PrepareFixedBlockArgsInstr());
}
} else {
entryBB.addInstr(new PrepareBlockArgsInstr(Operation.PREPARE_BLOCK_ARGS));
}
}
} else {
if (requireFrame) entryBB.addInstr(new PushMethodFrameInstr(scope.getName()));
if (requireBinding) entryBB.addInstr(new PushMethodBindingInstr());
}

// SSS FIXME: We are doing this conservatively.
// Only scopes that have unrescued exceptions need a GEB.
//
// Allocate GEB if necessary for popping
BasicBlock geb = cfg.getGlobalEnsureBB();
if (geb == null) {
Variable exc = scope.createTemporaryVariable();
geb = new BasicBlock(cfg, Label.getGlobalEnsureBlockLabel());
geb.addInstr(new ReceiveJRubyExceptionInstr(exc)); // JRuby Implementation exception handling
geb.addInstr(new ThrowExceptionInstr(exc));
cfg.addGlobalEnsureBB(geb);
}
// SSS FIXME: We are doing this conservatively.
// Only scopes that have unrescued exceptions need a GEB.
//
// Allocate GEB if necessary for popping
BasicBlock geb = cfg.getGlobalEnsureBB();
boolean gebProcessed = false;
if (geb == null) {
Variable exc = scope.createTemporaryVariable();
geb = new BasicBlock(cfg, Label.getGlobalEnsureBlockLabel());
geb.addInstr(new ReceiveJRubyExceptionInstr(exc)); // JRuby Implementation exception handling
geb.addInstr(new ThrowExceptionInstr(exc));
cfg.addGlobalEnsureBB(geb);
}

// Pop on all scope-exit paths
for (BasicBlock bb: cfg.getBasicBlocks()) {
Instr i = null;
ListIterator<Instr> instrs = bb.getInstrs().listIterator();
while (instrs.hasNext()) {
i = instrs.next();
// Right now, we only support explicit call protocol on methods.
// So, non-local returns and breaks don't get here.
// Non-local-returns and breaks are tricky since they almost always
// throw an exception and we don't multiple pops (once before the
// return/break, and once when the exception is caught).
if (!bb.isExitBB() && i instanceof ReturnBase) {
if (requireBinding || requireFrame) {
fixReturn(scope, (ReturnBase)i, instrs);
}
// Add before the break/return
instrs.previous();
if (requireBinding) instrs.add(new PopBindingInstr());
if (requireFrame) instrs.add(new PopMethodFrameInstr());
break;
// Pop on all scope-exit paths
for (BasicBlock bb: cfg.getBasicBlocks()) {
Instr i = null;
ListIterator<Instr> instrs = bb.getInstrs().listIterator();
while (instrs.hasNext()) {
i = instrs.next();
// Right now, we only support explicit call protocol on methods.
// So, non-local returns and breaks don't get here.
// Non-local-returns and breaks are tricky since they almost always
// throw an exception and we don't multiple pops (once before the
// return/break, and once when the exception is caught).
if (!bb.isExitBB() && i instanceof ReturnBase) {
if (requireBinding || requireFrame) {
fixReturn(scope, (ReturnBase)i, instrs);
}
// Add before the break/return
instrs.previous();
popSavedState(scope, requireBinding, requireFrame, savedViz, savedFrame, instrs);
if (bb == geb) gebProcessed = true;
break;
}
}

if (bb.isExitBB() && !bb.isEmpty()) {
// Last instr could be a return -- so, move iterator one position back
if (i != null && i instanceof ReturnBase) {
if (requireBinding || requireFrame) {
fixReturn(scope, (ReturnBase)i, instrs);
}
instrs.previous();
if (bb.isExitBB() && !bb.isEmpty()) {
// Last instr could be a return -- so, move iterator one position back
if (i != null && i instanceof ReturnBase) {
if (requireBinding || requireFrame) {
fixReturn(scope, (ReturnBase)i, instrs);
}
if (requireBinding) instrs.add(new PopBindingInstr());
if (requireFrame) instrs.add(new PopMethodFrameInstr());
instrs.previous();
}
popSavedState(scope, requireBinding, requireFrame, savedViz, savedFrame, instrs);
if (bb == geb) gebProcessed = true;
}

if (bb == geb) {
// Add before throw-exception-instr which would be the last instr
if (i != null) {
// Assumption: Last instr should always be a control-transfer instruction
assert i.getOperation().transfersControl(): "Last instruction of GEB in scope: " + scope + " is " + i + ", not a control-xfer instruction";
instrs.previous();
}
if (requireBinding) instrs.add(new PopBindingInstr());
if (requireFrame) instrs.add(new PopMethodFrameInstr());
if (!gebProcessed && bb == geb) {
// Add before throw-exception-instr which would be the last instr
if (i != null) {
// Assumption: Last instr should always be a control-transfer instruction
assert i.getOperation().transfersControl(): "Last instruction of GEB in scope: " + scope + " is " + i + ", not a control-xfer instruction";
instrs.previous();
}
popSavedState(scope, requireBinding, requireFrame, savedViz, savedFrame, instrs);
}
}

// This scope has an explicit call protocol flag now
scope.setExplicitCallProtocolFlag();
}

// This scope has an explicit call protocol flag now
scope.setExplicitCallProtocolFlag();

// LVA information is no longer valid after the pass
// FIXME: Grrr ... this seems broken to have to create a new object to invalidate
(new LiveVariableAnalysis()).invalidate(scope);
24 changes: 24 additions & 0 deletions core/src/main/java/org/jruby/runtime/MixedModeIRBlockBody.java
Original file line number Diff line number Diff line change
@@ -89,6 +89,30 @@ public String getName() {
return closure.getName();
}

@Override
protected IRubyObject callDirect(ThreadContext context, Block block, IRubyObject[] args, Block blockArg) {
if (callCount >= 0) promoteToFullBuild(context);
CompiledIRBlockBody jittedBody = this.jittedBody;
if (jittedBody != null) {
return jittedBody.callDirect(context, block, args, blockArg);
}

context.setCurrentBlockType(Block.Type.PROC);
return Interpreter.INTERPRET_BLOCK(context, block, null, interpreterContext, args, block.getBinding().getMethod(), blockArg);
}

@Override
protected IRubyObject yieldDirect(ThreadContext context, Block block, IRubyObject[] args, IRubyObject self) {
if (callCount >= 0) promoteToFullBuild(context);
CompiledIRBlockBody jittedBody = this.jittedBody;
if (jittedBody != null) {
return jittedBody.yieldDirect(context, block, args, self);
}

context.setCurrentBlockType(Block.Type.NORMAL);
return Interpreter.INTERPRET_BLOCK(context, block, self, interpreterContext, args, block.getBinding().getMethod(), Block.NULL_BLOCK);
}

protected IRubyObject commonYieldPath(ThreadContext context, Block block, IRubyObject[] args, IRubyObject self, Block blockArg) {
if (callCount >= 0) promoteToFullBuild(context);

2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/runtime/Signature.java
Original file line number Diff line number Diff line change
@@ -74,7 +74,7 @@ public boolean restKwargs() {
public boolean hasRest() { return rest != Rest.NONE; }

/**
* Are their an exact (fixed) number of parameters to this signature?
* Are there an exact (fixed) number of parameters to this signature?
*/
public boolean isFixed() {
return arityValue() >= 0;