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 at oracle.com
Thu Aug 29 07:17:24 UTC 2019
thanks for the explanation. Very unfortunate that anti-dependency analysis can not do better but
your fix seems reasonable to me.
On 28.08.19 16:32, Roland Westrelin wrote:
> Thanks for reviewing this, Tobias.
>> I've noticed that field2 in the test is unused.
> Good catch. I'll remove it.
>>> 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 = j;
>>> by anti dependence analysis.
>>> The store array 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, 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?
> 58 return field + res + field * 2;
> the load of static field "field".
>>> 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
>> 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?
> Dependency analysis is so conservative right now that fixing it seems a
> long way from happening. So it could possibly be improved but I don't
> see it being "fixed" for good.
> I'm not sure relaxing the verification code is better. Loop strip mining
> works under the assumption that some simple invariants remain
> true. That's what the verification code is here to check. Relaxing the
> verification code and thus the invariants would make reasoning about
> loop strip mining harder and doesn't seem like the right way to fix
> this. The invariant in this case is that nothing is in the outer loop
> other that the control flow for the outer loop, the safepoint and data
> nodes that are referred to from the safepoint.
>>> Unrelated to this fix, I wonder if the code at:
>>> 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.
> I'm hoping Vladimir will comment. If not, I'll file a RFE as you
More information about the hotspot-compiler-dev