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