RFR: 8324655: Identify integer minimum and maximum patterns created with if statements [v2]
Emanuel Peter
epeter at openjdk.org
Wed Feb 7 09:54:56 UTC 2024
On Wed, 7 Feb 2024 09:41:12 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Don't transform highly predictable branches
>
> src/hotspot/share/opto/cfgnode.cpp line 1802:
>
>> 1800:
>> 1801: // Ensure comparison is an integral type
>> 1802: if (opcode == Op_CmpI || opcode == Op_CmpL) {
>
> Nicer would be "bailout" ifs:
> Suggestion:
>
> if (opcode != Op_CmpI && opcode != Op_CmpL) { return nullptr; }
> bool is_long = opcode == Op_CmpL;
>
> // Only accept canonicalized le and lt comparisons
> int test = bol->_test._test;
> if (test != BoolTest::le && test != BoolTest::lt) { return nullptr; }
>
>
> It is nice to have all the "bailout" checks as early as possible, and then do the "meat" of the optimization afterward.
This also avoids nesting: nesting is difficult to read and understand. A sequence of "bailouts" is easier to read.
> src/hotspot/share/opto/cfgnode.cpp line 1824:
>
>> 1822: } else {
>> 1823: return nullptr;
>> 1824: }
>
> Maybe also have variables `phi_true/false`, would make the code easier to read.
Hmm. Maybe you can refactor this, or explain somehow why exactly the conditions lead to `min_path = 1/2`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17574#discussion_r1481189784
PR Review Comment: https://git.openjdk.org/jdk/pull/17574#discussion_r1481208533
More information about the hotspot-compiler-dev
mailing list