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

Commits on Jan 9, 2017

  1. Copy the full SHA
    b8e306f View commit details
  2. Copy the full SHA
    63744ff View commit details
  3. Copy the full SHA
    fdb3896 View commit details
  4. reproduce and resolve generated 'tableswitch' byte-code problem

    ... [2, 4] was considered consecutive generating corrupt jump code!
    
    fixes #4429
    kares committed Jan 9, 2017
    Copy the full SHA
    fa9060d View commit details
46 changes: 27 additions & 19 deletions core/src/main/java/org/jruby/compiler/impl/SkinnyMethodAdapter.java
Original file line number Diff line number Diff line change
@@ -563,35 +563,43 @@ public void start() {
getMethodVisitor().visitCode();
getMethodVisitor().visitLabel(start);
}


static final Runnable NO_LOCALS = new Runnable() { public void run() { /* no-op */ } };

public void end() {
end(new Runnable() {
public void run() {
}
});
end(NO_LOCALS);
}

public void end(Runnable locals) {
if (DEBUG) {
PrintWriter pw = new PrintWriter(System.out);
String className = "(unknown class)";
if (cv instanceof ClassWriter) {
className = new ClassReader(((ClassWriter)cv).toByteArray()).getClassName();
}
if (name != null) {
pw.write("*** Dumping " + className + "." + name + " ***\n");
} else {
pw.write("*** Dumping ***\n");
}
printer.print(pw);
pw.flush();
}
if (DEBUG) printByteCode(getClassName());
getMethodVisitor().visitLabel(end);
locals.run();
getMethodVisitor().visitMaxs(1, 1);
getMethodVisitor().visitEnd();
}

private String getClassName() {
if (cv instanceof ClassWriter) {
return new ClassReader(((ClassWriter) cv).toByteArray()).getClassName();
}
return "(unknown class)";
}

private PrintWriter outDebugWriter() {
return new PrintWriter(System.out);
}

private void printByteCode(final String className) {
PrintWriter pw = outDebugWriter();
if (name != null) {
pw.write("*** Dumping " + className + '.' + name + " ***\n");
} else {
pw.write("*** Dumping ***\n");
}
if (printer != null) printer.print(pw);
pw.flush();
}

public void local(int index, String name, Class type) {
getMethodVisitor().visitLocalVariable(name, ci(type), null, start, end, index);
}
Original file line number Diff line number Diff line change
@@ -212,7 +212,7 @@ public static byte[] createHandleBytes(Method method, String name) {
private static String createHandleName(Method method) {
Class returnType = method.getReturnType();
Class[] paramTypes = method.getParameterTypes();
return method.getDeclaringClass().getCanonicalName().replaceAll("\\.", "__") + "#" + method.getName() + "#" + JITCompiler.getHashForString(pretty(returnType, paramTypes));
return method.getDeclaringClass().getCanonicalName().replaceAll("\\.", "__") + '#' + method.getName() + '#' + JITCompiler.getHashForString(pretty(returnType, paramTypes));
}

public static void loadUnboxedArgument(SkinnyMethodAdapter m, int index, Class type) {
13 changes: 9 additions & 4 deletions core/src/main/java/org/jruby/ir/instructions/BSwitchInstr.java
Original file line number Diff line number Diff line change
@@ -127,9 +127,14 @@ public Label[] getJumpTargets() {
return jumpTargets;
}

private static boolean jumpsAreSorted(int[] jumps) {
int[] jumps2 = jumps.clone();
Arrays.sort(jumps2);
return Arrays.equals(jumps, jumps2);
private static boolean jumpsAreSorted(final int[] jumps) {
if ( jumps.length == 0 ) return true;
int prev = jumps[0];
for ( int i = 1; i < jumps.length; i++ ) {
int curr = jumps[i];
if ( prev > curr ) return false; // not sorted
prev = curr;
}
return true;
}
}
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/ir/operands/Label.java
Original file line number Diff line number Diff line change
@@ -38,7 +38,7 @@ public OperandType getOperandType() {

@Override
public String toString() {
return prefix + "_" + id + ":" + targetPC;
return prefix + '_' + id + ':' + targetPC;
}

@Override
@@ -101,7 +101,7 @@ public static Label decode(IRReaderDecoder d) {
// Check if this label was already created
// Important! Program would not be interpreted correctly
// if new name will be created every time
String fullLabel = prefix + "_" + id;
String fullLabel = prefix + '_' + id;
if (d.getVars().containsKey(fullLabel)) {
return (Label) d.getVars().get(fullLabel);
}
11 changes: 6 additions & 5 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -862,11 +862,12 @@ public void BSwitchInstr(BSwitchInstr bswitchinstr) {
for (int i = 0; i < targets.length; i++) jvmTargets[i] = getJVMLabel(targets[i]);

// if jump table is all contiguous values, use a tableswitch
int[] jumps = bswitchinstr.getJumps();
int low = jumps[0];
int high = jumps[jumps.length - 1];
int span = high - low;
if (span == jumps.length) {
int[] jumps = bswitchinstr.getJumps(); // always ordered e.g. [2, 3, 4]

int low = jumps[0]; // 2
int high = jumps[jumps.length - 1]; // 4
int span = high - low + 1;
if (span == jumps.length) { // perfectly compact - no "holes"
jvmAdapter().tableswitch(low, high, getJVMLabel(bswitchinstr.getElseTarget()), jvmTargets);
} else {
jvmAdapter().lookupswitch(getJVMLabel(bswitchinstr.getElseTarget()), bswitchinstr.getJumps(), jvmTargets);
57 changes: 56 additions & 1 deletion test/jruby/test_case.rb
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ def test_case_with_no_expression
end

def test_case_with_ranges
x = nil
case 10
when 1..3
x = 'a'
@@ -49,6 +48,62 @@ def test_case_with_else
assert_equal('c', x)
end

def test_case_consecutive
params = [1, 2, 3, 5, 10, 11, 14, 15, 16, 17, 8, 9]
expect = [10, 20, 30, 50, 0, 10, 40, 0, 10, 20, 8, 9]
assert_equal expect, params.map { |p| case_12345(p) }
end

def case_12345(p)
p = p % 5 if p >= 10
case p
when 1
10
when 2
20
when 3
30
when 4
40
when 5
50
else return p
end
end
private :case_12345

def test_case_with_holes
params = [1, 2, 3, 5, 10, 11, 15, 16, 18, 8]
expect = [10, nil, 30, 50, nil, 10, nil, 10, 30, nil]
assert_equal expect, params.map { |p| case_135(p) }
end

def case_135(p)
p = p % 5 if p >= 10
case p
when 1 then 10
when 3 then 30
when 5 then 50
end
end

def test_case_24 # GH-4429
args_21 = [1, 2]; args_22 = [2, 3]; args_23 = ['3', '4']
args_41 = [1, 2, nil, 3]; args_42 = ['2', '3', '4', '5']
expect = [2, 3, '4', 3, '5']
assert_equal expect, [args_21, args_22, args_23, args_41, args_42].map { |a| case_24(*a) }
assert_raise(ArgumentError) { case_24 }
assert_raise(ArgumentError) { case_24(1) }
end

def case_24(*args)
case s = args.size
when 2 then args[1]
when 4 then args[3]
else raise ArgumentError.new("size: #{s} " + args.inspect)
end
end

def test_case_no_match_returns_nil
x = case nil
when String then "HEH1"