RFR: 8371146: C2 SuperWord: VTransform::add_speculative_check uses pre_init that is pinned after Auto_Vectorization_Check, leading to bad graph
Christian Hagedorn
chagedorn at openjdk.org
Wed Nov 26 14:37:45 UTC 2025
On Wed, 26 Nov 2025 11:40:21 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>>> I suppose I don't. Do you think I have to? What about all the inputs of n? Those may also have a ctrl that is too low now. So if you want consistency we would have to fix up everything up the chain... :/
>>
>> That's the big question. It's probably hard to say if it's necessary or not but also hard to say if someone is actually relying on it. I would say: If ctrl is not optimal but legal, it does not matter that much. If it's illegal, we should probably fix it to avoid such wrong uses further down the line.
>>
>>> But I think this issue is not limited to the new is_available_for_speculative_check, but already existed for the much older is_pre_loop_invariant, which also uses compute_early_ctrl. So the problem is a little bigger, if it is really a problem at all - it may well be a problem.
>>
>> Right, that's my feeling as well that more places are off. As long as they are just not optimal but legal, we probably do not need to worry about them.
>>
>>> * We enhance our loop-opts verification, and verify ctrl after SuperWord. Then we will discover that a lot of the ctrl is set inaccurately, and fix it.
>>
>> That's a good idea for the future. Not sure how easy it will be to keep it as accurate as possible.
>>
>>> That way we can also have confidence that the fix is correct. If we did the fix now, without verification, this would just be a blind stab in the dark kind of fix ;)
>>
>> I'm concerned here that someone relies on the illegal ctrl later even though chances are probably low. But what do you think about just seting ctrl to the just computed early ctrl (if we are sure we are going to create the speculative check)? That would be an easy fix (might not be optimal but at least legal). But I agree that we should probably tackle this ctrl verification in general at some point.
>
> Right. The ctrl can either be legal (between the early and lates point), or outside it (illegal).
>
> I fear that we do have some illegal ctrl now:
> Imagine we use some node `n1(n2(n3(...)))`, where all of `n1-3` have ctrl between pre-loop and predicate, but their early is before the predicate. If we now use `n1` at the predicate and update its ctrl to early, then we would actually have to update the ctrl of `n2` and `n3` between early and predicate too, right? The whole expression would have to be moved, otherwise we have inputs that have a later ctrl than its outputs, and that is not right.
>
> The same is true for `is_pre_loop_invariant` with ctrl between inside pre-loop and before pre-loop.
>
> I'm very happy to file even a bug for this. But I'd prefer not to have to do it in this same issue here. Is that ok?
Yes, you are right, the problem/implications are more wide-spread and it's not much better when fixing one place while leaving the other ones in a illegal state. I think you have a better overview over Autovectorization to make an estimate if there are immediate problems with continuing with illegal ctrl or not (sounds like there are none - at least in theory). So, I agree with you to investigate that separately. I think it could also just be part of tackling verification for ctrl in general. You can move ahead with this PR then, thanks for discussing this aspect! :-)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28449#discussion_r2565252769
More information about the hotspot-compiler-dev
mailing list