[9] RFR(S): 8168996: C2 crash at postaloc.cpp:140 : assert(false) failed: unexpected yanked node

Tue Nov 29 07:06:20 UTC 2016

Thanks for the review, Vladimir!

Best regards,
Tobias

On 28.11.2016 21:26, Vladimir Kozlov wrote:
> Okay. It make sense now. Thank you, Tobias, for looking on it.
> 
> Vladimir
> 
> On 11/25/16 4:42 AM, Tobias Hartmann wrote:
>> Hi Vladimir,
>>
>> On 23.11.2016 19:20, Vladimir Kozlov wrote:
>>> Good fix.
>>
>> Thanks for the review!
>>
>>> Tobias, do you know why Load node was split though Phi? Usually it happens when, at least, on one of branches Load will fold or other optimization happens. I don't see that in this case.
>>
>> Here's the detailed explanation:
>>
>> We decide to split an IfNode (581) because it gets its input from a Phi (1967) in the same Region (582):
>>
>>  1967    Phi    ===  582  283  1969  1970  [[ 581 ]]  #bool !orig=[867]
>>  582    Region    ===  582  868  869  870  [[ 582  589  352  586  354  587  588  581  1967  1971 ]]  !jvms: String::isLatin1 @ bci:3 String::startsWith @ bci:50 String::startsWith @ bci:3 MethodHandles$Lookup::checkMethodName @ bci:3 MethodHandles$Lookup::resolveOrFail @ bci:19 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>  581    If    ===  582  1967  [[ 351  1383 ]] P=0.000000, C=7816.000000 !jvms: String::isLatin1 @ bci:0 String::startsWith @ bci:50 String::startsWith @ bci:3 MethodHandles$Lookup::checkMethodName @ bci:3 MethodHandles$Lookup::resolveOrFail @ bci:19 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>
>> The Stack looks like this:
>> #2 0x00007f7fc7064fcc in PhaseIterGVN::replace_node(n, phi) phaseX.hpp:512
>> #3 0x00007f7fc7c98038 in PhaseIdealLoop::split_up(Node* n, ...) split_if.cpp:219
>> #4 0x00007f7fc7c98bcb in PhaseIdealLoop::do_split_if(Node* iff) split_if.cpp:219
>>
>> To empty the block, we first look at all other Phi users of the Region (582) and recursively split their users through the region as well. Phi (586) is in the same Region (582) and has a LoadN user (1169) which is therefore split through the region:
>>
>>  586    Phi    ===  582  874  875  876  [[ 353  1169  893  362  359  356  888 ]]  #memory  Memory: @BotPTR *+bot, idx=Bot; !jvms: String::isLatin1 @ bci:3 String::startsWith @ bci:50 String::startsWith @ bci:3 MethodHandles$Lookup::checkMethodName @ bci:3 MethodHandles$Lookup::resolveOrFail @ bci:19 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>> $10 = void
>>  1169    LoadN    === _  586  1016  [[]]  @java/lang/Class:exact+124 * [narrow], name=security, idx=23; Volatile! #narrowoop: java/lang/SecurityManager * !jvms: MemberName$Factory::resolve @ bci:84 MemberName$Factory::resolveOrFail @ bci:4 MethodHandles$Lookup::resolveOrFail @ bci:44 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>  894    DecodeN    === _  1169  [[ 599 ]]  #java/lang/SecurityManager *  Oop:java/lang/SecurityManager * !jvms: MemberName::clone @ bci:1 MemberName$Factory::resolve @ bci:1 MemberName$Factory::resolveOrFail @ bci:4 MethodHandles$Lookup::resolveOrFail @ bci:44 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>
>> This explains how the DecodeN (894) ends up with a Phi input.
>>
>> Thanks,
>> Tobias
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 11/23/16 5:20 AM, Tobias Hartmann wrote:
>>>> Hi,
>>>>
>>>> please review the following patch:
>>>> https://bugs.openjdk.java.net/browse/JDK-8168996
>>>> http://cr.openjdk.java.net/~thartmann/8168996/webrev.00/
>>>>
>>>> We crash in the register allocator because of an "unexpected yanked node". The problem is that there is a MemBarAcquire node from a volatile object field load that keeps otherwise dead LoadN nodes alive (see graph at [1]). This is very similar to JDK-8048879 [2] with the difference that the MemBarAcquire -> DecodeN is not directly connected to the LoadN but there is a Phi in between. This Phi was created by the split if optimization by splitting the original LoadN node through a memory Phi and thus creating multiple LoadN nodes connected to a new data Phi. The Phi becomes dead later and is only kept alive by the MemBarAcquire node.
>>>>
>>>> The code in MemBarNode::Ideal() should take care of such a scenario by rewiring and finally removing the MemBar if the precedence input has only one user. The problem is that this code is never executed because the MemBar node is not added to the IGVN worklist after the users of the DecodeN node become dead. I fixed this by adding a special case to Node::has_special_unique_user() similar to what I did for Load nodes in the fix for JDK-8048879.
>>>>
>>>> I was not able to write a regression test for this but can reproduce the problem reliably with replay compilation which generates a very complex graph.
>>>>
>>>> Tested with replay compilation, JPRT and RBT (running).
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>> [1] C2 graph before RA:
>>>>
>>>>  1974    LoadN    === _  876  1016  [[ 1993 ]]  @java/lang/Class:exact+124 * [narrow], name=security, idx=23; Volatile! #narrowoop: java/lang/SecurityManager * !orig=[1169] !jvms: MemberName::testFlags @ bci:7 MemberName::testAllFlags @ bci:3 MemberName::isConstructor @ bci:3 MethodHandles$Lookup::checkMethod @ bci:14 MethodHandles$Lookup::getDirectMethodCommon @ bci:4 MethodHandles$Lookup::getDirectMethod @ bci:14 MethodHandles$Lookup::findStatic @ bci:23 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>>>  1973    LoadN    === _  875  1016  [[ 1993 ]]  @java/lang/Class:exact+124 * [narrow], name=security, idx=23; Volatile! #narrowoop: java/lang/SecurityManager * !orig=[1169] !jvms: MemberName::testFlags @ bci:7 MemberName::testAllFlags @ bci:3 MemberName::isConstructor @ bci:3 MethodHandles$Lookup::checkMethod @ bci:14 MethodHandles$Lookup::getDirectMethodCommon @ bci:4 MethodHandles$Lookup::getDirectMethod @ bci:14 MethodHandles$Lookup::findStatic @ bci:23 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>>>  1972    LoadN    === _  874  1016  [[ 1993 ]]  @java/lang/Class:exact+124 * [narrow], name=security, idx=23; Volatile! #narrowoop: java/lang/SecurityManager * !orig=[1169] !jvms: MemberName::testFlags @ bci:7 MemberName::testAllFlags @ bci:3 MemberName::isConstructor @ bci:3 MethodHandles$Lookup::checkMethod @ bci:14 MethodHandles$Lookup::getDirectMethodCommon @ bci:4 MethodHandles$Lookup::getDirectMethod @
>>>>  1993    Phi    ===  1983  1972  1973  1974  [[ 894 ]]  #narrowoop: java/lang/SecurityManager * !orig=[1971],[1169] !jvms: MethodHandles$Lookup::checkMethod @ bci:17 MethodHandles$Lookup::getDirectMethodCommon @ bci:4 MethodHandles$Lookup::getDirectMethod @ bci:14 MethodHandles$Lookup::findStatic @ bci:23 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>>>  894    DecodeN    === _  1993  [[ 599 ]]  #java/lang/SecurityManager *  Oop:java/lang/SecurityManager * !jvms: MemberName::clone @ bci:1 MemberName$Factory::resolve @ bci:1 MemberName$Factory::resolveOrFail @ bci:4 MethodHandles$Lookup::resolveOrFail @ bci:44 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>>>  599    MemBarAcquire    ===  892  1  893  1  1  894  [[ 890  365 ]]  !jvms: String::startsWith @ bci:50 String::startsWith @ bci:3 MethodHandles$Lookup::checkMethodName @ bci:3 MethodHandles$Lookup::resolveOrFail @ bci:19 MethodHandles$Lookup::findStatic @ bci:6 InnerClassLambdaMetafactory::buildCallSite @ bci:137
>>>>
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8048879
>>>>