RFR(S) 8251271- C2: Compile::_for_igvn list is corrupted after RenumberLiveNodes

[Christian Hagedorn]

Hi Xin

I'm not sure if we really want to copy all the nodes from new_worklist 
back to for_igvn() when it's not used anymore afterwards. I thought that 
this RFE just intents to keep a valid pointer in Compile::_for_igvn.

Nevertheless, I also took a closer look at that code and it seems that 
for_igvn() is not even required for PhaseRenumberLive? It's cleared just 
before the phase on L2086 and then PhaseRenumberLive does not seem to 
add anything to it and neither does PhaseRemoveUseless. So we are 
basically restoring an empty list afterwards. However, this could be 
cleaned up in an additional RFE.

Best regards,
Christian

On 03.09.20 10:56, Liu, Xin wrote:
> hi, Nhat and Christian,
> 
> I reviewed this patch. I feel resuming the old worklist looks suspicious.
> PhaseRenumberLive renumbers nodes associates with gvn. old _worklist might have wrong idx  and types.
> 
> Instead of resuming old _worklist,  you can copy out nodes from new_worklist to your current _worklist.
> 
> diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp
> --- a/src/hotspot/share/opto/compile.cpp
> +++ b/src/hotspot/share/opto/compile.cpp
> @@ -2089,7 +2089,10 @@
>         ResourceMark rm;
>         PhaseRenumberLive prl = PhaseRenumberLive(initial_gvn(), for_igvn(), &new_worklist);
>       }
> -    set_for_igvn(&new_worklist);
> +    for_igvn()->clear();
> +    while (new_worklist.size() > 0) {
> +     for_igvn()->push(new_worklist.rpop());
> +    }
>       igvn = PhaseIterGVN(initial_gvn());
>       igvn.optimize();
>     }
> 
> 
> thanks,
> --lx
> 
> ________________________________________
> From: hotspot-compiler-dev <hotspot-compiler-dev-retn at openjdk.java.net> on behalf of Christian Hagedorn <christian.hagedorn at oracle.com>
> Sent: Thursday, August 27, 2020 7:54 AM
> To: Nhat Nguyen; hotspot-compiler-dev at openjdk.java.net
> Subject: RE: [EXTERNAL] RFR(S) 8251271- C2: Compile::_for_igvn list is corrupted after RenumberLiveNodes
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> Hi Nhat
> 
> Looks good to me!
> 
> Just make sure you that next time you assign the bug to you or a sponsor
> and/or leave a comment that you intend to work on it to avoid the
> possibility of some duplicated work (was no problem in this case) ;-)
> 
> Best regards,
> Christian
> 
> On 26.08.20 20:55, Nhat Nguyen wrote:
>> Hi hotspot-compiler-dev,
>>
>> Please review the following patch to address https://bugs.openjdk.java.net/browse/JDK-8251271
>> The bug is currently assigned to Christian Hagedorn, but he was supportive of me submitting the patch instead.
>> I have run hotspot/tier1 and jdk/tier1 tests to make sure that the change is working as intended.
>>
>> webrev: http://cr.openjdk.java.net/~burban/nhat/JDK-8251271/webrev.00/
>>
>> Thank you,
>> Nhat
>>