RFR: 8356813: Improve Mod(I|L)Node::Value

[Quan Anh Mai]

On Sun, 25 May 2025 09:46:27 GMT, Hannes Greule <hgreule at openjdk.org> wrote:

>>> The original code probably returned TypeInt::POS for the same reason you bring up below:
>> 
>> I doubt that, as it doesn't account for the sign of the dividend at all here. We also can't keep the existing behavior (see the section about monotonicity in the PR description).
>> From my understanding, the node should also be kept alive no matter the value due to its control input. 
>> 
>> I'll test with returning TOP.
>
> Using TOP seems to work, but I'm still a bit hesitant to use it:
> - I'm not sure if it is intended to be used this way
> - `TypeNode`s get a special handling when they become TOP. That is not a problem as long as the Mod nodes aren't `TypeNode`s, but it looks rather dangerous.
> 
> I think I'd still prefer to use ZERO unless someone else can guarantee that using TOP will be fine. I could maybe give a more in-depth explanation why ZERO behaves monotonic here as a comment.

Please don't use zero. Using zero is NOT fine.

> I'm not sure if it is intended to be used this way

Yes it is, `TOP` denotes an empty set, and the result of `ModI/L` with the divisor being a constant zero is an empty set.

> `TypeNode`s get a special handling when they become `TOP`. That is not a problem as long as the `Mod` nodes aren't `TypeNode`s, but it looks rather dangerous.

`TypeNode` kills all paths below it if it becomes `TOP`, I think it is the expected behaviour when the divisor is a constant zero in that path.

> I could maybe give a more in-depth explanation why ZERO behaves monotonic here as a comment.

You are just lucky using zero in this particular operation and it seems to be monotonic, but if you apply the same logic to similar operations, like `Div`, `UMod`, you will see that zero can make them non-monotonic. As a result, I can confidently say that using zero here is incorrect.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25254#discussion_r2106171097