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

[Johannes Graham]

On Tue, 25 Feb 2025 07:49:27 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> test/hotspot/jtreg/compiler/c2/irTests/XorINodeIdealizationTests.java line 334:
>> 
>>> 332:         var xor = (x & 0b111) ^ (y & 0b100);
>>> 333:         return xor < 0b1000;
>>> 334:     }
>> 
>> These are nice simple examples, and we should keep them. But I'm missing these cases with randomization.
>> 
>> `calc_xor_upper_bound_of_non_neg` basically has two input ranges `[0, hi_0]` and `[0, hi_1]`, and gives us a new range `[0,max]`.
>> 
>> You could produce random input ranges like this:
>> 
>> public void test(int x) {
>>     int lo_x = con1;
>>     int hi_x = con2;
>>     x = x < lo_x ? lo_x : (x > hi_x ? hi_x : x);
>>     // x clamped to [lo_x, hi_x]
>>     int lo_y = con3;
>>     int hi_y = con4;
>>     y = y < lo_y ? lo_y : (y > hi_y ? hi_y : y);
>>     // y clamped to [lo_y, hi_y]
>>     int z = x ^ y;
>>     // This should now have a new range, possibly some [0, max]
>>     // Now let's test the range with some random if branches.
>>     int sum = 0;
>>     if (z > somecon1) { sum += 1; }
>>     if (z > somecon2) { sum += 2; }
>>     if (z > somecon3) { sum += 4; }
>>     // maybe add a few more...
>>     if (z > someconi) { sum += pow(2,i); }
>>     return sum;
>> }
>> 
>> The `sum` at the end gives you a summary over all the checks. If one wrongly constant folds, you'll be missing one of the power of 2 contributions to it, or have it wrongly added.
>> Now you do this with an `x` and a `y`
>> 
>> Maybe that's a little over-engineered, but it would target the `calc_xor_upper_bound_of_non_neg` logic really well.
>> What do you think?
>
> FYI I'm working on fuzzing the compiler currently, so I'm thinking about how to write such tests more generally, that's why I took the time to come up with this. If you have better ideas, I'm more than happy to see them ;)

I started down the path of more elaborate randomization (see code removed in https://github.com/openjdk/jdk/pull/23089/commits/4a2912021103cefbe30fb3cc9e7d303b63ea454d). Instead of that approach. I went with doing the more detailed coverage with gtest, and having just a few specific checks in the IR tests. For example the "pow2" tests are there because that was a scenario that caused some trouble. I think there is a relatively small set of "interesting" cases that are nice to cover with something deterministic. (I could add a few more tests with hard-coded interesting values). It feels disproportionate to do something more complicated for this PR. 

That said, if there was more tooling in place to make more broad random tests easier to write, it would be very attractive. Looking at PR https://github.com/openjdk/jdk/pull/23418, it looks like that's what you've got in mind. Specifically, these kinds of tests with constants are begging for a way to spew out a whole bunch of related test methods.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1970116155