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

[Vladimir Ivanov]

On Thu, 5 Nov 2020 09:27:18 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:
> 
>   comprehensive comment and code cleanup for BitfieldInsert transformation

Changes requested by vlivanov (Reviewer).

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

> 244: static juint value_range_mask(PhaseGVN *phase, Node* value) {
> 245:   int opcode = value->Opcode();
> 246:   if (opcode == Op_LShiftI && value->in(2)->is_Con()) {

`Node::is_Con()` returns `true` for TOP node, so it's incorrect to assume the value is of type int after the check.

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

> 847:   }
> 848:   // major_progress() check postpones the transformation after loop optimization
> 849:   if (can_reshape && !phase->C->major_progress() && Matcher::match_rule_supported(Op_BitfieldInsertI)) {

There's `C->post_loop_opts_phase()` flag added recently specifically for that purpose.

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

> 957:     }
> 958:   }
> 959:   if (can_reshape && !phase->C->major_progress() && Matcher::match_rule_supported(Op_BitfieldInsertL)) {

Same here: please, use `C->post_loop_opts_phase()` instead.

src/hotspot/share/opto/compile.cpp line 2000:

> 1998: 
> 1999: // execute posponed transformations
> 2000: void Compile::post_optimize_loops_igvn(PhaseIterGVN& igvn) {

Preferred way is to register nodes for processing using `Compile::record_for_post_loop_opts_igvn()`.
That way you avoid additional pass over the graph.

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

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