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 239:

> 237: 
> 238: //---------------------------Bitmask helper------------------------------------
> 239: // Get a bitmask for expression: "(a & 0xFF) << 8" -> 0xFF00

This comment is thoroughly misleading and inadequate. Please describe correctly what this routine does and why.

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

> 258: 
> 259: static julong get_bitmaskL(PhaseGVN *phase, Node* value) {
> 260:   int opcode = value->Opcode();

If you are not going to provide an equivalent comment to explain this routine then at least point the reader back to the comment for the preceding routine.

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

> 361:   }
> 362: 
> 363:   // Convert "((a & 0xFF) << 8) + (b & 0xFF)" into "((a & 0xFF) << 8) | (b & 0xFF)"

Where do the constants 0xFF and 8 in this comment come from?

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

> 486: 
> 487:   // Convert "((a & 0xFF) << 8) + (b & 0xFF)" into "((a & 0xFF) << 8) | (b & 0xFF)"
> 488:   julong bitmask1 = get_bitmaskL(phase, in(1));

Same question as before.

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

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