RFR: 8373134: C2: Min/Max users of Min/Max uses should be enqueued for GVN [v6]

Emanuel Peter epeter at openjdk.org
Fri Jan 9 13:49:51 UTC 2026


On Thu, 8 Jan 2026 10:42:27 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> Min/Max users of Min/Max uses need to be enqueued respectively to the GVN worklist to see if further optimizations can be applied. Without this, there are cases where additional potential ideal/identity optimizations are not applied. I need this fix to test min/max reassociation implementation with IR tests reliably.
>> 
>> Aside from the fix itself, I've refactored `MaxNode` to `MinMaxNode` and added a `is_MinMax` node query to simplify the fix.
>> 
>> I have also removed the Min/Max exceptions in `PhaseIterGVN::verify_Identity_for` since this fix fixes `compiler/codegen/TestBooleanVect.java` with `-XX:VerifyIterativeGVN=1110`.
>> 
>> To test this I've created a template framework test that validates the fix. I have tested with all Min/Max combinations including Float16, which I've verified with Intel SDE. Float16 does not use `Argument.NUMBER_42` because there's no support for it yet, see [JDK-8373977](https://bugs.openjdk.org/browse/JDK-8373977).
>> 
>> During development I noticed that the test only failed when the test had `b, a` parameters in that order, so I added tests for both cases as `a, b` and `b, a` so that all possible orders are covered and they don't slip in the future.
>> 
>> I've run tier1-3 tests on linux/x64 successfully.
>
> Galder Zamarreño has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits:
> 
>  - Merge branch 'master' into topic.uses-min-max
>  - Module also needed in the wrapper test class
>  - It's the templated test that needs the module
>  - Add missing module to test
>  - Merge branch 'master' into topic.uses-min-max
>  - Test Float16
>  - Only apply to uses that match original IR node
>  - Merge branch 'master' into topic.uses-min-max
>  - Use is_MinMax() instead of spelling out individual Min/Max opcodes
>  - Refactor MaxNode to MinMaxNode and add is_MinMax() query
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/067fd3cb...7229e345

Looks great, thanks for working on this Galder!

src/hotspot/share/opto/addnode.hpp line 334:

> 332: 
> 333: public:
> 334:   MinMaxNode( Node *in1, Node *in2 ) : AddNode(in1,in2) {

Suggestion:

  MinMaxNode(Node* in1, Node* in2) : AddNode(in1, in2) {

Might as well fix code style while touching it.

test/hotspot/jtreg/compiler/igvn/TestMinMaxIdentity.java line 26:

> 24: /*
> 25:  * @test
> 26:  * @bug 8354244

Bug ID is different to issue number. Intentional?
Suggestion:

 * @bug 8373134

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

PR Review: https://git.openjdk.org/jdk/pull/28895#pullrequestreview-3643975774
PR Review Comment: https://git.openjdk.org/jdk/pull/28895#discussion_r2676235207
PR Review Comment: https://git.openjdk.org/jdk/pull/28895#discussion_r2676243435


More information about the hotspot-compiler-dev mailing list