[15] RFR(S): 8235332: TestInstanceCloneAsLoadsStores.java fails with -XX:+StressGCM

[Nils Eliasson]

Hi Christian,

Change looks good,

Thanks for fixing!

// Nils

On 2020-01-15 12:02, Christian Hagedorn wrote:
>>>> If there's no GC barrier then:
>>>>
>>>> Node* control_proj_ac = bs->step_over_gc_barrier(proj_in->in(0));
>>>>
>>>> control_proj_ac is not a projection, it's proj_in->in(0) so the test
>>>> below can never succeed?
>>>
>>> If there is no GC barrier, then proj_in is a membar with its control
>>> input being a control projection from the ArrayCopyNode.
>>>
>>> As an example, I have taken the test case TestEliminatedCloneBadMemEdge
>>> [1] which does not disable ReduceInitialCardMarks and has no GC 
>>> barrier.
>>> We have the following situation at [2]:
>>>
>>> proj_in is 132 MemBarCPUOrder. The original code then looked at the
>>> MergeMem 100 input following in(Compile::AliasIdxRaw) which is 101 Proj
>>> and its input 98 ArrayCopy. This passes the check.
>>>
>>> My fix now directly looks at the control input of 132 MemBarCPUOrder
>>> which is the projection 99 Proj coming from the 98 ArrayCopy. This also
>>> passes the check.
>>
>> Ah! Right. I wonder if there's a reason why the current code follows the
>> control graph (as you do) instead of the memory graph. I couldn't find
>> one and your change looks good to me.
>
> Thank you for your review Roland! Was wondering about that, too.
>
> Best regards,
> Christian