RFR: 8267332: xor value should handle bounded values [v3]

[Tobias Hartmann]

On Thu, 20 May 2021 22:01:09 GMT, Nils Eliasson <neliasso at openjdk.org> wrote:

>> In the discussion of https://github.com/openjdk/jdk/pull/3938 a limitation in C2 was found. C2 fails to eliminate obvious bound checks for indexes that are masked with xor.
>> 
>> The Xor for two values that have a lower bound of zero or more, the resulting lower bound is zero.
>> The Xor for two values that have a upperbound above zero, the resulting upper bound is the max of the next_power_of_2-1.
>> 
>> Test supplied. 
>> 
>> Please review,
>> Best regards,
>> Nils Eliasson
>
> Nils Eliasson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed missing test case

Very nice. I've added some minor comments to the test.

test/hotspot/jtreg/compiler/types/TestMeetXor.java line 25:

> 23: 
> 24: /*
> 25:  * @test

This test looks like a good candidate for IR verification once the framework is integrated. Maybe file a follow-up RFE.

test/hotspot/jtreg/compiler/types/TestMeetXor.java line 28:

> 26:  * @bug 8267332
> 27:  * @summary Test meet on xor
> 28:  * @library /test/lib /

Test should have `@key randomness`

test/hotspot/jtreg/compiler/types/TestMeetXor.java line 39:

> 37: public class TestMeetXor {
> 38:     public static void main(String[] args) throws Exception {
> 39:         for (int i = 0; i < 10000; i++) {

Maybe increase number of iterations to make sure C2 compilation is triggered (and maybe also add `-Xbatch`).

test/hotspot/jtreg/compiler/types/TestMeetXor.java line 49:

> 47: 
> 48:     static int[] count = new int[256];
> 49:     static Random r = new Random(5);

You should use `jdk.test.lib.Utils.getRandomInstance()` here which also prints the seed.

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

Marked as reviewed by thartmann (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4136