RFR(S): 8229483: Sinking load out of loop may trigger: assert(found_sfpt) failed: no node in loop that's not input to safepoint

[Tobias Hartmann]

Hi Roland,

On 27.08.19 11:23, Roland Westrelin wrote:
> http://cr.openjdk.java.net/~roland/8229483/webrev.00/

I've noticed that field2 in the test is unused.

> The field load in the test case has an early control right below:
> 
> barrier = 1;
> 
> (it's a volatile store)
> 
> and a late control that's conservatively set to be right above:
> 
> array[0] = j;
> 
> by anti dependence analysis.
> 
> The store array[0] is sunk out of the inner loop in the outer strip
> mined loop right above the strip mined loop's safepoint. The load
> initially scheduled in the outer loop is a candidate for being cloned
> and sunk out of loop at the load usages. Because of the anti-dependence
> with the array[0], it is sunk into the outer strip mined loop eventhough
> it is not referenced by the safepoint. That breaks loop strip mining
> verification because the expectation is that any data node in the outer
> strip mined loop is there because it's referenced by the safepoint.

I don't see a field load in the loop, so which load are you referring to?

> The fix simply recognizes this special case when the load is being sunk
> out of loop and makes sure it is not moved in the outer strip mined
> loop.

Wouldn't it be better to relax the verification code if possible? If we ever fix dependency analysis
to be less restrictive (see JDK-8229449), we should move the load out of the loop, right?

> Unrelated to this fix, I wonder if the code at:
> 
> http://hg.openjdk.java.net/jdk/jdk/file/cb836bd08d58/src/hotspot/share/opto/loopopts.cpp#l1346
> 
> really does what the comment says it does for loads (and really does
> anything useful actually).

I'm not really familiar with that code but maybe file a RFE to investigate this later.

Best regards,
Tobias