RFR(S): 8228839: Non-CFG nodes have control edges to calls, instead of the call's control projection

[Vladimir Kozlov]

Thank you for explaining, Nils

I am okay with this temporary workaround then. Please, file bug to fix main issue.

Thanks,
Vladimir

On 8/21/19 12:56 PM, Nils Eliasson wrote:
> Hi Vladimir,
> 
> I want the workaround in promtly because the issue causes noise in the bigapps testing which is 
> really annoying, and might be hiding other problems.
> 
> The core issue - the PhaseIdealLoop::get_ctrl problem, might turn up quite large. Many types of 
> MultiNodes, like calls and Membars are at times set incorrectly (often in loopopts). SafepointNodes 
> are a direct subclass of MultiNodes but doesn't seem to use a control projection anywhere. (But 
> calls that are a subclass of SafepointNodes expect it - very confusing).
> 
> It is also quite common that the node retrieved from a PhaseIdealLoop::get_ctrl(..) is set as 
> control in another node, which propagates the problem.
> 
> I have tried to sanitize the ctrl-mapping by intercepting all MultiNodes (except SafePointNodes and 
> StartNodes) in PhaseIdealLoop::set_ctrl(). That seems to solve all downstream issues (without 
> causing new problems). But for a permanent fix I need to go through all the call sites of set_ctrl() 
> and find out why they don't use the projection.
> 
> I also need to investigate why SafepointNodes don't use control projections, and what it would take 
> to fix it.
> 
> Regards,
> 
> Nils
> 
> 
> On 2019-08-21 17:36, Vladimir Kozlov wrote:
>> Hi Nils,
>>
>> Did you find in which cases the control edge is set to Call node directly?
>>
>> Since the fix for JDK 14 we have time to fix it correctly instead of workaround.
>>
>> Thanks,
>> Vladimir
>>
>> On 8/21/19 3:30 AM, Nils Eliasson wrote:
>>> Hi,
>>>
>>> This is a workaround for the problem that sometimes non-CFG nodes have their ctrl 
>>> (PhaseIdealLoop::get_ctrl(n)) set directly to calls, instead of the call's control projection. 
>>> I'm working on a patch for that, but it might end up a bit intrusive. This is a workaround that 
>>> simply checks and adjusts for that case.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8228839
>>>
>>> Webrev: http://cr.openjdk.java.net/~neliasso/8228839/webrev.01/
>>>
>>> Please review,
>>>
>>> // Nils
>>>