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

[Hannes Greule]

On Mon, 25 Aug 2025 13:20:32 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>>> @eme64 I merged master and hopefully addressed your latest comments. Now that we have #17508 integrated, I could also directly update the unsigned variant, but I'm also fine with doing that separately. WDYT?
>>> 
>>> I also checked the constant folding part again (or generally whenever the RHS is a constant), these code paths are indeed not used by PhaseGVN directly (but by PhaseCCP and PhaseIdealLoop). That makes it a bit difficult to test that part properly.
>> 
>> Let's keep the patch as it is. With #17508 we will have to also probably refactor and add more tests, if we want to do any unsigned and known-bit optimizations.
>> 
>> ----------------
>> 
>> @SirYwell Thanks for the updates, I had a few more comments, but we are getting there :)
>
>> @eme64 I addressed your latest comments now, please re-review :)
>> 
>> Regarding my previous observation
>> 
>> > * 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?
>> 
>> should I open a new RFE for that? Or generally, what's your opinion on this?
> 
> Can you show some examples? Filing an RFE would surely not be wrong.

@eme64 gentle ping in case you missed my latest changes :)
Please let me know if there is more to do.

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

PR Comment: https://git.openjdk.org/jdk/pull/25254#issuecomment-3243887966