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