Skip to content

Commit

Permalink
Make ICs no longer be operands
Browse files Browse the repository at this point in the history
Clone WrappedIRClosure (mostly) now
Simplify creation logic in IRClosure and IRScope with a single prepareForInterpretation.
  • Loading branch information
enebo committed Oct 20, 2014
1 parent e71749f commit 1134fe3
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 73 deletions.
16 changes: 2 additions & 14 deletions core/src/main/java/org/jruby/ir/IRClosure.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,9 @@ public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, Stati
this.nestingDepth++;
}

public InterpreterContext prepareInterpreterContext(Operand self) {
if (interpreterContext != null) return interpreterContext; // Already prepared

initScope(false);

interpreterContext = new ClosureInterpreterContext(this, prepareInstructions(), self, getBlockBody());

return interpreterContext;
}

@Override
public synchronized InterpreterContext prepareForInterpretation() {
// This should have already been prepared during preparation of parent scopes.
// If this is null, it would be a bug and let users throw a NPE.
return interpreterContext;
public InterpreterContext allocateInterpreterContext(Instr[] instructionList) {
return new ClosureInterpreterContext(this, instructionList);
}

public void setBeginEndBlock() {
Expand Down
7 changes: 6 additions & 1 deletion core/src/main/java/org/jruby/ir/IRScope.java
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,11 @@ protected void initScope(boolean jitMode) {
}
}

/** Make version specific to scope which needs it (e.g. Closure vs non-closure). */
public InterpreterContext allocateInterpreterContext(Instr[] instructionList) {
return new InterpreterContext(this, instructionList);
}

/** Run any necessary passes to get the IR ready for interpretation */
public synchronized InterpreterContext prepareForInterpretation() {
if (interpreterContext != null) return interpreterContext; // Already prepared
Expand All @@ -619,7 +624,7 @@ public synchronized InterpreterContext prepareForInterpretation() {

// System.out.println("-- passes run for: " + this + " = " + java.util.Arrays.toString(executedPasses.toArray()));

interpreterContext = new InterpreterContext(this, prepareInstructions());
interpreterContext = allocateInterpreterContext(prepareInstructions());

return interpreterContext;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco
// SSS FIXME: Copied this from ast/LambdaNode ... Is this required here as well?
//
// JRUBY-5686: do this before executing so first time sets cref module
((ClosureInterpreterContext)getLambdaBody()).getStaticScope().determineModule();
((WrappedIRClosure) getLambdaBody()).getClosure().getStaticScope().determineModule();

// CON: This must not be happening, because nil would never cast to Block
// IRClosure body = getLambdaBody().getClosure();
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/ir/interpreter/Interpreter.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public static IRubyObject interpretCommonEval(Ruby runtime, String file, int lin
// ClosureInterpreterContext never retrieved as an operand in this context.
// So, self operand is not required here.
// Passing null to force early crasher if ever used differently.
evalScript.prepareInterpreterContext(null);
evalScript.prepareForInterpretation();
ThreadContext context = runtime.getCurrentContext();

IRubyObject rv = null;
Expand Down Expand Up @@ -122,7 +122,7 @@ public static void runBeginEndBlocks(List<IRClosure> beBlocks, ThreadContext con

for (IRClosure b: beBlocks) {
// SSS FIXME: Should I piggyback on WrappedIRClosure.retrieve or just copy that code here?
b.prepareInterpreterContext(b.getSelf());
b.prepareForInterpretation();
Block blk = (Block)(new WrappedIRClosure(b.getSelf(), b)).retrieve(context, self, currScope, context.getCurrentScope(), temp);
blk.yield(context, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,15 @@

import org.jruby.ir.IRClosure;
import org.jruby.ir.instructions.Instr;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Binding;
import org.jruby.runtime.Block;
import org.jruby.runtime.BlockBody;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

/**
* Interpreter knowledge needed to interpret a closure.
*/
public class ClosureInterpreterContext extends InterpreterContext {
private Operand self;
private BlockBody body;

public ClosureInterpreterContext(IRClosure scope, Instr[] instructions,
Operand self, BlockBody body) {
public ClosureInterpreterContext(IRClosure scope, Instr[] instructions) {
super(scope, instructions);

this.self = self;
this.body = body;
}

/**
Expand All @@ -33,17 +21,4 @@ public ClosureInterpreterContext(IRClosure scope, Instr[] instructions,
public DynamicScope newDynamicScope(ThreadContext context) {
throw new RuntimeException("We do not push bindings for closures");
}

@Override
public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
getStaticScope().determineModule();

// In non-inlining scenarios, this.self will always be %self.
// However, in inlined scenarios, this.self will be the self in the original scope where the closure
// was present before inlining.
IRubyObject selfVal = (this.self instanceof Self) ? self : (IRubyObject) this.self.retrieve(context, self, currScope, currDynScope, temp);
Binding binding = context.currentBinding(selfVal, currDynScope);

return new Block(body, binding);
}
}
20 changes: 1 addition & 19 deletions core/src/main/java/org/jruby/ir/operands/InterpreterContext.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
package org.jruby.ir.operands;

import java.util.List;
import org.jruby.ir.IRFlags;
import org.jruby.ir.IRMetaClassBody;
import org.jruby.ir.IRScope;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.representations.CFG;
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;

public class InterpreterContext extends Operand {
public class InterpreterContext {
private final int temporaryVariablecount;
private final int temporaryBooleanVariablecount;
private final int temporaryFixnumVariablecount;
Expand All @@ -38,8 +35,6 @@ public class InterpreterContext extends Operand {
private CFG cfg = null;

public InterpreterContext(IRScope scope, Instr[] instructions) {
super(null);

//FIXME: Remove once we conditionally plug in CFG on debug-only
this.cfg = scope.getCFG();

Expand All @@ -60,14 +55,6 @@ public InterpreterContext(IRScope scope, Instr[] instructions) {
this.receivesKeywordArguments = scope.getFlags().contains(IRFlags.RECEIVES_KEYWORD_ARGS);
}

@Override
public void addUsedVariables(List<Variable> l) {}

@Override
public Operand cloneForInlining(CloneInfo info) {
throw new IllegalStateException("Should not clone interp context");
}

public String getFileName() {
return fileName;
}
Expand Down Expand Up @@ -127,11 +114,6 @@ public boolean receivesKeywordArguments() {
return receivesKeywordArguments;
}

@Override
public Object retrieve(ThreadContext context, IRubyObject self, StaticScope currScope, DynamicScope currDynScope, Object[] temp) {
return super.retrieve(context, self, currScope, currDynScope, temp);
}

@Override
public String toString() {
StringBuilder buf = new StringBuilder();
Expand Down
15 changes: 5 additions & 10 deletions core/src/main/java/org/jruby/ir/operands/WrappedIRClosure.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,12 @@ public Operand getSimplifiedOperand(Map<Operand, Operand> valueMap, boolean forc

@Override
public Operand cloneForInlining(CloneInfo info) {
if (info instanceof SimpleCloneInfo) {
SimpleCloneInfo simpleCloneInfo = (SimpleCloneInfo) info;
// Making interp instrs so that if JIT hits IRClosure we will not concurrently modify the same IRScope.
if (info instanceof SimpleCloneInfo && !((SimpleCloneInfo) info).isEnsureBlockCloneMode()) {
closure.prepareForInterpretation(); // makes context as a side-effect

// For IRBuilding we are pre-interpretation and do want a traditional simple clone
if (simpleCloneInfo.isEnsureBlockCloneMode()) {
return new WrappedIRClosure(info.getRenamedVariable(self), closure.cloneForInlining(info));
}

// We are saving instructions so that if JIT hits IRClosure, it will not concurrently
// modify the same object.
return closure.prepareInterpreterContext(self);
// FIXME: It really bothers me we do not clone closure here but cloning like main clone case loses interpContext + other things.
return new WrappedIRClosure(info.getRenamedVariable(self), closure);
}

return new WrappedIRClosure(info.getRenamedVariable(self), closure.cloneForInlining(info));
Expand Down

0 comments on commit 1134fe3

Please sign in to comment.