RFR: 8282365: Consolidate and improve division by constant idealizations [v45]

[Raffaello Giulietti]

On Sat, 20 Jan 2024 11:50:37 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> src/hotspot/share/opto/divconstants.cpp line 161:
>> 
>>> 159:       c_ovf = c > min_signed;
>>> 160:       c += c - 1;
>>> 161:       rc += rc - d;     // rc = 2 * rc - d
>> 
>> Well, `rc - d` usually underflows. This is benign on the supported CPUs, but I think the code proposed originally is clearer.
>> It also makes clear that the subtraction in `-=` is safe, since `rc > d - rc`. With an addition `+=` one has to reason in reverse, so to say.
>> Further, Common Subexpression Elimination can easily detect that the expression `d - rc` is identical to the one in the `if` condition. Not sure if CSE can detect that `rc - d` is `-(d - rc)` and emit a subtraction.
>> Suggestion:
>> 
>>       rc -= d - rc;     // rc = 2 * rc - d
>
> I think it makes it more uniform with the other case and since we are writing it as `rc = 2 * rc - d`, expanding it to `rc + rc - d` seems more natural. Note that the type is unsigned so overflow behaviour is not undefined.

The purpose of the `//` comment is to make it even more clear that `rc -= d - rc` computes `rc = 2 * rc - d`. But in fact, the expansion `rc = rc - (d - rc)` is already clear enough, IMO.

Sure, the behavior with the underflow is specified: that's what I mean by "benign" in my previous comment. Yet, one must reason longer to be convinced that the underflow is harmless.

Of course, this is a subjective matter about clarity of the code. Everybody has their own opinion about ;-)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/9947#discussion_r1460405625