RFR: 8249893: AARCH64: optimize the construction of the value from the bits of the other two [v4]

[Andrew Dinn]

On Sun, 1 Nov 2020 17:14:12 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> Let me revive the change request [3] to C2 and AArch64 that applies Bitfield Insert instruction in the expression "(v1 & 0xFF) | ((v2 & 0xFF) << 8)". 
>>  
>> Compared to the last round of review [2] I updated the transformation to apply BFI in more cases and added a jtreg test.
>> 
>> As before, compared to the original patch [1], the transformation logic is now in the common C2 code: a new BitfieldInsert node has been introduced to replace Or+Shift+And sequence when possible, on AARCH a single BFI instruction is emitted for the new node.
>>  
>> [1] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-July/039161.html 
>> [2] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-August/039653.html
>> [3] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-August/039792.html
>
> Boris Ulasevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   adding conversion ((a & 0xff) << 8) + (b & 0xff) -> ((a & 0xff) << 8) | (b & 0xff)

src/hotspot/share/opto/addnode.cpp line 848:

> 846:     Node *src = in(2);
> 847:     int offset = 0;
> 848:     // Convert Or(v1, Shift(And(v2, 0xFF), shift)) into BitfieldInsert(dst, src, width, offset)

This comment is incoherent, misleading and totally inadequate. What is the 'Or' there for? Why doe it describe an And nested in a Shift when the code expects other cases. This is no use whatsoever to anyone who has to maintain your code.

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

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