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

[Nils Eliasson]

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
>>