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

Emanuel Peter epeter at openjdk.org
Fri Apr 5 10:08:13 UTC 2024


On Tue, 26 Mar 2024 08:31:48 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> This PR resolves [JDK-8327381](https://bugs.openjdk.org/browse/JDK-8327381)
>> 
>> Currently the transformations for expressions with patterns `((x & m) u<= m)` or `((m & x) u<= m)` to `true` is in `BoolNode::Ideal` function with a new constant node of value `1` created. However, this is technically a type-improving (reduction in range) transformation that's better suited in `BoolNode::Value` function.
>> 
>> New unit test `test/hotspot/jtreg/compiler/c2/TestBoolNodeGvn.java` asserting on IR nodes and correctness of this transformation is added and passing.
>
> Kangcheng Xu has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - update comments
>  - fix indentation again

Looks better, thanks for the updates.
I had another idea to make the code a bit more simple.

FYI: I'll be out of the office next week.

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) {

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18198#pullrequestreview-1982536313
PR Review Comment: https://git.openjdk.org/jdk/pull/18198#discussion_r1553305055


More information about the hotspot-compiler-dev mailing list