RFR: 8366815: C2: Delay Mod/Div by constant transformation

Emanuel Peter epeter at openjdk.org
Thu Oct 23 11:42:07 UTC 2025


On Thu, 23 Oct 2025 07:21:03 GMT, Hannes Greule <hgreule at openjdk.org> wrote:

>> src/hotspot/share/opto/divnode.cpp line 545:
>> 
>>> 543: 
>>> 544:   // Keep this node as-is for now; we want Value() and
>>> 545:   // other optimizations checking for this node type to work
>> 
>> Do we only need `Value` done first on the `Div` node, or also on uses of it?
>> It might be worth explaining it in a bit more detail here.
>> 
>> If it was just about calling `Value` on the `Div` first, we could probably check what `Value` returns here. But I fear that is not enough, right? Because it is the `Value` here that returns some range, and then some use sees that this range has specific characteristics, and can constant fold a comparison, for example. Did I get this right?
>
> So, the *main* reason why I'm including Div here is mainly because of #26143; before that the DivI/LNode::Value() is actually less precise than Value on the nodes created by `transform_int_divide`. With #26143, some results are more precise even for constant divisors. In such case, uses can benefit from seeing the (then) more precise range. (@ichttt found a case where the replacement fails to constant-fold, but that's just due to missing constant folding in MulHiLNode)
> 
> A secondary reason is other optimizations checking for Div inputs, though I didn't find any existing check that would actually benefit. There *might* be optimization opportunities that want to detect division, but that's just
> 
> Generally from what I've found the benefit is bigger for Mod nodes, because there calling Value on the replacements is significantly worse. And there we also encounter typical usages in combination with range checks.
> 
> Do you want me to expand both Div and Mod comments to cover more concrete benefits, depending on the operation?

Yes, I think it would make sense to have an explanation at both ends. Your nice example with the "rounding error" of 0..1 for `Div` makes a lot of sense. Seeing a similar example for `Mod` (where it could be worse, you say) would also be nice 😊 

You can copy the comments for the I/L cases, or only put it at one of them, and link from the other. There is an issue with a PR that refactors mod/div so that we only have one implementation each, and they can clean this up.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27886#discussion_r2454825775


More information about the hotspot-compiler-dev mailing list