RFR: 8276162: Optimise unsigned comparison pattern [v2]

[Vladimir Kozlov]

On Tue, 9 Nov 2021 02:05:01 GMT, Mai Đặng Quân Anh <duke at openjdk.java.net> wrote:

>> This patch changes operations in the form `x +- Integer.MIN_VALUE <=> y +- Integer.MIN_VALUE`, which is a pattern used to do unsigned comparisons, into `x u<=> y`.
>> 
>> In addition to being basic operations, they may be utilised to implement range checks such as the methods in `jdk.internal.util.Preconditions`, or in places where the compiler cannot deduce the non-negativeness of the bound as in `java.util.ArrayList`.
>> 
>> Thank you very much.
>
> Mai Đặng Quân Anh has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix copyright declaration

src/hotspot/share/opto/subnode.cpp line 1533:

> 1531: 
> 1532:   const int cmp1_op = cmp1->Opcode();
> 1533:   const int cmp2_op = cmp2->Opcode();

Use these in previous code too by move them to line `#1485` and replacing `op2`.

src/hotspot/share/opto/subnode.cpp line 1535:

> 1533:   const int cmp2_op = cmp2->Opcode();
> 1534: 
> 1535:   // Change x +- Integer.MIN_VALUE <=> y +- Integer.MIN_VALUE into x u<=> y

The comment should include `cmp2_op == Op_ConI` case.
Also it is not clear from comment and code  if different operations are allowed on both side: `x - MIN_INT <= y + MIN_INT`

src/hotspot/share/opto/subnode.cpp line 1537:

> 1535:   // Change x +- Integer.MIN_VALUE <=> y +- Integer.MIN_VALUE into x u<=> y
> 1536:   if ((_test._test == BoolTest::lt || _test._test == BoolTest::le ||
> 1537:         _test._test == BoolTest::gt || _test._test == BoolTest::ge) &&

You can use `(_test. is_less() || _test.is_greater())` here.

src/hotspot/share/opto/subnode.cpp line 1538:

> 1536:   if ((_test._test == BoolTest::lt || _test._test == BoolTest::le ||
> 1537:         _test._test == BoolTest::gt || _test._test == BoolTest::ge) &&
> 1538:       cop == Op_CmpI &&

`cop == Op_CmpI` should be first check before `_test` check.

src/hotspot/share/opto/subnode.cpp line 1542:

> 1540:       phase->type(cmp1->in(2)) == TypeInt::MIN) {
> 1541:     if (cmp2_op == Op_ConI) {
> 1542:       Node *ncmp2 = phase->intcon(java_add(cmp2->get_int(), min_jint));

What if `cmp1_op == Op_SubI` ?

src/hotspot/share/opto/subnode.cpp line 1545:

> 1543:       Node *ncmp = phase->transform(new CmpUNode(cmp1->in(1), ncmp2));
> 1544:       return new BoolNode(ncmp, _test._test);
> 1545:     } else if ((cmp2_op == Op_AddI || cmp2_op == Op_SubI) &&

Again the question about mismatching cmp1_op and cmp2_op.

src/hotspot/share/opto/subnode.cpp line 1555:

> 1553:   if ((_test._test == BoolTest::lt || _test._test == BoolTest::le ||
> 1554:         _test._test == BoolTest::gt || _test._test == BoolTest::ge) &&
> 1555:       cop == Op_CmpL &&

Same suggestions as for `Op_CmpI`

test/micro/org/openjdk/bench/vm/compiler/UnsignedComparison.java line 43:

> 41:     public long compareLong(long arg0, long arg1) {
> 42:         return arg0 + Long.MIN_VALUE < arg1 + Long.MIN_VALUE ? 1 : 0;
> 43:     }

Add tests for subtraction.

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

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