[14] RFR(S): 8229694: JVM crash in SWPointer during C2 OSR compilation

[Vladimir Kozlov]

Looks good. It looks like you now know our vector optimizations ;)
Please file a RFE, as you suggested, to clean this up later.

Thanks,
Vladimir

On 11/6/19 2:13 AM, Christian Hagedorn wrote:
> Hi
> 
> Please review the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8229694
> http://cr.openjdk.java.net/~chagedorn/8229694/webrev.00/
> 
> The JVM crashes in the testcase when trying to dereference mem at [1] which is NULL. This happens 
> when converting packs into vector nodes in SuperWord::output() by reading an unexpected NULL value 
> with SuperWord::align_to_ref() [2]. The corresponding field _align_to_ref is set to NULL at [3] 
> since best_align_to_mem_ref was assigned NULL just before at [4]. _packset only contained one pack 
> and there were no memory operations left to be processed (memops is empty). As a result 
> SuperWord::find_align_to_ref() will return NULL since it needs at least two operations to find an 
> alignment.
> 
> The fix is straight forward to directly use the alignment of the only pack remaining if there are no 
> memory operations left in memops to find another alignment.
> 
> 
> The testcase creates such a situation where only one pack remains at [4] when the loop is unrolled 
> four times. When calling SuperWord::find_adjacent_refs() there are:
> - 4 StoreI for intArr[j-1] = 400
> - 4 StoreC for shortArr[j] = 30
> - 2 StoreI for intArr[7] = 260 // Initially 4 but 2 are removed by IGVN in Ideal()
> - 2 StoreC for shortArr[10] = 10 // Initially 4 but 2 are removed by IGVN in Ideal()
> - 2 LoadI (and 2 StoreI) for iFld = intArr[j] // Initially 4 each but 2 of each are removed by IGVN 
> in Ideal()
> 
> The field stores are obviously ignored for the superword algorithm. intArr[j-1] aligns with 
> intArr[7] and therefore create_pack is true. The only pack created is one with two immediately 
> following stores for intArr[j-1]. The IGVN algorithm is not able to remove the first redundant store 
> to intArr[7] when the loop is unrolled the first time. Only when unrolling it again the second time, 
> it is able to remove the two newly created redundant stores to intArr[7]. This leaves us with the 
> following depencendies of stores: "intArr[j-1] -> intArr[j-1] -> intArr[7] -> intArr[j-1] -> 
> intArr[7] -> intArr[j-1]" from which only the first two operations can be used to create a pack.
> 
> The very same applies to the StoreC nodes. As a result, one pack for StoreI and one for StoreC are 
> created in total. There are now only the two LoadI nodes of intArr[j] left which are not aligned 
> with intArr[j-1]. Therefore, all StoreI packs are removed at [5]. This leaves us with exactly one 
> ShortC pack and an empty memops list which sets the alignment to NULL and eventually lets the JVM 
> crash at [1].
> 
> We might want to file an RFE to investigate further why IGVN cannot remove the first redundant 
> stores to intArr[7], shortArr[10], and iFld, respectively (even though it's quite useless to keep 
> setting the same values in a loop). This problem can also be observed if the loop only contains the 
> statement "iFld = intArr[j]". But I think even if those redundant stores would have been optimized 
> away we should have this fix to handle the situation with only one pack and no memory operations 
> remaining.
> 
> 
> Thank you!
> 
> Best regards,
> Christian
> 
> 
> [1] http://hg.openjdk.java.net/jdk/jdk/file/f61eea1869e4/src/hotspot/share/opto/superword.cpp#l3608
> [2] http://hg.openjdk.java.net/jdk/jdk/file/f61eea1869e4/src/hotspot/share/opto/superword.cpp#l2328
> [3] http://hg.openjdk.java.net/jdk/jdk/file/f61eea1869e4/src/hotspot/share/opto/superword.cpp#l732
> [4] http://hg.openjdk.java.net/jdk/jdk/file/f61eea1869e4/src/hotspot/share/opto/superword.cpp#l708
> [5] http://hg.openjdk.java.net/jdk/jdk/file/f61eea1869e4/src/hotspot/share/opto/superword.cpp#l688