[15] RFR(M): 8236493: C2: Inconsistent known instance type information after EA

[Tobias Hartmann]

Hi,

I found some non-trivial issues with this. I will follow up with a new webrev after fixing.

Thanks,
Tobias

On 14.01.20 15:06, Tobias Hartmann wrote:
> Hi,
> 
> please review the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8236493
> http://cr.openjdk.java.net/~thartmann/8236493/webrev.00/
> 
> While working on 8233164, I've noticed that escape analysis does not set known instance type
> information for LoadNodes and their users if the loaded object does not escape but that information
> is only (incrementally) propagated during IGVN. That's a problem because depending on the sequence
> in which nodes on the worklist are processed, optimizations might still see the not-yet-updated (and
> therefore incorrect) type. For example, another LoadNode optimization might rely on it's memory
> input to be a general memory slice and re-wire it's inputs accordingly (ignoring known instance type
> memory), when it is really reading from a known instance.
> 
> For a detailed description of an instance of such a problem, see my RFR for 8233164 [1]. There, the
> type of a CastPPNode is not updated (yet) and a Load user is incorrectly re-wired.
> 
> To catch this case before things go horribly wrong, I've added an assert to
> Node::raise_bottom_type(). Reverting the fix for 8233164 now immediately triggers the assert because
> the known instance id of a DecodeNNode is only updated during IGVN [2]. All new tests in
> TestInstanceIdPropagation.java trigger the assert as well.
> 
> The fix is to always eagerly propagate that information during EA. If an object loaded from the
> field of a non-escaping object is also non-escaping, we need to propagate that information.
> 
> Thanks,
> Tobias
> 
> [1] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-December/036616.html
> 
> [2] Failure with TestArrayCopyMemoryChain.java when reverting fix for 8233164:
> 
>  300	Proj	===  288  [[ 301  305  312 ]] #5 !jvms: TestArrayCopyMemoryChain::test2 @ bci:25
>  305	CheckCastPP	===  302  300  [[ 385  316 ]]  #byte[int:1]:NotNull:exact *,iid=288 !jvms:
> TestArrayCopyMemoryChain::test2 @ bci:25
>  385	EncodeP	=== _  305  [[ 387  403 ]]  #narrowoop: byte[int:1]:NotNull:exact *,iid=288 !orig=[402]
> !jvms: TestArrayCopyMemoryChain$Wrapper::<init> @ bci:11 TestArrayCopyMemoryChain::test2 @ bci:41
>  403 >	DecodeN	=== _  385  [[ 494  420 ]]  #byte[int:1]:NotNull:exact *,iid=288 !jvms:
> TestArrayCopyMemoryChain::test2 @ bci:48
>  494	CastPP	===  486  403  [[ 526  514 ]]  #byte[int:>=0]:NotNull:exact * !jvms:
> TestArrayCopyMemoryChain::test2 @ bci:71
>  420	CallStaticJava	===  404  327  341  8  9 ( 419  1  1  1  64  1  1  1  403  257  43  64  11  230
> ) [[ 421 ]] # Static uncommon_trap(reason='intrinsic_or_type_checked_inlining'
> action='make_not_entrant' debug_id='0')  void ( int ) C=0.000100 TestArrayCopyMemoryChain::test2 @
> bci:61 !jvms: TestArrayCopyMemoryChain::test2 @ bci:61
> # To suppress the following error report, specify this argument
> # after -XX: or in .hotspotrc:  SuppressErrorAt=/node.cpp:1088
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (/oracle/jdk_jdk/open/src/hotspot/share/opto/node.cpp:1088), pid=6482, tid=6496
> #  assert(false) failed: Known instance type should have been set during EA
>