RFR(S): 8219448: split-if update_uses accesses stale idom data

[Vladimir Kozlov]

Got it. In such case you need to replace lazy_replace() at line 575 with remove_dead_node() and you don't need assert 
outcnt() == 0 then.

Thanks,
Vladimir

On 2/28/19 11:48 AM, Nils Eliasson wrote:
> Hi,
> 
> I just moved it up from line 570.
> 
> http://hg.openjdk.java.net/jdk/jdk/file/196ab0abc685/src/hotspot/share/opto/split_if.cpp#l570
> 
> And on 575 we will call the replace on it, to finally kill it.
> 
> http://hg.openjdk.java.net/jdk/jdk/file/196ab0abc685/src/hotspot/share/opto/split_if.cpp#l575
> 
> So the node is dead, we just must handle the phi-uses first, and while doing that, a correct idom is required.
> 
> The code that triggers this bug has a diamond-shaped control flow below the split-if-region. The region-nodes that need 
> their idom corrected is far down and isn't touched during the split. But there is a call down there. It has one of its 
> data edges defined by a phi hanging on the split-region. So when we try to call spinup on it, it will traverse a broken 
> idom chain.
> 
> The conclusion of my investigation is that all regions that have the split-region as its idom, must be updated (even if 
> they are below). For that we have the lazy_update mechanism, and to make it trigger, I must mark the region as killed 
> slightly earlier.
> 
> Regards,
> 
> Nils
> 
> 
> 
> 
> 
> On 2019-02-28 20:00, Vladimir Kozlov wrote:
>> Hi Nils,
>>
>> You are updating map so that next code in idom_no_update() works for you:
>> http://hg.openjdk.java.net/jdk/jdk/file/196ab0abc685/src/hotspot/share/opto/loopnode.hpp#l928
>> Which seems a hack to me. I think we should fix spinup() method to skip old region when looking for idom (I assume 
>> that is where you have the problem).
>>
>> Thanks,
>> Vladimir
>>
>> On 2/28/19 5:18 AM, Nils Eliasson wrote:
>>> Hi,
>>>
>>> This patch fixes some of the idom updates in split-if. The updates are there, but at the end, which is to late. They 
>>> need to be correct when handling the uses of the region being split. When the stale idom is seen we end up asserting 
>>> or crashing.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8219448
>>>
>>> http://cr.openjdk.java.net/~neliasso/8219448/webrev.01/
>>>
>>> Please review,
>>>
>>> Nils Eliasson
>>>