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

[Tobias Hartmann]

Hi Felix,

On 18.06.20 11:02, Yangfei (Felix) wrote:
> For the first iteration on the loop, the MergeMem node actually have three phis as input.
> It looks like:
> (gdb) p this->dump()
>  556    MergeMem        === _  1  660  655  1  657  [[ 559 ]]  { N655:rawptr:BotPTR - N657:TestReplaceEquivPhis+12 * }  Memory: @BotPTR *+bot, idx=Bot; !orig=151 !jvms: TestReplaceEquivPhis::test @ bci:25
> $2 = void
> (gdb) p in(2)->dump()
>  660    Phi     ===  679  752  583  [[ 556  533 ]]  #memory  Memory: @BotPTR *+bot, idx=Bot;
> $3 = void
> (gdb) p in(3)->dump()
>  655    Phi     ===  679  752  583  [[ 556 ]]  #memory  Memory: @rawptr:BotPTR, idx=Raw;
> $4 = void
> (gdb) p in(5)->dump()
>  657    Phi     ===  679  752  583  [[ 556  516 ]]  #memory  Memory: @TestReplaceEquivPhis+12 *, name=iFld, idx=5;
> $5 = void
> 
> After the first iteration, in(3) (i.e., in(AliasIdxRaw)) was removed from the inputs. 
> I am not sure if this transformation is correct even through it does not make a difference on app behavior. 
> After that, the MergeMem have two phi nodes as input: in(5) and in(2) ((i.e., in(AliasIdxBot))).  This is the same structure as described in my first email. 
> 
> Here, I see in(2) is also an input for node 533: 
> (gdb) p in(2)->find(533)->dump() 
>  533    LoadI   === _  660  180  [[ 517  532  559 ]]  @java/lang/Class:exact+120 *, name=instanceCount, idx=6; Volatile! #int !orig=181 !jvms: TestReplaceEquivPhis::test @ bci:39 
> 
> If we have a store to the same memory slice as in(5) after the MergeMem node, I think we might trigger one similar bug if we decide to keep in(5) here. 

(Should be "same memory slice as 533 after" as you corrected in a private email)

Right, that could be a problem.

> So I guess it might be safer to go with the initially proposed patch.  

Let me run this through some performance testing, to see if it makes a difference.

Best regards,
Tobias