RFR: 8327381 Refactor type-improving transformations in BoolNode::Ideal to BoolNode::Value [v8]

[Emanuel Peter]

On Fri, 5 Apr 2024 09:58:12 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Kangcheng Xu has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - update comments
>>  - fix indentation again
>
> src/hotspot/share/opto/subnode.cpp line 1816:
> 
>> 1814:     // Change ((x & m) u<= m) or ((m & x) u<= m) to always true
>> 1815:     // Same with ((x & m) u< m+1) and ((m & x) u< m+1)
>> 1816:     if (cop == Op_CmpU && cmp1->Opcode() == Op_AndI) {
> 
> You made this a bit more complicated than the original. Or was there a specific reason for the `is_Sub`? I'd do this:
> Suggestion:
> 
>   // Change ((x & m) u<= m) or ((m & x) u<= m) to always true
>   // Same with ((x & m) u< m+1) and ((m & x) u< m+1)
>   Node* cmp = in(1);
>   if (cmp != nullptr && cmp->Opcode() == Op_CmpU) {
>     Node* cmp1 = cmp->in(1);
>     Node* cmp2 = cmp->in(2);
>     if (cmp1->Opcode() == Op_AndI) {

You could also move the whole code to its own method, and name it something like `BoolNode::Value_cmpu_and_mask`. Maybe you find an even more descriptive name.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18198#discussion_r1553309782