From 837cf85f7d5917f03c61c9bb4b8efe021de92b77 Mon Sep 17 00:00:00 2001 From: Quan Anh Mai Date: Fri, 25 Aug 2023 17:48:27 +0000 Subject: [PATCH] 8312547: Max/Min nodes Value implementation could be improved Reviewed-by: thartmann, kvn --- src/hotspot/share/opto/addnode.cpp | 67 +++++++++++-------- .../irTests/MaxMinINodeIdealizationTests.java | 20 +++++- 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index cf8f58d8e23..7ed8bc4f4ef 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -216,22 +216,19 @@ Node *AddNode::Ideal(PhaseGVN *phase, bool can_reshape) { // the other input's symbols. const Type* AddNode::Value(PhaseGVN* phase) const { // Either input is TOP ==> the result is TOP - const Type *t1 = phase->type( in(1) ); - const Type *t2 = phase->type( in(2) ); - if( t1 == Type::TOP ) return Type::TOP; - if( t2 == Type::TOP ) return Type::TOP; - - // Either input is BOTTOM ==> the result is the local BOTTOM - const Type *bot = bottom_type(); - if( (t1 == bot) || (t2 == bot) || - (t1 == Type::BOTTOM) || (t2 == Type::BOTTOM) ) - return bot; + const Type* t1 = phase->type(in(1)); + const Type* t2 = phase->type(in(2)); + if (t1 == Type::TOP || t2 == Type::TOP) { + return Type::TOP; + } // Check for an addition involving the additive identity - const Type *tadd = add_of_identity( t1, t2 ); - if( tadd ) return tadd; + const Type* tadd = add_of_identity(t1, t2); + if (tadd != nullptr) { + return tadd; + } - return add_ring(t1,t2); // Local flavor of type addition + return add_ring(t1, t2); // Local flavor of type addition } //------------------------------add_identity----------------------------------- @@ -513,7 +510,9 @@ const Type *AddFNode::add_of_identity( const Type *t1, const Type *t2 ) const { // This also type-checks the inputs for sanity. Guaranteed never to // be passed a TOP or BOTTOM type, these are filtered out by pre-check. const Type *AddFNode::add_ring( const Type *t0, const Type *t1 ) const { - // We must be adding 2 float constants. + if (!t0->isa_float_constant() || !t1->isa_float_constant()) { + return bottom_type(); + } return TypeF::make( t0->getf() + t1->getf() ); } @@ -544,7 +543,9 @@ const Type *AddDNode::add_of_identity( const Type *t1, const Type *t2 ) const { // This also type-checks the inputs for sanity. Guaranteed never to // be passed a TOP or BOTTOM type, these are filtered out by pre-check. const Type *AddDNode::add_ring( const Type *t0, const Type *t1 ) const { - // We must be adding 2 double constants. + if (!t0->isa_double_constant() || !t1->isa_double_constant()) { + return bottom_type(); + } return TypeD::make( t0->getd() + t1->getd() ); } @@ -1367,9 +1368,12 @@ Node* MinLNode::Ideal(PhaseGVN* phase, bool can_reshape) { } //------------------------------add_ring--------------------------------------- -const Type *MinFNode::add_ring( const Type *t0, const Type *t1 ) const { - const TypeF *r0 = t0->is_float_constant(); - const TypeF *r1 = t1->is_float_constant(); +const Type* MinFNode::add_ring(const Type* t0, const Type* t1 ) const { + const TypeF* r0 = t0->isa_float_constant(); + const TypeF* r1 = t1->isa_float_constant(); + if (r0 == nullptr || r1 == nullptr) { + return bottom_type(); + } if (r0->is_nan()) { return r0; @@ -1389,9 +1393,12 @@ const Type *MinFNode::add_ring( const Type *t0, const Type *t1 ) const { } //------------------------------add_ring--------------------------------------- -const Type *MinDNode::add_ring( const Type *t0, const Type *t1 ) const { - const TypeD *r0 = t0->is_double_constant(); - const TypeD *r1 = t1->is_double_constant(); +const Type* MinDNode::add_ring(const Type* t0, const Type* t1) const { + const TypeD* r0 = t0->isa_double_constant(); + const TypeD* r1 = t1->isa_double_constant(); + if (r0 == nullptr || r1 == nullptr) { + return bottom_type(); + } if (r0->is_nan()) { return r0; @@ -1411,9 +1418,12 @@ const Type *MinDNode::add_ring( const Type *t0, const Type *t1 ) const { } //------------------------------add_ring--------------------------------------- -const Type *MaxFNode::add_ring( const Type *t0, const Type *t1 ) const { - const TypeF *r0 = t0->is_float_constant(); - const TypeF *r1 = t1->is_float_constant(); +const Type* MaxFNode::add_ring(const Type* t0, const Type* t1) const { + const TypeF* r0 = t0->isa_float_constant(); + const TypeF* r1 = t1->isa_float_constant(); + if (r0 == nullptr || r1 == nullptr) { + return bottom_type(); + } if (r0->is_nan()) { return r0; @@ -1433,9 +1443,12 @@ const Type *MaxFNode::add_ring( const Type *t0, const Type *t1 ) const { } //------------------------------add_ring--------------------------------------- -const Type *MaxDNode::add_ring( const Type *t0, const Type *t1 ) const { - const TypeD *r0 = t0->is_double_constant(); - const TypeD *r1 = t1->is_double_constant(); +const Type* MaxDNode::add_ring(const Type* t0, const Type* t1) const { + const TypeD* r0 = t0->isa_double_constant(); + const TypeD* r1 = t1->isa_double_constant(); + if (r0 == nullptr || r1 == nullptr) { + return bottom_type(); + } if (r0->is_nan()) { return r0; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java index e0ef87f95a9..f07cca4d1f1 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java @@ -28,7 +28,7 @@ import compiler.lib.ir_framework.*; /* * @test - * @bug 8290248 + * @bug 8290248 8312547 * @summary Test that Ideal transformations of MaxINode and MinINode are * being performed as expected. * @library /test/lib / @@ -45,9 +45,11 @@ public class MaxMinINodeIdealizationTests { "testMax2L", "testMax2R", "testMax2LNoLeftAdd", "testMax3", + "testMax4", "testMin1", "testMin2", - "testMin3"}) + "testMin3", + "testMin4"}) public void runPositiveTests() { int a = RunInfo.getRandom().nextInt(); int min = Integer.MIN_VALUE; @@ -73,10 +75,12 @@ public class MaxMinINodeIdealizationTests { Asserts.assertEQ(testMax2L(a) , testMax2R(a)); Asserts.assertEQ(Math.max(a >> 1, ((a >> 1) + 11)) , testMax2LNoLeftAdd(a)); Asserts.assertEQ(Math.max(a, a) , testMax3(a)); + Asserts.assertEQ(0 , testMax4(a)); Asserts.assertEQ(Math.min(((a >> 1) + 100), Math.min(((a >> 1) + 150), 200)), testMin1(a)); Asserts.assertEQ(Math.min(((a >> 1) + 10), ((a >> 1) + 11)) , testMin2(a)); Asserts.assertEQ(Math.min(a, a) , testMin3(a)); + Asserts.assertEQ(0 , testMin4(a)); } // The transformations in test*1 and test*2 can happen only if the compiler has enough information @@ -203,6 +207,18 @@ public class MaxMinINodeIdealizationTests { return Math.min(i, i); } + @Test + @IR(failOn = {IRNode.MAX_I}) + public int testMax4(int i) { + return Math.max(i, 0) < 0 ? 1 : 0; + } + + @Test + @IR(failOn = {IRNode.MIN_I}) + public int testMin4(int i) { + return Math.min(i, 0) > 0 ? 1 : 0; + } + @Run(test = {"testTwoLevelsDifferentXY", "testTwoLevelsNoLeftConstant", "testTwoLevelsNoRightConstant",