anti deps, loadKlass and calls

[Vladimir Kozlov]

I agree that LoadKlass and LoadRange should return false for
needs_anti_dependence_check since they are accessing memory
which can't be modified.

Vladimir

Tom Rodriguez wrote:
> I'm looking at a compilation bailout that results from a failure to 
> schedule a chain of loadKlass with an extra precedence edge.  This is 
> bug 6857159.  This is the cycle that causes the problem:
> 
> 
>  24     tlsLoadP        ===  16  [[ 25  23 ]]
>  9      loadKlass       === _  18  22  [[ 8  13 ]]  *  Klass: *
>  22     loadKlass       === _  18  23  [[ 9 ]] klass java/lang/Thread: 
> 0x080de410 *  Klass:klass java/lang/Thread: 0x080de410 * !jvms: 
> ct$ct0::run @ bci:7
> 13     CallDynamicJavaDirect   ===  15  17  18  19  0  20  0  0  | 9  [[ 
> 14  12  21  26 ]] Dynamic  ct$ct0::message # void ( ct$ct0:NotNull * ) 
> ct$ct0::run @ bci:1 !jvms: ct$ct0::run @ bci:1
>  26     MachProj        ===  13  [[ 23  4  29  31 ]] #2/unmatched  
> Memory: @BotPTR *+bot, idx=Bot; !jvms: ct$ct0::run @ bci:1
>  23     loadP   === _  26  24  [[ 22  4 ]] java/lang/Thread:NotNull *  
> Oop:java/lang/Thread:NotNull * !jvms: ct$ct0::run @ bci:4
> 
> The code corresponds to a checkcast of Thread.currentThread() to a type 
> in the middle of hierarchy of classes.  The test case is in the bug 
> report.  The first loadKlass is loading the klass from the header and 
> it's in its own alias class and is marked as not rewritable.  This 
> causes the anti dep code to just skip it.  The loadKlass with the 
> precedence edge is a load from the superclass display list used by 
> checkcast.  Since it's not in it's own alias class we can't set the 
> is_rewritable flag to false so we will actually look for anti deps.  
> Because it uses the same memory input as the call we end up creating an 
> anti dep edge between them even though the loadP is actually dependent 
> on the output of the call.  So maybe LoadKlass should return false for 
> needs_anti_dependence_check?  The memory edges on them should always 
> guarantee correct placement.  The other thing that seems wrong is that 
> the loadP getting the current thread from tls should use 
> immutable_memory instead of the current memory since it's an unchanging 
> value.  Either of these changes actually fix the problem but I think 
> skipping the anti-dep check is the more correct one.
> 
> This failure was caused by 6385730 where we changed LoadKlass and 
> LoadRange to use the starting memory to indicate that they could be 
> commoned across the whole compile.  Prior to that they would have used 
> the same memory as the loadP so it would have worked correctly.
> 
> tom