RFR: 8322589: Add Ideal transformation: (~a) & (~b) => ~(a | b)

[Emanuel Peter]

On Tue, 24 Oct 2023 04:49:20 GMT, Zhiqiang Zang <duke at openjdk.org> wrote:

> Hello,
> 
> `(~a) & (~b) => ~(a | b)` is a widely seen pattern, for example it is implemented for LLVM [here](https://github.com/llvm/llvm-project/blob/397f1ce9efb4eea1ee10fe4833f733b8c7abd878/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp#L1616C28-L1616C28); however it is missing in current implementation of hotspot. This pull request adds this transformation and associated tests.
> 
> Thanks.

Looks like a good idea :)
I left a few suggestions below.

src/hotspot/share/opto/mulnode.cpp line 617:

> 615:       && phase->type(in(1)->in(2)) == TypeInt::MINUS_1
> 616:       && in(2)->Opcode() == Op_XorI
> 617:       && in(1)->in(2) == in(2)->in(2)) {

minor code style issue: please take the `&&` to the end of the line. That is what I usually see. It also makes reading the lines easier, as they are aligned with the first line.

src/hotspot/share/opto/mulnode.cpp line 618:

> 616:       && in(2)->Opcode() == Op_XorI
> 617:       && in(1)->in(2) == in(2)->in(2)) {
> 618:     return new XorINode(phase->transform(new OrINode(in(1)->in(1), in(2)->in(1))), in(1)->in(2));

The nesting of this line is difficult to read. I suggest you take multiple lines and name intermediate results with something helpful.

test/hotspot/jtreg/compiler/c2/irTests/AndINodeIdealizationTests.java line 50:

> 48: 
> 49:         assertResult(0, 0);
> 50:         assertResult(a, a);

Suggestion:

        assertResult(a, b);

I assume you wanted this? Otherwise `b` is useless ;)

test/hotspot/jtreg/compiler/c2/irTests/AndLNodeIdealizationTests.java line 50:

> 48: 
> 49:         assertResult(0, 0);
> 50:         assertResult(a, a);

Suggestion:

        assertResult(a, b);

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16333#pullrequestreview-1806210975
PR Review Comment: https://git.openjdk.org/jdk/pull/16333#discussion_r1443035127
PR Review Comment: https://git.openjdk.org/jdk/pull/16333#discussion_r1443038716
PR Review Comment: https://git.openjdk.org/jdk/pull/16333#discussion_r1443043016
PR Review Comment: https://git.openjdk.org/jdk/pull/16333#discussion_r1443043862