RFR: 8347645: C2: XOR bounded value handling blocks constant folding [v46]
Johannes Graham
duke at openjdk.org
Tue Apr 1 02:52:17 UTC 2025
On Tue, 1 Apr 2025 02:14:35 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:
>> Thanks.
>>
>>> The naming of that method evolved during the course of the review of this PR. I believe the thinking was that the check was not necessarily an overall upper bound, and a simpler name would imply it was more general.
>>
>> There's usually a lot of invariants a function assumes and it's simply impractical to encode everything in the name. Speaking of this particular case (`calc_xor_upper_bound_of_non_neg`):
>> * `calc_` is redundant and IMO only adds noise;
>> * `_non_neg` part is confusing; I'd stress instead that it works on **ranges**.
>>
>> So, `xor_upper_bound_for_ranges` then? (And, please, explain in the comment what's the correspondense between `S` and `U` template type parameters.)
>>
>>> `addnodeXorUtil.hpp`
>>
>> I'm fine with placing it under `opto`. Please, rename the file into `src/hotspot/share/opto/utilities/xor.hpp`.
>
> @iwanowww
>
>> `_non_neg` part is confusing; I'd stress instead that it works on ranges.
>
> I find it easier to think of it as calculating the upperbound of the xor of 2 non-negative integers whose upperbounds are given in the parameters.
Renamed to `xor_upper_bound_for_ranges` before I saw your comment, @merykitty. I'd be ok with another name though. With the last changes, the method is no longer a member of the class, so it's no longer going to get as many eyes on it without context, so maybe it matters less now.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23089#issuecomment-2767917005
More information about the hotspot-compiler-dev
mailing list