RFR(S): 8243670: Unexpected test result caused by C2 MergeMemNode::Ideal

[Tobias Hartmann]

Hi Felix,

were you able to figure out how we ended up with two Phis with same input but different _adr_type?

Thanks,
Tobias

On 28.04.20 08:02, Yangfei (Felix) wrote:
> Hi,
> 
>   Please help review this patch fixing a C2 issue.  
>       Bug: https://bugs.openjdk.java.net/browse/JDK-8243670 
>       Webrev: http://cr.openjdk.java.net/~fyang/8243670/webrev.00/
> 
> As described on the issue, C2 generates incorrect code for the following OSR compile:
>     420    4 %  b  4       TestReplaceEquivPhis::test @ 25 (107 bytes)
> 
>   v = iFld;      // load from field "iFld"             
>   iFld = TestReplaceEquivPhis.instanceCount; // store to field "iFld"
> 
> From the C2 JIT code, load and store of field "iFld" are misplaced.
> Looks like this is initially caused by the replace equivalent phis transformation in MergeMemNode::Ideal.
> 
> Call trace:
> #0  MergeMemNode::Ideal (this=0x7fff580c70c0, phase=0x7fff7d9407b0, can_reshape=true) at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/memnode.cpp:4621
> #1  0x00007ffff6020bcd in PhaseGVN::apply_ideal (this=0x7fff7d9407b0, k=0x7fff580c70c0, can_reshape=true)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/phaseX.cpp:806
> #2  0x00007ffff60223ef in PhaseIterGVN::transform_old (this=0x7fff7d9407b0, n=0x7fff580c70c0)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/phaseX.cpp:1229
> #3  0x00007ffff602217a in PhaseIterGVN::optimize (this=0x7fff7d9407b0) at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/phaseX.cpp:1175
> #4  0x00007ffff5e32618 in PhaseIdealLoop::build_and_optimize (this=0x7fff7d93fa90, mode=LoopOptsDefault)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/loopnode.cpp:3192
> #5  0x00007ffff5800831 in PhaseIdealLoop::PhaseIdealLoop (this=0x7fff7d93fa90, igvn=..., mode=LoopOptsDefault)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/loopnode.hpp:951
> #6  0x00007ffff580092c in PhaseIdealLoop::optimize (igvn=..., mode=LoopOptsDefault) at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/loopnode.hpp:1026
> #7  0x00007ffff57f4553 in Compile::optimize_loops (this=0x7fff7d942d00, igvn=..., mode=LoopOptsDefault)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/compile.cpp:1970
> #8  0x00007ffff57f5308 in Compile::Optimize (this=0x7fff7d942d00) at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/compile.cpp:2182
> #9  0x00007ffff57ee5a3 in Compile::Compile (this=0x7fff7d942d00, ci_env=0x7fff7d943810, target=0x7fff580eeb70, osr_bci=25, subsume_loads=true,
>     do_escape_analysis=true, eliminate_boxing=true, directive=0x7ffff031b430) at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/compile.cpp:736
> #10 0x00007ffff56ebc23 in C2Compiler::compile_method (this=0x7ffff035e940, env=0x7fff7d943810, target=0x7fff580eeb70, entry_bci=25, directive=0x7ffff031b430)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/opto/c2compiler.cpp:111
> #11 0x00007ffff5808cd0 in CompileBroker::invoke_compiler_on_method (task=0x7ffff03b6a10)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/compiler/compileBroker.cpp:2210
> #12 0x00007ffff58079eb in CompileBroker::compiler_thread_loop () at /home/yangfei/openjdk-jdk/src/hotspot/share/compiler/compileBroker.cpp:1894
> #13 0x00007ffff62557f1 in compiler_thread_entry (thread=0x7ffff035f800, __the_thread__=0x7ffff035f800)
>     at /home/yangfei/openjdk-jdk/src/hotspot/share/runtime/thread.cpp:3454
> #14 0x00007ffff6250a11 in JavaThread::thread_main_inner (this=0x7ffff035f800) at /home/yangfei/openjdk-jdk/src/hotspot/share/runtime/thread.cpp:1969
> #15 0x00007ffff62508bf in JavaThread::run (this=0x7ffff035f800) at /home/yangfei/openjdk-jdk/src/hotspot/share/runtime/thread.cpp:1952
> #16 0x00007ffff624cb2c in Thread::call_run (this=0x7ffff035f800) at /home/yangfei/openjdk-jdk/src/hotspot/share/runtime/thread.cpp:399
> #17 0x00007ffff5fbf288 in thread_native_entry (thread=0x7ffff035f800) at /home/yangfei/openjdk-jdk/src/hotspot/os/linux/os_linux.cpp:789
> #18 0x00007ffff71976db in start_thread (arg=0x7fff7d944700) at pthread_create.c:463
> #19 0x00007ffff78f188f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
> 
> graph before the transformation looks like:
> 
>   1: Phi1( Phi1 and Phi2 have the same input edges)  #memory  Memory: @TestReplaceEquivPhis+12 *, name=iFld, idx=5;
>   2: Phi2( Phi1 and Phi2 have the same input edges)  #memory  Memory: @BotPTR *+bot, idx=Bot;
>   3: LoadI( input: 1)   =>  name=iFld, idx=5
>   4: MergeMem( input:1, 2)
>   5: MemBarAcqure( input: 4)
>   6: Proj( input: 5)
>   7: StoreI( input: 6)   => name=iFld, idx=5
> 
> Here Phi1 and Phi2 have same input edges.  Input from Phi1 to MergeMem is simplified by MergeMemoryNode::Ideal.
> 
> graph after the transformation looks like:
> 
>   1: Phi1( Phi1 and Phi2 have the same input edges)  #memory  Memory: @TestReplaceEquivPhis+12 *, name=iFld, idx=5;
>   2: Phi2( Phi1 and Phi2 have the same input edges)  #memory  Memory: @BotPTR *+bot, idx=Bot;
>   3: LoadI( input: 1)    =>  name=iFld, idx=5
>   4: MergeMem( input: 2)
>   5: MemBarAcqure( input: 4)
>   6: Proj( input: 5)
>   7: StoreI( input: 6)    => name=iFld, idx=5
> 
> As a result, PhaseCFG::insert_anti_dependences won't insert an anti-dependence edge between 3 and 4 to place the load correctly.
> This transformation is there from day one.  With -XX:-SplitIfBlocks option, it triggers more errors.  Proposed webrev simply disables it.
> Tier1-3 tested on x86-64-linux-gnu.  Specjbb2015 shows no performance regression with this change.
> Suggestions?
> 
> Thanks,
> Felix
>