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: 7b8a61f8a44e
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 8d34a507cc0b
Choose a head ref
  • 2 commits
  • 4 files changed
  • 1 contributor

Commits on Oct 12, 2014

  1. (Closure)LocalVariable hashCode should include both name & offset. 

    * Since an identically-named local variable in a nested closure refers
  to the same runtime variable as a local variable in the parent scope,
  in IR, we consider them "same" variables as well (for the purposes of
  cloning, LVA, etc.).

  However, because of shadowing when variables are declared in block
  args, to prevent mixing up different variables across scopes, we have
  to include offset in the hashCode. See code snippet below where
  _, n, s shadow similarly named variables in surrounding scopes.

  FIXME: Do we need a different LocalVariable type for these?

-------------------------------------
  def yielder
    [1,2].each { |i| yield "boo", i }
  end

  def foo
    yielder do |_, n|
      p "1: n=#{n}"
      yielder do |s, _|
        p "2: s=#{s}"
        yielder do |n,s|
          p "3: n=#{n}, s=#{s}"
        end
      end
    end
  end

  foo
-------------------------------------
    @subbuss
    subbuss committed Oct 12, 2014
    Copy the full SHA
    2fe209d View commit details

  2. Clone instructions before interpretation. 

    * Followup on 7a8bf5f.

* Prevent JIT/opts passes from clobbering the instructions that
  interpreter is working on. This sets the stage for enabling
  multiple versions of a scope to be active on the execution stack
  at the same time.

* The following fields in IRScope are still referenced at runtime
  Of these, 3 are immutable and are safe. But, there are 2 fields
  that could be clobbered by opt passes / JIT and need to made
  available to the interpreter context.
  - scope name -- immutable
  - scope type -- immutable
  - parameter list -- immutable
  - tmp var count
  - scope ir flags
    @subbuss
    subbuss committed Oct 12, 2014
    2
    Copy the full SHA
    8d34a50 View commit details
10 changes: 8 additions & 2 deletions core/src/main/java/org/jruby/ir/IRClosure.java
View file
Original file line number Diff line number Diff line change
@@ -26,18 +26,22 @@ public class IRClosure extends IRScope {

private int nestingDepth; // How many nesting levels within a method is this closure nested in?

private BlockBody body;

private boolean isBeginEndBlock;

// Block parameters
private List<Operand> blockArgs;

/** The parameter names, for Proc#parameters */
private String[] parameterList;

private Arity arity;
private int argumentType;
public boolean addedGEBForUncaughtBreaks;

/** Added for interp/JIT purposes */
private BlockBody body;

/** Added for JIT purposes */
private Handle handle;

// Used by other constructions and by IREvalScript as well
@@ -288,6 +292,8 @@ protected IRClosure cloneForInlining(CloneInfo ii, IRClosure clone) {
// FIXME: This is fragile. Untangle this state.
// Why is this being copied over to InterpretedIRBlockBody?
clone.setParameterList(this.parameterList);
clone.addedGEBForUncaughtBreaks = this.addedGEBForUncaughtBreaks;
clone.isBeginEndBlock = this.isBeginEndBlock;

SimpleCloneInfo clonedII = ii.cloneForCloningClosure(clone);

34 changes: 24 additions & 10 deletions core/src/main/java/org/jruby/ir/IRScope.java
View file
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@
import org.jruby.ir.representations.BasicBlock;
import org.jruby.ir.representations.CFG;
import org.jruby.ir.representations.CFGLinearizer;
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.ir.transformations.inlining.CFGInliner;
import org.jruby.ir.transformations.inlining.SimpleCloneInfo;
import org.jruby.parser.StaticScope;
@@ -476,29 +477,42 @@ private synchronized Instr[] prepareInstructions() {
setupLinearization();

SimpleCloneInfo cloneInfo = new SimpleCloneInfo(this, false);
// Clear old set
initNestedClosures();

// FIXME: If CFG (or linearizedBBList) knew number of instrs we could end up allocing better

// Pass 1. Set up IPCs for labels and instruct and build linear instr list
// Pass 1. Set up IPCs for labels and instructions and build linear instr list
List<Instr> newInstrs = new ArrayList<Instr>();
int ipc = 0;
for (BasicBlock b: linearizedBBList) {
// All same-named labels must be same Java instance for this to work or we would need
// to examine all Label operands and update this as well which would be expensive.
b.getLabel().setTargetPC(ipc);
Label l = b.getLabel();
Label newL = cloneInfo.getRenamedLabel(l);
l.setTargetPC(ipc);
newL.setTargetPC(ipc);

List<Instr> bbInstrs = b.getInstrs();
int bbInstrsLength = bbInstrs.size();
for (int i = 0; i < bbInstrsLength; i++) {
Instr instr = bbInstrs.get(i);

if (instr instanceof Specializeable) {
instr = ((Specializeable) instr).specializeForInterpretation();
bbInstrs.set(i, instr);
}

if (!(instr instanceof ReceiveSelfInstr)) {
newInstrs.add(instr);
instr.setIPC(ipc);
Instr newInstr = instr.clone(cloneInfo);
// if (newInstr == instr) {
// System.out.println("Instruction " + instr.getOperation() + " returns itself on clone. Probably fragile!");
// }

if (newInstr instanceof Specializeable) {
newInstr = ((Specializeable) newInstr).specializeForInterpretation();
// Make sure debug CFG is identical to interpreted instr output
if (IRRuntimeHelpers.isDebug()) {
bbInstrs.set(i, ((Specializeable) instr).specializeForInterpretation());
}
}

newInstrs.add(newInstr);
newInstr.setIPC(ipc);
ipc++;
}
}
11 changes: 4 additions & 7 deletions core/src/main/java/org/jruby/ir/operands/ClosureLocalVariable.java
View file
Original file line number Diff line number Diff line change
@@ -16,23 +16,20 @@ public ClosureLocalVariable(IRClosure scope, String name, int scopeDepth, int lo
this.definingScope = scope;
}

@Override
public int hashCode() {
return name.hashCode();
}

@Override
public boolean equals(Object obj) {
if (obj == null || !(obj instanceof ClosureLocalVariable)) return false;

return name.equals(((LocalVariable) obj).name);
return hashCode() == obj.hashCode();
}

public int compareTo(Object arg0) {
// ENEBO: what should compareTo when it is not comparable?
if (!(arg0 instanceof ClosureLocalVariable)) return 0;

return name.compareTo(((LocalVariable) arg0).name);
int a = hashCode();
int b = arg0.hashCode();
return a < b ? -1 : (a == b ? 0 : 1);
}

@Override
11 changes: 8 additions & 3 deletions core/src/main/java/org/jruby/ir/operands/LocalVariable.java
View file
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ public class LocalVariable extends Variable implements DepthCloneable {
protected final String name;
protected final int scopeDepth;
protected final int offset;
protected final int hcode;

// FIXME: We should resolve to an index into an array but localvariable has no allocator
public LocalVariable(String name, int scopeDepth, int location) {
@@ -30,6 +31,7 @@ protected LocalVariable(OperandType type, String name, int scopeDepth, int locat
this.name = name;
this.scopeDepth = scopeDepth;
this.offset = location;
this.hcode = (name + ":" + offset).hashCode();
}

public boolean isSameDepth(LocalVariable other) {
@@ -60,20 +62,23 @@ public String toString() {

@Override
public int hashCode() {
return name.hashCode();
return hcode;
}

@Override
public boolean equals(Object obj) {
if (obj == null || !(obj instanceof LocalVariable)) return false;

return name.equals(((LocalVariable) obj).name);
return hashCode() == obj.hashCode();
}

public int compareTo(Object arg0) {
// ENEBO: what should compareTo when it is not comparable?
if (!(arg0 instanceof LocalVariable)) return 0;

return name.compareTo(((LocalVariable) arg0).name);
int a = hashCode();
int b = arg0.hashCode();
return a < b ? -1 : (a == b ? 0 : 1);
}

@Override