RFR: 8350896: Integer/Long.compress gets wrong type from CompressBitsNode::Value [v2]

[Jatin Bhateja]

On Wed, 12 Mar 2025 08:43:23 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments resolutions
>
> @jatin-bhateja Thanks for looking into this! I left a first set of comments :)
> 
> Primarily, it is about these issues:
> - We need good comments, preferably even proofs. Because we got things wrong the last time, and there were no comments/proofs. It's difficult to get this sort of arithmetic transformation right, and it is hard to review. Proofs help to think through all the steps carefully.
> - Test coverage: I would like to see some more randomized cases of input ranges.

Hi @eme64 , I have addressed your comments, let me know if you need further clarifications.

> src/hotspot/share/opto/intrinsicnode.cpp line 278:
> 
>> 276:       } else {
>> 277:       // Case 3) Mask value range only includes +ve values, this can again be
>> 278:       // used to ascertain known Zero bits of resultant value.
> 
> I would put this case as the first, swapping it with Case 1).
> And I would say something more explicit like this:
> `Case 3) The mask value range is non-negative. Hence, the mask has at least one zero bit.`

Case ordering is in accordance with the mask value range.
case 1) mask value spans across -ve and -ve value ranges.
case 2) mask value strictly lie within -ve value range. 
case 3) mask value strictly lie within +ve value range.

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

PR Comment: https://git.openjdk.org/jdk/pull/23947#issuecomment-2771593965
PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2024244475