RFR: 8347645: C2: XOR bounded value handling blocks constant folding [v4]

[Emanuel Peter]

On Wed, 22 Jan 2025 20:23:02 GMT, Johannes Graham <duke at openjdk.org> wrote:

>> C2 does not eliminate XOR nodes with constant arguments. This has a noticeable effect on `Long.expand` with a constant mask, on architectures that don't have instructions equivalent  to `PDEP` to be used in an intrinsic.
>> 
>> This patch demonstrates a potential fix to the problem, but there might well be better ways to do it.
>
> Johannes Graham has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix test names

@j3graham Thanks for working on this.

I see you added some constant-folding tests, thanks for randomizing the constants and fixing the comments!

You do not just touch constant-folding code, but also the `BOOL` case and other non-constant cases. Do we have adequate testing for those? I'm especially asking because we are now changing the order of those optimizations - and it would be a shame if we got a regression because of it and did not catch if because we have no tests.

Maybe we do have tests, then you can just point me to them. Otherwise, it would be great if you could add some more IR tests for those cases as well :)

Thanks again!

src/hotspot/share/opto/addnode.cpp line 994:

> 992:   const TypeInt *r1 = t1->is_int();
> 993: 
> 994:   if( !r0->is_con() || !r1->is_con() ) {

Suggestion:

  if( !r0->is_con() || !r1->is_con()) {

That is the new style, please apply it everywhere in the code.

src/hotspot/share/opto/addnode.cpp line 1008:

> 1006:       const TypeInt* t2x = TypeInt::make(0, round_down_power_of_2(r1->_hi) + (round_down_power_of_2(r1->_hi) - 1), r1->_widen);
> 1007:       return t1x->meet(t2x);
> 1008:     }

I know you only moved this code - but can you add some more comments about what it does? Maybe add some examples? It would help me review this code faster if I don't have to figure it out myself ;)

src/hotspot/share/opto/addnode.cpp line 1041:

> 1039:       const TypeLong* t2x = TypeLong::make(0, round_down_power_of_2(r1->_hi) + (round_down_power_of_2(r1->_hi) - 1), r1->_widen);
> 1040:       return t1x->meet(t2x);
> 1041:     }

Is there a reason why the `BOOL` check is not here as well? Does `long` not have one as well? This could be a follow-up RFE of course.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23089#pullrequestreview-2569139266
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1926540045
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1926546031
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1926547601