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

[Hannes Greule]

On Mon, 2 Jun 2025 10:58:45 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Hannes Greule has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add randomized test
>
> src/hotspot/share/opto/divnode.cpp line 1237:
> 
>> 1235:     // We don't need to check for min_jint % '-1' as its result is defined when using jlong.
>> 1236:     if (i1->get_con_as_long(bt) == min_jlong && i2->get_con_as_long(bt) == -1) {
>> 1237:       return TypeInteger::zero(bt);
> 
> Is this correct? For `bt = T_INT` is this really equivalent?
> 
> `i1->get_con() == min_jint`
> We might get `min_jint` back here.
> 
> `i1->get_con_as_long(bt) == min_jlong`
> Would we not return `min_jint` here, and then the condition is false?
> 
> Do we have an IR test for this?

This special case is only needed because `min_jlong % -1L` in C++ is UB (afaik) and the idiv instruction triggers a SIGFPE in such case. But `min_jint % -1L` *using long arithmetic* correctly produces 0.

I think it would make sense to expand tests for constant folding, but I'll have to check if that actually gets called, see **Other** in the PR description (copied):
> If the divisor is a constant, we will directly replace the Mod(I|L)Node with more but less expensive nodes in ::Ideal(). Type analysis for these nodes combined is less precise, means we miss potential cases were this would help e.g., removing range checks. Would it make sense to delay the replacement?

So there's a chance this code was never called before...

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

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