RFR(M): 8229495: SIGILL in C2 generated OSR compilation
Christian Hagedorn
christian.hagedorn at oracle.com
Wed Jul 8 13:41:30 UTC 2020
Hi Roland
That's a nice solution and looks reasonable to me. Thanks for the
detailed explanation!
I submitted some testing.
Some minor general comments:
1824 // Add back the predicate for the value at the beginning of
the first entry
1825 prev_proj = clone_skeleton_predicate(iff, init, max_value,
entry, proj, ctrl, outer_loop, prev_proj);
This comment seems to be outdated as you now clone both skeleton
predicates with the same function call in different loop iterations.
- In loopopts.cpp: While fixing the spacing you could also add curly
braces to the one-liner if statements like
955 if (n_op == Op_MergeMem) return n;
> I implemented this by using 2 subclasses to Opaque1 to denotate init and
> stride and facilitate pattern matching. I had to extend _class_id to
> juint to make Opaque1 a new class.
While at it, you might want to consider to update other uses of the
pattern Opcode() == Op_Opaque1 by is_Opaque1() as well like in
loopTransform.cpp:
1158 assert(iff->in(1)->in(1)->Opcode() == Op_Opaque1, "unexpected
predicate shape");
> Finally, the "asserts" above used to be removed at the
> end of compilation. I now leave them in debug builds so we can catch
> similar bugs earlier.
That's helpful!
> The bug doesn't reproduce with the included test case anymore but after
> I backed out a couple unrelated changes I could use the test case to
> verify the bug is indeed fixed.
I observed a Java Fuzzer crash ("fatal error: DEBUG MESSAGE: duplicated
predicate failed which is impossible") this weekend which looked very
similar to this bug and indeed it could be fixed with your patch. You
could add it as additional testcase. Here is the simplified code and the
command line I used to reproduce it.
$ java -Xcomp -XX:-TieredCompilation -XX:CompileOnly=Test::test Test.java
public class Test {
public static int iFld = 0;
public static short sFld = 1;
public static void main(String[] strArr) {
test();
}
public static int test() {
int x = 11;
int y = 0;
int j = 0;
int iArr[] = new int[400];
init(iArr);
for (int i = 0; i < 2; i++) {
doNothing();
for (j = 10; j > 1; j -= 2) {
sFld += (short)j;
iArr = iArr;
y += (j * 3);
x = (iArr[j - 1]/ x);
x = sFld;
}
int k = 1;
while (++k < 8) {
iFld += x;
}
}
return Float.floatToIntBits(654) + x + j + y;
}
// Inlined
public static void doNothing() {
}
// Inlined
public static void init(int[] a) {
for (int j = 0; j < a.length; j++) {
a[j] = 0;
}
}
}
Best regards,
Christian
More information about the hotspot-compiler-dev
mailing list