RFR: 8349139: C2: Div looses dependency on condition that guarantees divisor not null in counted loop [v2]

[Emanuel Peter]

On Fri, 14 Feb 2025 18:24:25 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - Merge branch 'master' into JDK-8349139
>>  - fix & test
>
> Hmmm, may be you are right. I think adding a comment at `PhiNode` saying that people must not rely on it being pinned at the `Region` for dependencies would be a wise move, I can't think of any reason for that besides value narrowing right now but being pinned is a property of `Phi` regardless and we should tell people not to rely on this behaviour.
> 
> For this bug, I think a more general fix is to try to compare the type of the `Phi` with that of the input it is going to be replaced with. If the former is not wider than the latter then we add a `CastNode`, since the cast is only about value range, not strict dependency, we can use `CarryDependency` instead of `UnconditionalDependency`. Am I right?

Ah, I only just now read the comments from @merykitty and you. Oops. Hmm.

Yes it seems that the `CountedLoop` trip `phi` is special. That's maybe not great to have such implicit assumptions laying around. But not sure what would have been the better alternative.

@rwestrel 
> It reproduces this issue and is actually a better test case because it doesn't even need StressGCM:

Are you saying this is another test? I'd really be happy if we had more tests for this case, because the current version seems fragile, since it is an almost perfect copy of a previous test that became ineffective this makes me even more nervous 😅

-------------

PR Comment: https://git.openjdk.org/jdk/pull/23617#issuecomment-2681081313