RFR(M): 8172850: Anti-dependency on membar causes crash in register allocator due to invalid instruction scheduling

[Vladimir Kozlov]

Good. But make sure that membar in GC barrier does not allow oop store 
to move below it. It should not since membar's memory edge should point 
to it but you changed address type to RAW so I am worry. Please, check.

Otherwise changes are good.

Thanks,
Vladimir

On 1/20/17 5:55 AM, Tobias Hartmann wrote:
> Hi,
>
> please review the following fix:
> https://bugs.openjdk.java.net/browse/JDK-8172850
> http://cr.openjdk.java.net/~thartmann/8172850/v1/webrev.00/
>
> TestMembarDependencies::test1 emits a NULL check for f2 (see line 90) that is represented as CmpN(LoadN(MEM), ConN)) and matched to testN_mem_reg0(MEM) on x86 where MEM is the memory immediately after the m1() call. The problem now is that with 8172145 [1], the MemBarVolatile emitted for the GC barrier at "f1 = obj" has an anti-dependency with the LoadN(MEM). As a result, C2 cannot schedule the testN_mem_reg0 in the block where the result is needed but has to schedule it "early", right after the call to m1. We then fail because the register allocator tries to spill the flag register to keep the result of the testN instruction live until it's needed.
>
> A slightly different and very simple version of the test triggers a bailout in block local scheduling for similar reasons (see comments in TestMembarDependencies::test2()).
>
> This only affects volatile membars that are emitted for the GC barrier because they don't have a wide memory effect in the C2 IR after the fix for 8087341 [2]. Backing out 8087341 solves the problem because the membar kills all memory and the testN_mem_reg0 is wired to its memory output and must be scheduled after.
>
> I fixed this by propagating the adr_type of the membar to the corresponding MachNode (similar to what Roland did in the Shenandoah repository [3]) and set the adr_type of the volatile membar to AliasIdxRaw when emitted for the GC barrier. Like this, the barrier membar is never anti-dependent on any other load and testN_mem_reg0 can be scheduled in the block where the result is needed.
>
> I also added missing NULL initialization and asserts to catch accidental spilling of the flags register.
>
> Thanks to Roland for his help!
>
> Tested with replay compilation, regression test and RBT (running).
>
> Thanks,
> Tobias
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8172145
> [2] https://bugs.openjdk.java.net/browse/JDK-8087341
> [3] http://hg.openjdk.java.net/shenandoah/jdk9/hotspot/rev/cb127be5c910
>