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

Commits on May 20, 2016

  1. Disable the AddLocalVarLoadStore pass to fix #3891.

    This was a good experiment, but we're not properly ensuring the
    heap variables are being loaded live when needed, causing examples
    like that in #3891 to fail to propagate changes across threads.
    By implementing LocalVariable load/store logic in JIT and turning
    off the "Add" pass, we basically revert heap vars to always being
    read/written immediately, as in JRuby 1.7.25.
    
    It may be possible to improve the pass so that it localizes the
    loads and stores better and ensures we don't miss updates we
    should see, but this commit will test whether the "nuclear option"
    passes all our suites.
    headius committed May 20, 2016
    Copy the full SHA
    cf2df89 View commit details
  2. Fixes to get LocalVariable compiling in all contexts.

    Many places just used jvmStoreLocal to store the variable, which
    assumed (because we only ran with call protocol in place) that
    all such stores would be to Java locals. I refactored this method
    to support LocalVariable as well as a different form that avoids
    stack-juggling to insert the value into the scope. This appears to
    get almost all code compiling that compiled before.
    headius committed May 20, 2016
    Copy the full SHA
    1c183c0 View commit details
  3. Copy the full SHA
    88a8896 View commit details
  4. Move OptDelegationPass after OptDynScope so we have temp locals.

    We can't store Block in heap scope, so we need this pass to come
    later.
    headius committed May 20, 2016
    Copy the full SHA
    6f37585 View commit details

Commits on May 26, 2016

  1. Merge pull request #3898 from headius/disable_add_loadstore

    Disable the AddLocalVarLoadStore pass to fix #3891.
    headius committed May 26, 2016
    Copy the full SHA
    1c5c931 View commit details
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/ir/IRManager.java
Original file line number Diff line number Diff line change
@@ -30,7 +30,7 @@
public class IRManager {
public static final String SAFE_COMPILER_PASSES = "";
public static final String DEFAULT_BUILD_PASSES = "";
public static final String DEFAULT_JIT_PASSES = "LocalOptimizationPass,OptimizeDelegationPass,DeadCodeElimination,AddLocalVarLoadStoreInstructions,OptimizeDynScopesPass,AddCallProtocolInstructions,EnsureTempsAssigned";
public static final String DEFAULT_JIT_PASSES = "LocalOptimizationPass,DeadCodeElimination,OptimizeDynScopesPass,OptimizeDelegationPass,AddCallProtocolInstructions,EnsureTempsAssigned";
public static final String DEFAULT_INLINING_COMPILER_PASSES = "LocalOptimizationPass";

private final CompilerPass deadCodeEliminationPass = new DeadCodeElimination();
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@
import org.jruby.ir.instructions.CopyInstr;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.instructions.ReifyClosureInstr;
import org.jruby.ir.operands.LocalVariable;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.representations.BasicBlock;
@@ -41,6 +42,11 @@ private static void optimizeDelegatedVars(IRScope s) {
for (Instr i: bb.getInstrs()) {
if (i instanceof ReifyClosureInstr) {
ReifyClosureInstr ri = (ReifyClosureInstr) i;

// can't store un-reified block in DynamicScope (only accepts IRubyObject)
// FIXME: (con) it would be nice to not have this limitation
if (ri.getResult() instanceof LocalVariable) continue;

unusedExplicitBlocks.put(ri.getResult(), ri.getSource());
} else {
Iterator<Operand> it = unusedExplicitBlocks.keySet().iterator();
119 changes: 110 additions & 9 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -390,7 +390,43 @@ private org.objectweb.asm.Label getJVMLabel(Label label) {
}

private void jvmStoreLocal(Variable variable) {
if (variable instanceof TemporaryLocalVariable) {
if (variable instanceof LocalVariable) {

LocalVariable localvariable = (LocalVariable) variable;

jvmLoadLocal(DYNAMIC_SCOPE);

jvmAdapter().swap();

if (localvariable.getScopeDepth() == 0) {

switch (localvariable.getOffset()) {
case 0:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueZeroDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
case 1:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueOneDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
case 2:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueTwoDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
case 3:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueThreeDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
default:
jvmAdapter().ldc(localvariable.getOffset());
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueDepthZeroVoid", sig(void.class, IRubyObject.class, int.class));
break;
}

} else {

jvmAdapter().ldc(localvariable.getOffset());
jvmAdapter().ldc(localvariable.getScopeDepth());
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueVoid", sig(void.class, IRubyObject.class, int.class, int.class));

}
} else if (variable instanceof TemporaryLocalVariable) {
switch (((TemporaryLocalVariable)variable).getType()) {
case FLOAT: jvmAdapter().dstore(getJVMLocalVarIndex(variable)); break;
case FIXNUM: jvmAdapter().lstore(getJVMLocalVarIndex(variable)); break;
@@ -402,6 +438,62 @@ private void jvmStoreLocal(Variable variable) {
}
}

private void jvmStoreLocal(Runnable source, Variable variable) {
if (variable instanceof LocalVariable) {

LocalVariable localvariable = (LocalVariable) variable;

jvmLoadLocal(DYNAMIC_SCOPE);

if (localvariable.getScopeDepth() == 0) {

source.run();

switch (localvariable.getOffset()) {
case 0:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueZeroDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
case 1:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueOneDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
case 2:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueTwoDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
case 3:
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueThreeDepthZeroVoid", sig(void.class, IRubyObject.class));
break;
default:
jvmAdapter().ldc(localvariable.getOffset());
jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueDepthZeroVoid", sig(void.class, IRubyObject.class, int.class));
break;
}

} else {

source.run();

jvmAdapter().ldc(localvariable.getOffset());
jvmAdapter().ldc(localvariable.getScopeDepth());

jvmAdapter().invokevirtual(p(DynamicScope.class), "setValueVoid", sig(void.class, IRubyObject.class, int.class, int.class));

}
} else if (variable instanceof TemporaryLocalVariable) {
source.run();

switch (((TemporaryLocalVariable)variable).getType()) {
case FLOAT: jvmAdapter().dstore(getJVMLocalVarIndex(variable)); break;
case FIXNUM: jvmAdapter().lstore(getJVMLocalVarIndex(variable)); break;
case BOOLEAN: jvmAdapter().istore(getJVMLocalVarIndex(variable)); break;
default: jvmMethod().storeLocal(getJVMLocalVarIndex(variable)); break;
}
} else {
source.run();

jvmMethod().storeLocal(getJVMLocalVarIndex(variable));
}
}

private void jvmStoreLocal(String specialVar) {
jvmMethod().storeLocal(getJVMLocalVarIndex(specialVar));
}
@@ -1017,14 +1109,23 @@ public void ConstMissingInstr(ConstMissingInstr constmissinginstr) {
public void CopyInstr(CopyInstr copyinstr) {
Operand src = copyinstr.getSource();
Variable res = copyinstr.getResult();
if (res instanceof TemporaryFloatVariable) {
loadFloatArg(src);
} else if (res instanceof TemporaryFixnumVariable) {
loadFixnumArg(src);
} else {
visit(src);
}
jvmStoreLocal(res);

storeHeapOrStack(src, res);
}

private void storeHeapOrStack(final Operand value, final Variable res) {
jvmStoreLocal(new Runnable() {
@Override
public void run() {
if (res instanceof TemporaryFloatVariable) {
loadFloatArg(value);
} else if (res instanceof TemporaryFixnumVariable) {
loadFixnumArg(value);
} else {
visit(value);
}
}
}, res);
}

@Override
49 changes: 49 additions & 0 deletions core/src/main/java/org/jruby/runtime/DynamicScope.java
Original file line number Diff line number Diff line change
@@ -219,6 +219,17 @@ public IRubyObject setValue(IRubyObject value, int offset, int depth) {
return setValue(offset, value, depth);
}

/**
* Set value in current dynamic scope or one of its captured scopes.
*
* @param offset zero-indexed value that represents where variable lives
* @param value to set
* @param depth how many captured scopes down this variable should be set
*/
public void setValueVoid(IRubyObject value, int offset, int depth) {
setValue(offset, value, depth);
}

/**
* setValue for depth zero
*
@@ -227,26 +238,64 @@ public IRubyObject setValue(IRubyObject value, int offset, int depth) {
*/
public abstract IRubyObject setValueDepthZero(IRubyObject value, int offset);

/**
* setValue for depth zero
*
* @param value to set
* @param offset zero-indexed value that represents where variable lives
*/
public void setValueDepthZeroVoid(IRubyObject value, int offset) {
setValueDepthZero(value, offset);
}

/**
* Set value zero in this scope;
*/
public abstract IRubyObject setValueZeroDepthZero(IRubyObject value);

/**
* Set value zero in this scope;
*/
public void setValueZeroDepthZeroVoid(IRubyObject value) {
setValueZeroDepthZero(value);
}

/**
* Set value one in this scope.
*/
public abstract IRubyObject setValueOneDepthZero(IRubyObject value);

/**
* Set value one in this scope.
*/
public void setValueOneDepthZeroVoid(IRubyObject value) {
setValueOneDepthZero(value);
}

/**
* Set value two in this scope.
*/
public abstract IRubyObject setValueTwoDepthZero(IRubyObject value);

/**
* Set value two in this scope.
*/
public void setValueTwoDepthZeroVoid(IRubyObject value) {
setValueTwoDepthZero(value);
}

/**
* Set value three in this scope.
*/
public abstract IRubyObject setValueThreeDepthZero(IRubyObject value);

/**
* Set value three in this scope.
*/
public void setValueThreeDepthZeroVoid(IRubyObject value) {
setValueThreeDepthZero(value);
}

@Override
public String toString() {
return toString(new StringBuffer(), "");