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

[Quan Anh Mai]

On Fri, 31 Jan 2025 07:12:07 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:
> 
>   move template def to header

src/hotspot/share/opto/addnode.hpp line 257:

> 255: 
> 256: template<class S, class U>
> 257: static S calc_xor_max(const S hi_0, const S hi_1) {

If this is exposed in the header file, then I think it would be better to have it be a static function of a class to avoid convoluting the global namespace. Suggestion: Can you have `XorINode::calc_max` which will call a static `calc_xor_max` inside the cpp file.

src/hotspot/share/opto/addnode.hpp line 465:

> 463: };
> 464: 
> 465: template<class S, class U> S calc_xor_max(const S hi_0, const S hi_1);

This can probably removed.

test/hotspot/gtest/opto/test_xor_node.cpp line 40:

> 38: 
> 39:   // Skip out-of-bounds values for convenience
> 40:   if(val_0> hi_0 || val_0 < S(0) || val_1 > hi_1 || val_1< S(0)) {

Please be consistent with the style.

test/hotspot/gtest/opto/test_xor_node.cpp line 50:

> 48: 
> 49: template <class S>
> 50: void test_exhaustive_values(S hi_0, S hi_1){

You may want to test exhaustively for the `hi` values here, too. E.g:

    void test_exhaustive(S limit) {
        for (S hi0 = 0; hi0 <= limit; hi0++) {
            for (S hi1 = 0; hi1 <= limit; hi1++) {
                S max = calc_max(hi0, hi1);
                for (S v0 = 0; v0 <= hi0; v0++) {
                    for (S v1 = 0; v1 <= hi1; v1++) {
                        S v = v0 | v1;
                        EXPECT_LE(v, max);
                    }
                }
            }
        }
    }

test/hotspot/jtreg/compiler/c2/irTests/XorINodeIdealizationTests.java line 41:

> 39:     private static final int CONST_2 = G.next();
> 40: 
> 41: 

Extra line break

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1937016636
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1937016935
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1937021237
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1937020738
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1937022273