From be1888f9c9c530010d2d0d428fd3c4d7814c65df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 12 Apr 2023 22:20:29 +0200 Subject: [PATCH 01/25] Clean up MinI/MaxI Ideal function --- src/hotspot/share/opto/addnode.cpp | 71 ++++++++++-------------------- src/hotspot/share/opto/addnode.hpp | 6 +++ 2 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 3c56f5bd77058..1d46f7ff78c09 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1067,6 +1067,18 @@ Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const return res; } +Node* MaxNode::constant_add_input(Node* n, jint* con) { + if (n->Opcode() == Op_AddI && n->in(2)->is_Con()) { + const Type* t = n->in(2)->bottom_type(); + if (t == Type::TOP) { + return nullptr; + } + *con = t->is_int()->get_con(); + n = n->in(1); + } + return n; +} + //============================================================================= //------------------------------add_ring--------------------------------------- // Supplied function returns the sum of the inputs. @@ -1103,27 +1115,15 @@ Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { } // Get left input & constant - Node* x = l; jint x_off = 0; - if (x->Opcode() == Op_AddI && // Check for "x+c0" and collect constant - x->in(2)->is_Con()) { - const Type* t = x->in(2)->bottom_type(); - if (t == Type::TOP) return nullptr; // No progress - x_off = t->is_int()->get_con(); - x = x->in(1); - } + Node* x = constant_add_input(l, &x_off); + if (x == nullptr) return nullptr; // Scan a right-spline-tree for MAXs - Node* y = r; jint y_off = 0; // Check final part of MAX tree - if (y->Opcode() == Op_AddI && // Check for "y+c1" and collect constant - y->in(2)->is_Con()) { - const Type* t = y->in(2)->bottom_type(); - if (t == Type::TOP) return nullptr; // No progress - y_off = t->is_int()->get_con(); - y = y->in(1); - } + Node* y = constant_add_input(r, &y_off); + if (x == nullptr) return nullptr; if (x->_idx > y->_idx && r->Opcode() != Op_MaxI) { swap_edges(1, 2); return this; @@ -1133,15 +1133,9 @@ Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { if (r->Opcode() == Op_MaxI) { assert(r != r->in(2), "dead loop in MaxINode::Ideal"); - y = r->in(1); // Check final part of MAX tree - if (y->Opcode() == Op_AddI &&// Check for "y+c1" and collect constant - y->in(2)->is_Con()) { - const Type* t = y->in(2)->bottom_type(); - if (t == Type::TOP) return nullptr; // No progress - y_off = t->is_int()->get_con(); - y = y->in(1); - } + y = constant_add_input(r->in(1), &y_off); + if (y == nullptr) return nullptr; if (x->_idx > y->_idx) return new MaxINode(r->in(1), phase->transform(new MaxINode(l, r->in(2)))); @@ -1186,27 +1180,15 @@ Node *MinINode::Ideal(PhaseGVN *phase, bool can_reshape) { } // Get left input & constant - Node *x = l; jint x_off = 0; - if( x->Opcode() == Op_AddI && // Check for "x+c0" and collect constant - x->in(2)->is_Con() ) { - const Type *t = x->in(2)->bottom_type(); - if( t == Type::TOP ) return nullptr; // No progress - x_off = t->is_int()->get_con(); - x = x->in(1); - } + Node* x = constant_add_input(l, &x_off); + if (x == nullptr) return nullptr; // Scan a right-spline-tree for MINs - Node *y = r; jint y_off = 0; // Check final part of MIN tree - if( y->Opcode() == Op_AddI && // Check for "y+c1" and collect constant - y->in(2)->is_Con() ) { - const Type *t = y->in(2)->bottom_type(); - if( t == Type::TOP ) return nullptr; // No progress - y_off = t->is_int()->get_con(); - y = y->in(1); - } + Node* y = constant_add_input(r, &y_off); + if (y == nullptr) return nullptr; if( x->_idx > y->_idx && r->Opcode() != Op_MinI ) { swap_edges(1, 2); return this; @@ -1216,15 +1198,8 @@ Node *MinINode::Ideal(PhaseGVN *phase, bool can_reshape) { if( r->Opcode() == Op_MinI ) { assert( r != r->in(2), "dead loop in MinINode::Ideal" ); - y = r->in(1); // Check final part of MIN tree - if( y->Opcode() == Op_AddI &&// Check for "y+c1" and collect constant - y->in(2)->is_Con() ) { - const Type *t = y->in(2)->bottom_type(); - if( t == Type::TOP ) return nullptr; // No progress - y_off = t->is_int()->get_con(); - y = y->in(1); - } + y = constant_add_input(r->in(1), &y_off); if( x->_idx > y->_idx ) return new MinINode(r->in(1),phase->transform(new MinINode(l,r->in(2)))); diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index fb47972e8b328..7d7cdea19da78 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -284,6 +284,12 @@ class MaxNode : public AddNode { static Node* min_diff_with_zero(Node* a, Node* b, const Type* t, PhaseGVN& gvn) { return build_min_max_diff_with_zero(a, b, false, t, gvn); } + + // Return: + // , if n is of the form x + C, where 'C' is a non-TOP constant; + // , if n is of the form x + C, where 'C' is a TOP constant; + // otherwise. + static Node* constant_add_input(Node* n, jint* con); }; //------------------------------MaxINode--------------------------------------- From 9b218df8e547c492d28b780033195237abf3309b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 12 Apr 2023 22:22:56 +0200 Subject: [PATCH 02/25] Add basic test from https://github.com/openjdk/jdk/pull/13260 --- .../compiler/lib/ir_framework/IRNode.java | 10 ++ .../loopopts/superword/MinMaxRed_Int.java | 108 ++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java index 127e96f97d755..9da6a48810a3c 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java @@ -804,6 +804,16 @@ public class IRNode { superWordNodes(MUL_REDUCTION_VL, "MulReductionVL"); } + public static final String MIN_REDUCTION_V = PREFIX + "MIN_REDUCTION_V" + POSTFIX; + static { + superWordNodes(MIN_REDUCTION_V, "MinReductionV"); + } + + public static final String MAX_REDUCTION_V = PREFIX + "MAX_REDUCTION_V" + POSTFIX; + static { + superWordNodes(MAX_REDUCTION_V, "MaxReductionV"); + } + public static final String NEG_V = PREFIX + "NEG_V" + POSTFIX; static { beforeMatchingNameRegex(NEG_V, "NegV(F|D)"); diff --git a/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java b/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java new file mode 100644 index 0000000000000..a1a3f413fd956 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8302673 + * @summary [SuperWord] MaxReduction and MinReduction should vectorize for int + * @library /test/lib / + * @run driver compiler.loopopts.superword.MinMaxRed_Int + */ + +package compiler.loopopts.superword; + +import compiler.lib.ir_framework.*; + +public class MinMaxRed_Int { + public static void main(String[] args) throws Exception { + TestFramework framework = new TestFramework(); + framework.addFlags("-XX:+IgnoreUnrecognizedVMOptions", + "-XX:LoopUnrollLimit=250", + "-XX:CompileThresholdScaling=0.1"); + framework.start(); + } + + @Run(test = {"maxReductionImplement"}, + mode = RunMode.STANDALONE) + public void runMaxTest() { + int[] a = new int[1024]; + int[] b = new int[1024]; + ReductionInit(a, b); + int res = 0; + for (int j = 0; j < 2000; j++) { + res = maxReductionImplement(a, b, res); + } + if (res == 0) { + System.out.println("Success"); + } else { + throw new AssertionError("Failed"); + } + } + + /* + @Run(test = {"minReductionImplement"}, + mode = RunMode.STANDALONE) + public void runMinTest() { + int[] a = new int[1024]; + int[] b = new int[1024]; + ReductionInit(a, b); + int res = 1; + for (int j = 0; j < 2000; j++) { + res = minReductionImplement(a, b, res); + } + if (res == -1023*1023) { + System.out.println("Success"); + } else { + throw new AssertionError("Failed"); + } + } + */ + public static void ReductionInit(int[] a, int[] b) { + for (int i = 0; i < a.length; i++) { + a[i] = -i; + b[i] = i; + } + } + /* + @Test + @IR(applyIf = {"SuperWordReductions", "true"}, + applyIfCPUFeatureOr = { "sse4.1", "true" , "asimd" , "true"}, + counts = {IRNode.MIN_REDUCTION_V, " > 0"}) + public static int minReductionImplement(int[] a, int[] b, int res) { + for (int i = 0; i < a.length; i++) { + res = Math.min(res, a[i] * b[i]); + } + return res; + } + */ + @Test + @IR(applyIf = {"SuperWordReductions", "true"}, + applyIfCPUFeatureOr = { "sse4.1", "true" , "asimd" , "true"}, + counts = {IRNode.MAX_REDUCTION_V, " > 0"}) + public static int maxReductionImplement(int[] a, int[] b, int res) { + for (int i = 0; i < a.length; i++) { + res = Math.max(res, a[i] * b[i]); + } + return res; + } +} From cd8422736f90fdec22b51e24d340c9e1c7110d7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 14 Apr 2023 13:53:18 +0200 Subject: [PATCH 03/25] Added more Max test cases with swapped inputs --- .../irTests/MaxMinINodeIdealizationTests.java | 50 +++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java index 2a42c503d539a..fbee7e1f42e16 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java @@ -39,7 +39,12 @@ public static void main(String[] args) { TestFramework.run(); } - @Run(test = {"testMax1", "testMax2", "testMax3", "testMin1", "testMin2", "testMin3"}) + @Run(test = {"testMax1LL", "testMax1LR", "testMax1RL", "testMax1RR", + "testMax2L", "testMax2R", + "testMax3", + "testMin1", + "testMin2", + "testMin3"}) public void runMethod() { int a = RunInfo.getRandom().nextInt(); int min = Integer.MIN_VALUE; @@ -53,8 +58,12 @@ public void runMethod() { @DontCompile public void assertResult(int a) { - Asserts.assertEQ(Math.max(((a >> 1) + 100), Math.max(((a >> 1) + 150), 200)), testMax1(a)); - Asserts.assertEQ(Math.max(((a >> 1) + 10), ((a >> 1) + 11)) , testMax2(a)); + Asserts.assertEQ(Math.max(Math.max(((a >> 1) + 150), 200), ((a >> 1) + 100)), testMax1LL(a)); + Asserts.assertEQ(testMax1LL(a) , testMax1LR(a)); + Asserts.assertEQ(testMax1LL(a) , testMax1RL(a)); + Asserts.assertEQ(testMax1LL(a) , testMax1RR(a)); + Asserts.assertEQ(Math.max(((a >> 1) + 10), ((a >> 1) + 11)) , testMax2L(a)); + Asserts.assertEQ(testMax2L(a) , testMax2R(a)); Asserts.assertEQ(Math.max(a, a) , testMax3(a)); Asserts.assertEQ(Math.min(((a >> 1) + 100), Math.min(((a >> 1) + 150), 200)), testMin1(a)); @@ -72,10 +81,34 @@ public void assertResult(int a) { @IR(counts = {IRNode.MAX_I, "1", IRNode.ADD , "1", }) - public int testMax1(int i) { + public int testMax1LL(int i) { + return Math.max(Math.max(((i >> 1) + 150), 200), ((i >> 1) + 100)); + } + + @Test + @IR(counts = {IRNode.MAX_I, "1", + IRNode.ADD , "1", + }) + public int testMax1LR(int i) { + return Math.max(Math.max(200, ((i >> 1) + 150)), ((i >> 1) + 100)); + } + + @Test + @IR(counts = {IRNode.MAX_I, "1", + IRNode.ADD , "1", + }) + public int testMax1RL(int i) { return Math.max(((i >> 1) + 100), Math.max(((i >> 1) + 150), 200)); } + @Test + @IR(counts = {IRNode.MAX_I, "1", + IRNode.ADD , "1", + }) + public int testMax1RR(int i) { + return Math.max(((i >> 1) + 100), Math.max(200, ((i >> 1) + 150))); + } + // Similarly, transform min(x + c0, min(y + c1, z)) to min(add(x, c2), z) if x == y, where c2 = MIN2(c0, c1). @Test @IR(counts = {IRNode.MIN_I, "1", @@ -91,10 +124,17 @@ public int testMin1(int i) { @Test @IR(failOn = {IRNode.MAX_I}) @IR(counts = {IRNode.ADD, "1"}) - public int testMax2(int i) { + public int testMax2L(int i) { return Math.max((i >> 1) + 10, (i >> 1) + 11); } + @Test + @IR(failOn = {IRNode.MAX_I}) + @IR(counts = {IRNode.ADD, "1"}) + public int testMax2R(int i) { + return Math.max((i >> 1) + 11, (i >> 1) + 10); + } + // Similarly, transform min(x + c0, y + c1) to add(x, c2) if x == y, where c2 = MIN2(c0, c1). @Test @IR(failOn = {IRNode.MIN_I}) From 3973102146928ee5b047d266d7db5e18633e430f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 14 Apr 2023 13:55:54 +0200 Subject: [PATCH 04/25] Handle all four max-of-max cases without canonicalization --- src/hotspot/share/opto/addnode.cpp | 154 ++++++++++++----------------- src/hotspot/share/opto/addnode.hpp | 2 + 2 files changed, 66 insertions(+), 90 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 1d46f7ff78c09..9e8aca31ee0f4 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1067,6 +1067,14 @@ Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const return res; } +// Check if addition of an integer with type 't' and a constant 'c' can overflow +static bool can_overflow(const TypeInt* t, jint c) { + jint t_lo = t->_lo; + jint t_hi = t->_hi; + return ((c < 0 && (java_add(t_lo, c) > t_lo)) || + (c > 0 && (java_add(t_hi, c) < t_hi))); +} + Node* MaxNode::constant_add_input(Node* n, jint* con) { if (n->Opcode() == Op_AddI && n->in(2)->is_Con()) { const Type* t = n->in(2)->bottom_type(); @@ -1079,6 +1087,36 @@ Node* MaxNode::constant_add_input(Node* n, jint* con) { return n; } +bool MaxNode::optimize_max_of_max(PhaseGVN* phase, bool min, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out) { + const TypeInt* tx = phase->type(x)->isa_int(); + for (uint input = 1; input <= 2; input++) { + Node* add = n->in(input); + Node* z = n->in(input == 1 ? 2 : 1); + if (left) { + x = constant_add_input(add, &x_off); + } else { + y = constant_add_input(add, &y_off); + } + if (x == nullptr || y == nullptr) { + *out = nullptr; + return true; + } + if (x == y && tx != nullptr && + !can_overflow(tx, x_off) && + !can_overflow(tx, y_off)) { + jint c2 = min ? MIN2(x_off, y_off) : MAX2(x_off, y_off); + Node* add_x_c2 = phase->transform(new AddINode(x, phase->intcon(c2))); + if (min) { + *out = new MinINode(add_x_c2, z); + } else { + *out = new MaxINode(add_x_c2, z); + } + return true; + } + } + return false; +} + //============================================================================= //------------------------------add_ring--------------------------------------- // Supplied function returns the sum of the inputs. @@ -1090,73 +1128,37 @@ const Type *MaxINode::add_ring( const Type *t0, const Type *t1 ) const { return TypeInt::make( MAX2(r0->_lo,r1->_lo), MAX2(r0->_hi,r1->_hi), MAX2(r0->_widen,r1->_widen) ); } -// Check if addition of an integer with type 't' and a constant 'c' can overflow -static bool can_overflow(const TypeInt* t, jint c) { - jint t_lo = t->_lo; - jint t_hi = t->_hi; - return ((c < 0 && (java_add(t_lo, c) > t_lo)) || - (c > 0 && (java_add(t_hi, c) < t_hi))); -} - // Ideal transformations for MaxINode Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { - // Force a right-spline graph Node* l = in(1); Node* r = in(2); - // Transform MaxI1(MaxI2(a, b), c) into MaxI1(a, MaxI2(b, c)) - // to force a right-spline graph for the rest of MaxINode::Ideal(). - if (l->Opcode() == Op_MaxI) { - assert(l != l->in(1), "dead loop in MaxINode::Ideal"); - r = phase->transform(new MaxINode(l->in(2), r)); - l = l->in(1); - set_req_X(1, l, phase); - set_req_X(2, r, phase); - return this; - } - - // Get left input & constant jint x_off = 0; Node* x = constant_add_input(l, &x_off); if (x == nullptr) return nullptr; - - // Scan a right-spline-tree for MAXs jint y_off = 0; - // Check final part of MAX tree Node* y = constant_add_input(r, &y_off); - if (x == nullptr) return nullptr; - if (x->_idx > y->_idx && r->Opcode() != Op_MaxI) { - swap_edges(1, 2); - return this; - } - - const TypeInt* tx = phase->type(x)->isa_int(); - - if (r->Opcode() == Op_MaxI) { - assert(r != r->in(2), "dead loop in MaxINode::Ideal"); - // Check final part of MAX tree - y = constant_add_input(r->in(1), &y_off); - if (y == nullptr) return nullptr; - - if (x->_idx > y->_idx) - return new MaxINode(r->in(1), phase->transform(new MaxINode(l, r->in(2)))); - - // Transform MAX2(x + c0, MAX2(x + c1, z)) into MAX2(x + MAX2(c0, c1), z) - // if x == y and the additions can't overflow. - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - return new MaxINode(phase->transform(new AddINode(x, phase->intcon(MAX2(x_off, y_off)))), r->in(2)); + if (y == nullptr) return nullptr; + Node* out; + if (l->Opcode() == Op_MaxI) { + if (optimize_max_of_max(phase, /*min=*/false, /*left=*/true, x, y, l, x_off, y_off, &out)) { + return out; + } + } else if (r->Opcode() == Op_MaxI) { + if (optimize_max_of_max(phase, /*min=*/false, /*left=*/false, x, y, r, x_off, y_off, &out)) { + return out; } } else { // Transform MAX2(x + c0, y + c1) into x + MAX2(c0, c1) // if x == y and the additions can't overflow. + const TypeInt* tx = phase->type(x)->isa_int(); if (x == y && tx != nullptr && !can_overflow(tx, x_off) && !can_overflow(tx, y_off)) { - return new AddINode(x, phase->intcon(MAX2(x_off, y_off))); + jint c2 = false ? MIN2(x_off, y_off) : MAX2(x_off, y_off); + return new AddINode(x, phase->intcon(c2)); } } - return nullptr; + return nullptr; } //============================================================================= @@ -1164,60 +1166,32 @@ Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { // MINs show up in range-check loop limit calculations. Look for // "MIN2(x+c0,MIN2(y,x+c1))". Pick the smaller constant: "MIN2(x+c0,y)" Node *MinINode::Ideal(PhaseGVN *phase, bool can_reshape) { - Node *progress = nullptr; - // Force a right-spline graph - Node *l = in(1); - Node *r = in(2); - // Transform MinI1( MinI2(a,b), c) into MinI1( a, MinI2(b,c) ) - // to force a right-spline graph for the rest of MinINode::Ideal(). - if( l->Opcode() == Op_MinI ) { - assert( l != l->in(1), "dead loop in MinINode::Ideal" ); - r = phase->transform(new MinINode(l->in(2),r)); - l = l->in(1); - set_req_X(1, l, phase); - set_req_X(2, r, phase); - return this; - } - - // Get left input & constant + Node* l = in(1); + Node* r = in(2); jint x_off = 0; Node* x = constant_add_input(l, &x_off); if (x == nullptr) return nullptr; - - // Scan a right-spline-tree for MINs jint y_off = 0; - // Check final part of MIN tree Node* y = constant_add_input(r, &y_off); if (y == nullptr) return nullptr; - if( x->_idx > y->_idx && r->Opcode() != Op_MinI ) { - swap_edges(1, 2); - return this; - } - - const TypeInt* tx = phase->type(x)->isa_int(); - - if( r->Opcode() == Op_MinI ) { - assert( r != r->in(2), "dead loop in MinINode::Ideal" ); - // Check final part of MIN tree - y = constant_add_input(r->in(1), &y_off); - - if( x->_idx > y->_idx ) - return new MinINode(r->in(1),phase->transform(new MinINode(l,r->in(2)))); - - // Transform MIN2(x + c0, MIN2(x + c1, z)) into MIN2(x + MIN2(c0, c1), z) - // if x == y and the additions can't overflow. - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - return new MinINode(phase->transform(new AddINode(x, phase->intcon(MIN2(x_off, y_off)))), r->in(2)); + Node* out; + if (l->Opcode() == Op_MinI) { + if (optimize_max_of_max(phase, /*min=*/true, /*left=*/true, x, y, l, x_off, y_off, &out)) { + return out; + } + } else if (r->Opcode() == Op_MinI) { + if (optimize_max_of_max(phase, /*min=*/true, /*left=*/false, x, y, r, x_off, y_off, &out)) { + return out; } } else { // Transform MIN2(x + c0, y + c1) into x + MIN2(c0, c1) // if x == y and the additions can't overflow. + const TypeInt* tx = phase->type(x)->isa_int(); if (x == y && tx != nullptr && !can_overflow(tx, x_off) && !can_overflow(tx, y_off)) { - return new AddINode(x,phase->intcon(MIN2(x_off,y_off))); + jint c2 = true ? MIN2(x_off, y_off) : MAX2(x_off, y_off); + return new AddINode(x,phase->intcon(c2)); } } return nullptr; diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 7d7cdea19da78..8ab04d3697be8 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -290,6 +290,8 @@ class MaxNode : public AddNode { // , if n is of the form x + C, where 'C' is a TOP constant; // otherwise. static Node* constant_add_input(Node* n, jint* con); + + static bool optimize_max_of_max(PhaseGVN* phase, bool min, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out); }; //------------------------------MaxINode--------------------------------------- From c56efa2d341b2a8d93e2855aebd0076bf896f7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 5 May 2023 22:39:42 +0200 Subject: [PATCH 05/25] Extract MaxINode::Ideal() and MinINode::Ideal() into MaxNode::IdealI() --- src/hotspot/share/opto/addnode.cpp | 101 +++++++++++------------------ src/hotspot/share/opto/addnode.hpp | 9 ++- 2 files changed, 47 insertions(+), 63 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 9e8aca31ee0f4..9b56fc4a3080f 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1087,7 +1087,40 @@ Node* MaxNode::constant_add_input(Node* n, jint* con) { return n; } -bool MaxNode::optimize_max_of_max(PhaseGVN* phase, bool min, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out) { +Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { + assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); + Node* l = in(1); + Node* r = in(2); + jint x_off = 0; + Node* x = constant_add_input(l, &x_off); + if (x == nullptr) return nullptr; + jint y_off = 0; + Node* y = constant_add_input(r, &y_off); + if (y == nullptr) return nullptr; + Node* out; + if (l->Opcode() == opcode) { + if (optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { + return out; + } + } else if (r->Opcode() == opcode) { + if (optimize_max_of_max(phase, opcode, /*left=*/false, x, y, r, x_off, y_off, &out)) { + return out; + } + } else { + // Transform MIN2/MAX2(x + c0, y + c1) into x + MIN2/MAX2(c0, c1) + // if x == y and the additions can't overflow. + const TypeInt* tx = phase->type(x)->isa_int(); + if (x == y && tx != nullptr && + !can_overflow(tx, x_off) && + !can_overflow(tx, y_off)) { + return new AddINode(x, phase->intcon(extreme(x_off, y_off, opcode))); + } + } + return nullptr; +} + +bool MaxNode::optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out) { + assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); const TypeInt* tx = phase->type(x)->isa_int(); for (uint input = 1; input <= 2; input++) { Node* add = n->in(input); @@ -1104,9 +1137,9 @@ bool MaxNode::optimize_max_of_max(PhaseGVN* phase, bool min, bool left, Node* x, if (x == y && tx != nullptr && !can_overflow(tx, x_off) && !can_overflow(tx, y_off)) { - jint c2 = min ? MIN2(x_off, y_off) : MAX2(x_off, y_off); + jint c2 = extreme(x_off, y_off, opcode); Node* add_x_c2 = phase->transform(new AddINode(x, phase->intcon(c2))); - if (min) { + if (opcode == Op_MinI) { *out = new MinINode(add_x_c2, z); } else { *out = new MaxINode(add_x_c2, z); @@ -1130,71 +1163,15 @@ const Type *MaxINode::add_ring( const Type *t0, const Type *t1 ) const { // Ideal transformations for MaxINode Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { - Node* l = in(1); - Node* r = in(2); - jint x_off = 0; - Node* x = constant_add_input(l, &x_off); - if (x == nullptr) return nullptr; - jint y_off = 0; - Node* y = constant_add_input(r, &y_off); - if (y == nullptr) return nullptr; - Node* out; - if (l->Opcode() == Op_MaxI) { - if (optimize_max_of_max(phase, /*min=*/false, /*left=*/true, x, y, l, x_off, y_off, &out)) { - return out; - } - } else if (r->Opcode() == Op_MaxI) { - if (optimize_max_of_max(phase, /*min=*/false, /*left=*/false, x, y, r, x_off, y_off, &out)) { - return out; - } - } else { - // Transform MAX2(x + c0, y + c1) into x + MAX2(c0, c1) - // if x == y and the additions can't overflow. - const TypeInt* tx = phase->type(x)->isa_int(); - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - jint c2 = false ? MIN2(x_off, y_off) : MAX2(x_off, y_off); - return new AddINode(x, phase->intcon(c2)); - } - } - return nullptr; + return IdealI(phase, can_reshape, Op_MaxI); } //============================================================================= //------------------------------Idealize--------------------------------------- // MINs show up in range-check loop limit calculations. Look for // "MIN2(x+c0,MIN2(y,x+c1))". Pick the smaller constant: "MIN2(x+c0,y)" -Node *MinINode::Ideal(PhaseGVN *phase, bool can_reshape) { - Node* l = in(1); - Node* r = in(2); - jint x_off = 0; - Node* x = constant_add_input(l, &x_off); - if (x == nullptr) return nullptr; - jint y_off = 0; - Node* y = constant_add_input(r, &y_off); - if (y == nullptr) return nullptr; - Node* out; - if (l->Opcode() == Op_MinI) { - if (optimize_max_of_max(phase, /*min=*/true, /*left=*/true, x, y, l, x_off, y_off, &out)) { - return out; - } - } else if (r->Opcode() == Op_MinI) { - if (optimize_max_of_max(phase, /*min=*/true, /*left=*/false, x, y, r, x_off, y_off, &out)) { - return out; - } - } else { - // Transform MIN2(x + c0, y + c1) into x + MIN2(c0, c1) - // if x == y and the additions can't overflow. - const TypeInt* tx = phase->type(x)->isa_int(); - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - jint c2 = true ? MIN2(x_off, y_off) : MAX2(x_off, y_off); - return new AddINode(x,phase->intcon(c2)); - } - } - return nullptr; +Node* MinINode::Ideal(PhaseGVN* phase, bool can_reshape) { + return IdealI(phase, can_reshape, Op_MinI); } //------------------------------add_ring--------------------------------------- diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 8ab04d3697be8..06734008b9255 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -285,13 +285,20 @@ class MaxNode : public AddNode { return build_min_max_diff_with_zero(a, b, false, t, gvn); } + static jint extreme(jint a, jint b, int opcode) { + assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); + return opcode == Op_MinI ? MIN2(a, b) : MAX2(a, b); + } + // Return: // , if n is of the form x + C, where 'C' is a non-TOP constant; // , if n is of the form x + C, where 'C' is a TOP constant; // otherwise. static Node* constant_add_input(Node* n, jint* con); - static bool optimize_max_of_max(PhaseGVN* phase, bool min, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out); + Node* IdealI(PhaseGVN* phase, bool can_reshape, int opcode); + + static bool optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out); }; //------------------------------MaxINode--------------------------------------- From 2149d34af7e28a2ae928dba9c5415e2846dd22f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 5 May 2023 22:55:01 +0200 Subject: [PATCH 06/25] Flatten nested if --- src/hotspot/share/opto/addnode.cpp | 32 ++++++++++++++---------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 9b56fc4a3080f..3257c2b47e3eb 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1098,23 +1098,21 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { Node* y = constant_add_input(r, &y_off); if (y == nullptr) return nullptr; Node* out; - if (l->Opcode() == opcode) { - if (optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { - return out; - } - } else if (r->Opcode() == opcode) { - if (optimize_max_of_max(phase, opcode, /*left=*/false, x, y, r, x_off, y_off, &out)) { - return out; - } - } else { - // Transform MIN2/MAX2(x + c0, y + c1) into x + MIN2/MAX2(c0, c1) - // if x == y and the additions can't overflow. - const TypeInt* tx = phase->type(x)->isa_int(); - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - return new AddINode(x, phase->intcon(extreme(x_off, y_off, opcode))); - } + if (l->Opcode() == opcode && + optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { + return out; + } + if (r->Opcode() == opcode && + optimize_max_of_max(phase, opcode, /*left=*/false, x, y, r, x_off, y_off, &out)) { + return out; + } + // Transform MIN2/MAX2(x + c0, y + c1) into x + MIN2/MAX2(c0, c1) + // if x == y and the additions can't overflow. + const TypeInt* tx = phase->type(x)->isa_int(); + if (x == y && tx != nullptr && + !can_overflow(tx, x_off) && + !can_overflow(tx, y_off)) { + return new AddINode(x, phase->intcon(extreme(x_off, y_off, opcode))); } return nullptr; } From 7adf075429668fbe8b0c81ba5382ef156acad789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 5 May 2023 23:01:07 +0200 Subject: [PATCH 07/25] Re-add comment --- src/hotspot/share/opto/addnode.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 3257c2b47e3eb..0537495b48a70 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1098,6 +1098,8 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { Node* y = constant_add_input(r, &y_off); if (y == nullptr) return nullptr; Node* out; + // Transform MIN2/MAX2(x + c0, MIN2/MAX2(x + c1, z)) into MIN2/MAX2(x + MAX2/MAX2(c0, c1), z) + // if x == y and the additions can't overflow. Handle the four possible left/right combinations. if (l->Opcode() == opcode && optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { return out; From 872bcbee39941956b9819de5b81c9c02623cfee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 5 May 2023 23:43:03 +0200 Subject: [PATCH 08/25] Simplify --- src/hotspot/share/opto/addnode.cpp | 99 ++++++++++++++++-------------- src/hotspot/share/opto/addnode.hpp | 15 +---- 2 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 0537495b48a70..51ed3374d1891 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1067,15 +1067,11 @@ Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const return res; } -// Check if addition of an integer with type 't' and a constant 'c' can overflow -static bool can_overflow(const TypeInt* t, jint c) { - jint t_lo = t->_lo; - jint t_hi = t->_hi; - return ((c < 0 && (java_add(t_lo, c) > t_lo)) || - (c > 0 && (java_add(t_hi, c) < t_hi))); -} - -Node* MaxNode::constant_add_input(Node* n, jint* con) { +// Return: +// , if n is of the form x + C, where 'C' is a non-TOP constant; +// , if n is of the form x + C, where 'C' is a TOP constant; +// otherwise. +static Node* constant_add_input(Node* n, jint* con) { if (n->Opcode() == Op_AddI && n->in(2)->is_Con()) { const Type* t = n->in(2)->bottom_type(); if (t == Type::TOP) { @@ -1087,41 +1083,28 @@ Node* MaxNode::constant_add_input(Node* n, jint* con) { return n; } -Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { - assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); - Node* l = in(1); - Node* r = in(2); - jint x_off = 0; - Node* x = constant_add_input(l, &x_off); - if (x == nullptr) return nullptr; - jint y_off = 0; - Node* y = constant_add_input(r, &y_off); - if (y == nullptr) return nullptr; - Node* out; - // Transform MIN2/MAX2(x + c0, MIN2/MAX2(x + c1, z)) into MIN2/MAX2(x + MAX2/MAX2(c0, c1), z) - // if x == y and the additions can't overflow. Handle the four possible left/right combinations. - if (l->Opcode() == opcode && - optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { - return out; - } - if (r->Opcode() == opcode && - optimize_max_of_max(phase, opcode, /*left=*/false, x, y, r, x_off, y_off, &out)) { - return out; - } - // Transform MIN2/MAX2(x + c0, y + c1) into x + MIN2/MAX2(c0, c1) - // if x == y and the additions can't overflow. +// Whether addition of an integer with type 't' and a constant 'c' can overflow. +static bool can_overflow(const TypeInt* t, jint c) { + jint t_lo = t->_lo; + jint t_hi = t->_hi; + return ((c < 0 && (java_add(t_lo, c) > t_lo)) || + (c > 0 && (java_add(t_hi, c) < t_hi))); +} + +static Node* try_to_extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off, int opcode) { + assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); const TypeInt* tx = phase->type(x)->isa_int(); if (x == y && tx != nullptr && !can_overflow(tx, x_off) && !can_overflow(tx, y_off)) { - return new AddINode(x, phase->intcon(extreme(x_off, y_off, opcode))); + jint c = opcode == Op_MinI ? MIN2(x_off, y_off) : MAX2(x_off, y_off); + return new AddINode(x, phase->intcon(c)); } return nullptr; } -bool MaxNode::optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out) { +static bool optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out) { assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); - const TypeInt* tx = phase->type(x)->isa_int(); for (uint input = 1; input <= 2; input++) { Node* add = n->in(input); Node* z = n->in(input == 1 ? 2 : 1); @@ -1134,22 +1117,46 @@ bool MaxNode::optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* *out = nullptr; return true; } - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - jint c2 = extreme(x_off, y_off, opcode); - Node* add_x_c2 = phase->transform(new AddINode(x, phase->intcon(c2))); - if (opcode == Op_MinI) { - *out = new MinINode(add_x_c2, z); - } else { - *out = new MaxINode(add_x_c2, z); - } - return true; + Node* addi = try_to_extract_addition(phase, x, x_off, y, y_off, opcode); + if (addi == nullptr) { + continue; + } + if (opcode == Op_MinI) { + *out = new MinINode(phase->transform(addi), z); + } else { + *out = new MaxINode(phase->transform(addi), z); } + return true; } return false; } +Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { + assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); + Node* l = in(1); + Node* r = in(2); + jint x_off = 0; + Node* x = constant_add_input(l, &x_off); + if (x == nullptr) return nullptr; + jint y_off = 0; + Node* y = constant_add_input(r, &y_off); + if (y == nullptr) return nullptr; + Node* out; + // Transform MIN2/MAX2(x + c0, MIN2/MAX2(x + c1, z)) into MIN2/MAX2(x + MAX2/MAX2(c0, c1), z) + // if x == y and the additions can't overflow. Handle the four possible left/right combinations. + if (l->Opcode() == opcode && + optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { + return out; + } + if (r->Opcode() == opcode && + optimize_max_of_max(phase, opcode, /*left=*/false, x, y, r, x_off, y_off, &out)) { + return out; + } + // Transform MIN2/MAX2(x + c0, y + c1) into x + MIN2/MAX2(c0, c1) + // if x == y and the additions can't overflow. + return try_to_extract_addition(phase, x, x_off, y, y_off, opcode); +} + //============================================================================= //------------------------------add_ring--------------------------------------- // Supplied function returns the sum of the inputs. diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 06734008b9255..f310946ff1c1b 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -258,6 +258,7 @@ class MaxNode : public AddNode { virtual int Opcode() const = 0; virtual int max_opcode() const = 0; virtual int min_opcode() const = 0; + Node* IdealI(PhaseGVN* phase, bool can_reshape, int opcode); static Node* unsigned_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) { return build_min_max(a, b, true, true, t, gvn); @@ -285,20 +286,6 @@ class MaxNode : public AddNode { return build_min_max_diff_with_zero(a, b, false, t, gvn); } - static jint extreme(jint a, jint b, int opcode) { - assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); - return opcode == Op_MinI ? MIN2(a, b) : MAX2(a, b); - } - - // Return: - // , if n is of the form x + C, where 'C' is a non-TOP constant; - // , if n is of the form x + C, where 'C' is a TOP constant; - // otherwise. - static Node* constant_add_input(Node* n, jint* con); - - Node* IdealI(PhaseGVN* phase, bool can_reshape, int opcode); - - static bool optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out); }; //------------------------------MaxINode--------------------------------------- From d7c7987c0b68eee5155d152a1252eb41a195dbb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Mon, 8 May 2023 22:16:25 +0200 Subject: [PATCH 09/25] Refactor --- src/hotspot/share/opto/addnode.cpp | 122 +++++++++++++---------------- src/hotspot/share/opto/addnode.hpp | 1 - 2 files changed, 56 insertions(+), 67 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 51ed3374d1891..7c1176b9cf25f 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1067,6 +1067,25 @@ Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const return res; } +//============================================================================= +//------------------------------add_ring--------------------------------------- +// Supplied function returns the sum of the inputs. +const Type *MaxINode::add_ring( const Type *t0, const Type *t1 ) const { + const TypeInt *r0 = t0->is_int(); // Handy access + const TypeInt *r1 = t1->is_int(); + + // Otherwise just MAX them bits. + return TypeInt::make( MAX2(r0->_lo,r1->_lo), MAX2(r0->_hi,r1->_hi), MAX2(r0->_widen,r1->_widen) ); +} + +// Check if addition of an integer with type 't' and a constant 'c' can overflow +static bool can_overflow(const TypeInt* t, jint c) { + jint t_lo = t->_lo; + jint t_hi = t->_hi; + return ((c < 0 && (java_add(t_lo, c) > t_lo)) || + (c > 0 && (java_add(t_hi, c) < t_hi))); +} + // Return: // , if n is of the form x + C, where 'C' is a non-TOP constant; // , if n is of the form x + C, where 'C' is a TOP constant; @@ -1083,15 +1102,9 @@ static Node* constant_add_input(Node* n, jint* con) { return n; } -// Whether addition of an integer with type 't' and a constant 'c' can overflow. -static bool can_overflow(const TypeInt* t, jint c) { - jint t_lo = t->_lo; - jint t_hi = t->_hi; - return ((c < 0 && (java_add(t_lo, c) > t_lo)) || - (c > 0 && (java_add(t_hi, c) < t_hi))); -} - -static Node* try_to_extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off, int opcode) { +// Transform opcode(x + x_off, y + y_off) into x + opcode(x_off, y_off), where +// opcode is either MinI or MaxI, if x == y and the additions can't overflow. +static Node* extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off, int opcode) { assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); const TypeInt* tx = phase->type(x)->isa_int(); if (x == y && tx != nullptr && @@ -1103,69 +1116,46 @@ static Node* try_to_extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* return nullptr; } -static bool optimize_max_of_max(PhaseGVN* phase, int opcode, bool left, Node* x, Node* y, Node* n, jint x_off, jint y_off, Node** out) { - assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); - for (uint input = 1; input <= 2; input++) { - Node* add = n->in(input); - Node* z = n->in(input == 1 ? 2 : 1); - if (left) { - x = constant_add_input(add, &x_off); - } else { - y = constant_add_input(add, &y_off); - } - if (x == nullptr || y == nullptr) { - *out = nullptr; - return true; - } - Node* addi = try_to_extract_addition(phase, x, x_off, y, y_off, opcode); - if (addi == nullptr) { - continue; - } - if (opcode == Op_MinI) { - *out = new MinINode(phase->transform(addi), z); - } else { - *out = new MaxINode(phase->transform(addi), z); - } - return true; - } - return false; -} - Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); - Node* l = in(1); - Node* r = in(2); jint x_off = 0; - Node* x = constant_add_input(l, &x_off); + Node* x = constant_add_input(in(1), &x_off); if (x == nullptr) return nullptr; jint y_off = 0; - Node* y = constant_add_input(r, &y_off); + Node* y = constant_add_input(in(2), &y_off); if (y == nullptr) return nullptr; - Node* out; - // Transform MIN2/MAX2(x + c0, MIN2/MAX2(x + c1, z)) into MIN2/MAX2(x + MAX2/MAX2(c0, c1), z) - // if x == y and the additions can't overflow. Handle the four possible left/right combinations. - if (l->Opcode() == opcode && - optimize_max_of_max(phase, opcode, /*left=*/true, x, y, l, x_off, y_off, &out)) { - return out; - } - if (r->Opcode() == opcode && - optimize_max_of_max(phase, opcode, /*left=*/false, x, y, r, x_off, y_off, &out)) { - return out; - } - // Transform MIN2/MAX2(x + c0, y + c1) into x + MIN2/MAX2(c0, c1) - // if x == y and the additions can't overflow. - return try_to_extract_addition(phase, x, x_off, y, y_off, opcode); -} - -//============================================================================= -//------------------------------add_ring--------------------------------------- -// Supplied function returns the sum of the inputs. -const Type *MaxINode::add_ring( const Type *t0, const Type *t1 ) const { - const TypeInt *r0 = t0->is_int(); // Handy access - const TypeInt *r1 = t1->is_int(); - - // Otherwise just MAX them bits. - return TypeInt::make( MAX2(r0->_lo,r1->_lo), MAX2(r0->_hi,r1->_hi), MAX2(r0->_widen,r1->_widen) ); + // Try to transform opcode(x + x_off, opcode(y + y_off, z)) into opcode(x + + // opcode(x_off, y_off), z), where opcode is either MinI or MaxI, if x == y + // and the additions can't overflow. Handle + for (uint outer_input = 1; outer_input <= 2; outer_input++) { + Node* n = in(outer_input); + if (n->Opcode() != opcode) { + continue; + } + for (uint inner_input = 1; inner_input <= 2; inner_input++) { + Node* add = n->in(inner_input); + Node* z = n->in(inner_input == 1 ? 2 : 1); + if (outer_input == 1) { + x = constant_add_input(add, &x_off); + } else { + y = constant_add_input(add, &y_off); + } + if (x == nullptr || y == nullptr) { + return nullptr; + } + Node* addi = extract_addition(phase, x, x_off, y, y_off, opcode); + if (addi == nullptr) { + continue; + } + if (opcode == Op_MinI) { + return new MinINode(phase->transform(addi), z); + } else { + return new MaxINode(phase->transform(addi), z); + } + } + } + // Try to transform opcode(x + x_off, y + y_off) into x + opcode(x_off, y_off). + return extract_addition(phase, x, x_off, y, y_off, opcode); } // Ideal transformations for MaxINode diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index f310946ff1c1b..cca442d460e69 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -285,7 +285,6 @@ class MaxNode : public AddNode { static Node* min_diff_with_zero(Node* a, Node* b, const Type* t, PhaseGVN& gvn) { return build_min_max_diff_with_zero(a, b, false, t, gvn); } - }; //------------------------------MaxINode--------------------------------------- From 89cebb15334ebd016b2be46c618d88973466a231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Mon, 8 May 2023 23:08:35 +0200 Subject: [PATCH 10/25] Add some comments --- src/hotspot/share/opto/addnode.cpp | 47 ++++++++++++++++++------------ 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 7c1176b9cf25f..455fdc82eff83 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1118,39 +1118,48 @@ static Node* extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* y, jin Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); + Node* l = in(1); + Node* r = in(2); jint x_off = 0; - Node* x = constant_add_input(in(1), &x_off); + Node* x = constant_add_input(l, &x_off); if (x == nullptr) return nullptr; jint y_off = 0; - Node* y = constant_add_input(in(2), &y_off); + Node* y = constant_add_input(r, &y_off); if (y == nullptr) return nullptr; - // Try to transform opcode(x + x_off, opcode(y + y_off, z)) into opcode(x + - // opcode(x_off, y_off), z), where opcode is either MinI or MaxI, if x == y - // and the additions can't overflow. Handle + // Try to transform opcode(x + x_off, opcode(y + y_off, z)) (in any of its + // four possible permutations given by opcode's commutativity) into + // opcode(x + opcode(x_off, y_off), z), where opcode is either MinI or MaxI, + // if x == y and the additions can't overflow. for (uint outer_input = 1; outer_input <= 2; outer_input++) { - Node* n = in(outer_input); + Node* n = in(outer_input); // Inner opcode operation. if (n->Opcode() != opcode) { continue; } for (uint inner_input = 1; inner_input <= 2; inner_input++) { - Node* add = n->in(inner_input); - Node* z = n->in(inner_input == 1 ? 2 : 1); - if (outer_input == 1) { - x = constant_add_input(add, &x_off); - } else { - y = constant_add_input(add, &y_off); - } - if (x == nullptr || y == nullptr) { - return nullptr; + Node* inner_add = n->in(inner_input); // Addition within inner opcode. + Node* z = n->in(inner_input == 1 ? 2 : 1); // Opposite operand. + if (outer_input == 1) { // Update x from left inner add. + x = constant_add_input(inner_add, &x_off); + if (x == nullptr) { + return nullptr; + } + } else { // Update y from right inner add. + y = constant_add_input(inner_add, &y_off); + if (y == nullptr) { + return nullptr; + } } - Node* addi = extract_addition(phase, x, x_off, y, y_off, opcode); - if (addi == nullptr) { + // We have collected x, x_off, y, y_off, and z. Try to extract the inner + // addition if x == y and the additions can't overflow. + Node* result = extract_addition(phase, x, x_off, y, y_off, opcode); + if (result == nullptr) { continue; } + Node* transformed = phase->transform(result); if (opcode == Op_MinI) { - return new MinINode(phase->transform(addi), z); + return new MinINode(transformed, z); } else { - return new MaxINode(phase->transform(addi), z); + return new MaxINode(transformed, z); } } } From d53cb5341cd8e851990f440f9883b3d26ab353f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 10 May 2023 21:30:21 +0200 Subject: [PATCH 11/25] Complement idealization tests with negative ones --- .../irTests/MaxMinINodeIdealizationTests.java | 85 +++++++++++++++++-- 1 file changed, 79 insertions(+), 6 deletions(-) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java index fbee7e1f42e16..203feb5ed7fe9 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java @@ -1,4 +1,5 @@ /* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2022, Arm Limited. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -45,19 +46,19 @@ public static void main(String[] args) { "testMin1", "testMin2", "testMin3"}) - public void runMethod() { + public void runPositiveTests() { int a = RunInfo.getRandom().nextInt(); int min = Integer.MIN_VALUE; int max = Integer.MAX_VALUE; - assertResult(a); - assertResult(0); - assertResult(min); - assertResult(max); + assertPositiveResult(a); + assertPositiveResult(0); + assertPositiveResult(min); + assertPositiveResult(max); } @DontCompile - public void assertResult(int a) { + public void assertPositiveResult(int a) { Asserts.assertEQ(Math.max(Math.max(((a >> 1) + 150), 200), ((a >> 1) + 100)), testMax1LL(a)); Asserts.assertEQ(testMax1LL(a) , testMax1LR(a)); Asserts.assertEQ(testMax1LL(a) , testMax1RL(a)); @@ -156,4 +157,76 @@ public int testMax3(int i) { public int testMin3(int i) { return Math.min(i, i); } + + @Run(test = {"testTwoLevelsDifferentXY", + "testTwoLevelsNoLeftConstant", + "testTwoLevelsNoRightConstant", + "testDifferentXY", + "testNoLeftConstant", + "testNoRightConstant"}) + public void runNegativeTests() { + int a = RunInfo.getRandom().nextInt(); + int min = Integer.MIN_VALUE; + int max = Integer.MAX_VALUE; + + assertNegativeResult(a); + assertNegativeResult(0); + assertNegativeResult(min); + assertNegativeResult(max); + + testTwoLevelsDifferentXY(10); + testTwoLevelsNoLeftConstant(10, 42); + testTwoLevelsNoRightConstant(10, 42); + testDifferentXY(10); + testNoLeftConstant(10, 42); + testNoRightConstant(10, 42); + } + + @DontCompile + public void assertNegativeResult(int a) { + Asserts.assertEQ(Math.max(Math.max(((a >> 1) + 150), 200), ((a >> 2) + 100)), testTwoLevelsDifferentXY(a)); + Asserts.assertEQ(Math.max(Math.max(((a >> 1) + a*2), 200), ((a >> 1) + 100)), testTwoLevelsNoLeftConstant(a, a*2)); + Asserts.assertEQ(Math.max(Math.max(((a >> 1) + 150), 200), ((a >> 1) + a*2)), testTwoLevelsNoRightConstant(a, a*2)); + Asserts.assertEQ(Math.max((a >> 1) + 10, (a >> 2) + 11), testDifferentXY(a)); + Asserts.assertEQ(Math.max((a >> 1) + a*2, (a >> 1) + 11), testNoLeftConstant(a, a*2)); + Asserts.assertEQ(Math.max((a >> 1) + 10, (a >> 1) + a*2), testNoRightConstant(a, a*2)); + } + + @Test + @IR(counts = {IRNode.MAX_I, "2"}) + public int testTwoLevelsDifferentXY(int i) { + return Math.max(Math.max(((i >> 1) + 150), 200), ((i >> 2) + 100)); + } + + @Test + @IR(counts = {IRNode.MAX_I, "2"}) + public int testTwoLevelsNoLeftConstant(int i, int c0) { + return Math.max(Math.max(((i >> 1) + c0), 200), ((i >> 1) + 100)); + } + + @Test + @IR(counts = {IRNode.MAX_I, "2"}) + public int testTwoLevelsNoRightConstant(int i, int c1) { + return Math.max(Math.max(((i >> 1) + 150), 200), ((i >> 1) + c1)); + } + + @Test + @IR(counts = {IRNode.MAX_I, "1"}) + public int testDifferentXY(int i) { + return Math.max((i >> 1) + 10, (i >> 2) + 11); + } + + @Test + @IR(counts = {IRNode.MAX_I, "1"}) + public int testNoLeftConstant(int i, int c0) { + return Math.max((i >> 1) + c0, (i >> 1) + 11); + } + + @Test + @IR(counts = {IRNode.MAX_I, "1"}) + public int testNoRightConstant(int i, int c1) { + return Math.max((i >> 1) + 10, (i >> 1) + c1); + } + + } From 3c120957ea70c94be6e0fa4caeb023246d0599db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 10 May 2023 22:19:03 +0200 Subject: [PATCH 12/25] Uncomment tests --- .../jtreg/compiler/loopopts/superword/MinMaxRed_Int.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java b/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java index a1a3f413fd956..943c67dcfb618 100644 --- a/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java +++ b/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java @@ -59,7 +59,6 @@ public void runMaxTest() { } } - /* @Run(test = {"minReductionImplement"}, mode = RunMode.STANDALONE) public void runMinTest() { @@ -76,14 +75,14 @@ public void runMinTest() { throw new AssertionError("Failed"); } } - */ + public static void ReductionInit(int[] a, int[] b) { for (int i = 0; i < a.length; i++) { a[i] = -i; b[i] = i; } } - /* + @Test @IR(applyIf = {"SuperWordReductions", "true"}, applyIfCPUFeatureOr = { "sse4.1", "true" , "asimd" , "true"}, @@ -94,7 +93,7 @@ public static int minReductionImplement(int[] a, int[] b, int res) { } return res; } - */ + @Test @IR(applyIf = {"SuperWordReductions", "true"}, applyIfCPUFeatureOr = { "sse4.1", "true" , "asimd" , "true"}, From 83e7630ea287fd03d46454e1c8292a6fe6f7c863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 10 May 2023 22:29:44 +0200 Subject: [PATCH 13/25] Update copyright header --- src/hotspot/share/opto/addnode.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index cca442d460e69..29e0b620d4e0f 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it From 9fd482b5e98dea03e0a1e4e6b65970edf04595ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 10 May 2023 22:38:50 +0200 Subject: [PATCH 14/25] Refine comments --- src/hotspot/share/opto/addnode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 455fdc82eff83..c86cb4ee3ae55 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1131,12 +1131,12 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { // opcode(x + opcode(x_off, y_off), z), where opcode is either MinI or MaxI, // if x == y and the additions can't overflow. for (uint outer_input = 1; outer_input <= 2; outer_input++) { - Node* n = in(outer_input); // Inner opcode operation. + Node* n = in(outer_input); // Inner opcode operation (opcode(y + y_off, z)). if (n->Opcode() != opcode) { continue; } for (uint inner_input = 1; inner_input <= 2; inner_input++) { - Node* inner_add = n->in(inner_input); // Addition within inner opcode. + Node* inner_add = n->in(inner_input); // Addition within inner opcode (y + y_off). Node* z = n->in(inner_input == 1 ? 2 : 1); // Opposite operand. if (outer_input == 1) { // Update x from left inner add. x = constant_add_input(inner_add, &x_off); From c736c0a008c1514883ae476ef5e9fc0f8fcc314d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Tue, 23 May 2023 22:24:07 +0200 Subject: [PATCH 15/25] Randomize array values in min/max test computation --- .../compiler/loopopts/superword/MinMaxRed_Int.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java b/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java index 943c67dcfb618..9b17fa710dedd 100644 --- a/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java +++ b/test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java @@ -32,8 +32,14 @@ package compiler.loopopts.superword; import compiler.lib.ir_framework.*; +import java.util.Arrays; +import java.util.Random; +import jdk.test.lib.Utils; public class MinMaxRed_Int { + + private static final Random random = Utils.getRandomInstance(); + public static void main(String[] args) throws Exception { TestFramework framework = new TestFramework(); framework.addFlags("-XX:+IgnoreUnrecognizedVMOptions", @@ -52,7 +58,7 @@ public void runMaxTest() { for (int j = 0; j < 2000; j++) { res = maxReductionImplement(a, b, res); } - if (res == 0) { + if (res == Arrays.stream(a).max().getAsInt()) { System.out.println("Success"); } else { throw new AssertionError("Failed"); @@ -69,7 +75,7 @@ public void runMinTest() { for (int j = 0; j < 2000; j++) { res = minReductionImplement(a, b, res); } - if (res == -1023*1023) { + if (res == Arrays.stream(a).min().getAsInt()) { System.out.println("Success"); } else { throw new AssertionError("Failed"); @@ -78,8 +84,8 @@ public void runMinTest() { public static void ReductionInit(int[] a, int[] b) { for (int i = 0; i < a.length; i++) { - a[i] = -i; - b[i] = i; + a[i] = random.nextInt(); + b[i] = 1; } } From 8f8faf271464ef2ca96fef32c6b51b834dfc8f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Sun, 28 May 2023 15:39:45 +0200 Subject: [PATCH 16/25] Make auxiliary add operand extraction function return a tuple --- src/hotspot/share/opto/addnode.cpp | 63 ++++++++++++++++++------------ 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index c86cb4ee3ae55..f414088f5eacb 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -33,6 +33,7 @@ #include "opto/mulnode.hpp" #include "opto/phaseX.hpp" #include "opto/subnode.hpp" +#include "utilities/pair.hpp" // Portions of code courtesy of Clifford Click @@ -1086,20 +1087,25 @@ static bool can_overflow(const TypeInt* t, jint c) { (c > 0 && (java_add(t_hi, c) < t_hi))); } -// Return: -// , if n is of the form x + C, where 'C' is a non-TOP constant; -// , if n is of the form x + C, where 'C' is a TOP constant; -// otherwise. -static Node* constant_add_input(Node* n, jint* con) { - if (n->Opcode() == Op_AddI && n->in(2)->is_Con()) { - const Type* t = n->in(2)->bottom_type(); - if (t == Type::TOP) { - return nullptr; - } - *con = t->is_int()->get_con(); - n = n->in(1); +typedef const Pair ConstAddOperands; + +// If n is an addition of a variable x with a constant C, return either +// if C is non-TOP or if C is TOP. Otherwise, return . +static ConstAddOperands as_add_with_constant(Node* n) { + if (n->Opcode() != Op_AddI) { + return ConstAddOperands(n, 0); + } + Node* x = n->in(1); + Node* c = n->in(2); + if (!c->is_Con()) { + return ConstAddOperands(n, 0); } - return n; + const Type* c_type = c->bottom_type(); + if (c_type == Type::TOP) { + // If C is TOP, return nullptr to stop idealization. + return ConstAddOperands(nullptr, 0); + } + return ConstAddOperands(x, c_type->is_int()->get_con()); } // Transform opcode(x + x_off, y + y_off) into x + opcode(x_off, y_off), where @@ -1118,14 +1124,19 @@ static Node* extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* y, jin Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); + Node* l = in(1); - Node* r = in(2); - jint x_off = 0; - Node* x = constant_add_input(l, &x_off); + ConstAddOperands xC = as_add_with_constant(l); + Node* x = xC.first; if (x == nullptr) return nullptr; - jint y_off = 0; - Node* y = constant_add_input(r, &y_off); + jint x_off = xC.second; + + Node* r = in(2); + ConstAddOperands yC = as_add_with_constant(r); + Node* y = yC.first; if (y == nullptr) return nullptr; + jint y_off = yC.second; + // Try to transform opcode(x + x_off, opcode(y + y_off, z)) (in any of its // four possible permutations given by opcode's commutativity) into // opcode(x + opcode(x_off, y_off), z), where opcode is either MinI or MaxI, @@ -1138,15 +1149,19 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { for (uint inner_input = 1; inner_input <= 2; inner_input++) { Node* inner_add = n->in(inner_input); // Addition within inner opcode (y + y_off). Node* z = n->in(inner_input == 1 ? 2 : 1); // Opposite operand. + ConstAddOperands inner_xyC = as_add_with_constant(inner_add); + if (inner_xyC.first == nullptr) { + return nullptr; + } if (outer_input == 1) { // Update x from left inner add. - x = constant_add_input(inner_add, &x_off); - if (x == nullptr) { - return nullptr; + x = inner_xyC.first; + if (x != inner_add) { + x_off = inner_xyC.second; } } else { // Update y from right inner add. - y = constant_add_input(inner_add, &y_off); - if (y == nullptr) { - return nullptr; + y = inner_xyC.first; + if (y != inner_add) { + y_off = inner_xyC.second; } } // We have collected x, x_off, y, y_off, and z. Try to extract the inner From 3aba9bcd34685c2eeb8ce721343dfd44067558d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Sun, 28 May 2023 21:07:19 +0200 Subject: [PATCH 17/25] Refactor idealization and extracted Identity transformation for clarity --- src/hotspot/share/opto/addnode.cpp | 181 ++++++++++++++++------------- src/hotspot/share/opto/addnode.hpp | 6 +- 2 files changed, 105 insertions(+), 82 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index f414088f5eacb..97bc0b8d7882f 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1068,18 +1068,7 @@ Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const return res; } -//============================================================================= -//------------------------------add_ring--------------------------------------- -// Supplied function returns the sum of the inputs. -const Type *MaxINode::add_ring( const Type *t0, const Type *t1 ) const { - const TypeInt *r0 = t0->is_int(); // Handy access - const TypeInt *r1 = t1->is_int(); - - // Otherwise just MAX them bits. - return TypeInt::make( MAX2(r0->_lo,r1->_lo), MAX2(r0->_hi,r1->_hi), MAX2(r0->_widen,r1->_widen) ); -} - -// Check if addition of an integer with type 't' and a constant 'c' can overflow +// Check if addition of an integer with type 't' and a constant 'c' can overflow. static bool can_overflow(const TypeInt* t, jint c) { jint t_lo = t->_lo; jint t_hi = t->_hi; @@ -1087,104 +1076,130 @@ static bool can_overflow(const TypeInt* t, jint c) { (c > 0 && (java_add(t_hi, c) < t_hi))); } +// If x == y and neither add(x, x_off) nor add(y, y_off) overflow, return +// add(x, op(x_off, y_off)). Otherwise, return nullptr. +Node* MaxNode::extract_add(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off) { + int opcode = Opcode(); + assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); + const TypeInt* tx = phase->type(x)->isa_int(); + if (x == y && tx != nullptr && + !can_overflow(tx, x_off) && + !can_overflow(tx, y_off)) { + jint c = opcode == Op_MinI ? MIN2(x_off, y_off) : MAX2(x_off, y_off); + return new AddINode(x, phase->intcon(c)); + } + return nullptr; +} + typedef const Pair ConstAddOperands; -// If n is an addition of a variable x with a constant C, return either -// if C is non-TOP or if C is TOP. Otherwise, return . +// If n is an integer addition of x with a non-TOP constant C, return . +// Otherwise, return . static ConstAddOperands as_add_with_constant(Node* n) { + ConstAddOperands null(nullptr, 0); if (n->Opcode() != Op_AddI) { - return ConstAddOperands(n, 0); + return null; } Node* x = n->in(1); Node* c = n->in(2); if (!c->is_Con()) { - return ConstAddOperands(n, 0); + return null; } const Type* c_type = c->bottom_type(); if (c_type == Type::TOP) { - // If C is TOP, return nullptr to stop idealization. - return ConstAddOperands(nullptr, 0); + return null; } return ConstAddOperands(x, c_type->is_int()->get_con()); } -// Transform opcode(x + x_off, y + y_off) into x + opcode(x_off, y_off), where -// opcode is either MinI or MaxI, if x == y and the additions can't overflow. -static Node* extract_addition(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off, int opcode) { - assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); - const TypeInt* tx = phase->type(x)->isa_int(); - if (x == y && tx != nullptr && - !can_overflow(tx, x_off) && - !can_overflow(tx, y_off)) { - jint c = opcode == Op_MinI ? MIN2(x_off, y_off) : MAX2(x_off, y_off); - return new AddINode(x, phase->intcon(c)); - } - return nullptr; -} - -Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape, int opcode) { +Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { + int opcode = Opcode(); assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); - - Node* l = in(1); - ConstAddOperands xC = as_add_with_constant(l); - Node* x = xC.first; - if (x == nullptr) return nullptr; - jint x_off = xC.second; - - Node* r = in(2); - ConstAddOperands yC = as_add_with_constant(r); - Node* y = yC.first; - if (y == nullptr) return nullptr; - jint y_off = yC.second; - - // Try to transform opcode(x + x_off, opcode(y + y_off, z)) (in any of its - // four possible permutations given by opcode's commutativity) into - // opcode(x + opcode(x_off, y_off), z), where opcode is either MinI or MaxI, - // if x == y and the additions can't overflow. - for (uint outer_input = 1; outer_input <= 2; outer_input++) { - Node* n = in(outer_input); // Inner opcode operation (opcode(y + y_off, z)). - if (n->Opcode() != opcode) { + // Try to transform the following pattern, in any of its four possible + // permutations induced by op's commutativity: + // op(op(add(inner, inner_off), inner_other), add(outer, outer_off)) + // into + // op(add(inner, op(inner_off, outer_off)), inner_other), + // where: + // op is either MinI or MaxI, and + // inner == outer, and + // the additions cannot overflow. + for (uint inner_op_index = 1; inner_op_index <= 2; inner_op_index++) { + if (in(inner_op_index)->Opcode() != opcode) { continue; } - for (uint inner_input = 1; inner_input <= 2; inner_input++) { - Node* inner_add = n->in(inner_input); // Addition within inner opcode (y + y_off). - Node* z = n->in(inner_input == 1 ? 2 : 1); // Opposite operand. - ConstAddOperands inner_xyC = as_add_with_constant(inner_add); - if (inner_xyC.first == nullptr) { - return nullptr; - } - if (outer_input == 1) { // Update x from left inner add. - x = inner_xyC.first; - if (x != inner_add) { - x_off = inner_xyC.second; - } - } else { // Update y from right inner add. - y = inner_xyC.first; - if (y != inner_add) { - y_off = inner_xyC.second; - } + Node* outer_add = in(inner_op_index == 1 ? 2 : 1); + ConstAddOperands outer_add_operands = as_add_with_constant(outer_add); + if (outer_add_operands.first == nullptr) { + continue; + } + Node* outer = outer_add_operands.first; + jint outer_off = outer_add_operands.second; + // One operand is a MinI/MaxI and the other is an integer addition with + // constant. Test the operands of the inner MinI/MaxI. + for (uint inner_add_index = 1; inner_add_index <= 2; inner_add_index++) { + Node* inner_op = in(inner_op_index); + Node* inner_add = inner_op->in(inner_add_index); + ConstAddOperands inner_add_operands = as_add_with_constant(inner_add); + if (inner_add_operands.first == nullptr) { + continue; } - // We have collected x, x_off, y, y_off, and z. Try to extract the inner - // addition if x == y and the additions can't overflow. - Node* result = extract_addition(phase, x, x_off, y, y_off, opcode); - if (result == nullptr) { + Node* inner = inner_add_operands.first; + jint inner_off = inner_add_operands.second; + // Try to extract the inner add. + Node* add_extracted = extract_add(phase, inner, inner_off, outer, outer_off); + if (add_extracted == nullptr) { continue; } - Node* transformed = phase->transform(result); + Node* add_transformed = phase->transform(add_extracted); + Node* inner_other = inner_op->in(inner_add_index == 1 ? 2 : 1); if (opcode == Op_MinI) { - return new MinINode(transformed, z); + return new MinINode(add_transformed, inner_other); } else { - return new MaxINode(transformed, z); + return new MaxINode(add_transformed, inner_other); } } } - // Try to transform opcode(x + x_off, y + y_off) into x + opcode(x_off, y_off). - return extract_addition(phase, x, x_off, y, y_off, opcode); + // Try to transform + // op(add(x, x_off), add(y, y_off)) + // into + // add(x, op(x_off, y_off)), + // where: + // op is either MinI or MaxI, and + // inner == outer, and + // the additions cannot overflow. + ConstAddOperands xC = as_add_with_constant(in(1)); + ConstAddOperands yC = as_add_with_constant(in(2)); + if (xC.first == nullptr || yC.first == nullptr) return nullptr; + return extract_add(phase, xC.first, xC.second, yC.first, yC.second); +} + +Node* MaxNode::IdentityI(PhaseGVN* phase) { + assert(Opcode() == Op_MinI || Opcode() == Op_MaxI, "Unexpected opcode"); + if (in(1) == in(2)) { + return in(1); + } + return MaxNode::Identity(phase); } // Ideal transformations for MaxINode Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { - return IdealI(phase, can_reshape, Op_MaxI); + return IdealI(phase, can_reshape); +} + +Node* MaxINode::Identity(PhaseGVN* phase) { + return IdentityI(phase); +} + +//============================================================================= +//------------------------------add_ring--------------------------------------- +// Supplied function returns the sum of the inputs. +const Type *MaxINode::add_ring( const Type *t0, const Type *t1 ) const { + const TypeInt *r0 = t0->is_int(); // Handy access + const TypeInt *r1 = t1->is_int(); + + // Otherwise just MAX them bits. + return TypeInt::make( MAX2(r0->_lo,r1->_lo), MAX2(r0->_hi,r1->_hi), MAX2(r0->_widen,r1->_widen) ); } //============================================================================= @@ -1192,7 +1207,11 @@ Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { // MINs show up in range-check loop limit calculations. Look for // "MIN2(x+c0,MIN2(y,x+c1))". Pick the smaller constant: "MIN2(x+c0,y)" Node* MinINode::Ideal(PhaseGVN* phase, bool can_reshape) { - return IdealI(phase, can_reshape, Op_MinI); + return IdealI(phase, can_reshape); +} + +Node* MinINode::Identity(PhaseGVN* phase) { + return IdentityI(phase); } //------------------------------add_ring--------------------------------------- diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 29e0b620d4e0f..8212210978b2e 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -252,13 +252,15 @@ class MaxNode : public AddNode { private: static Node* build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn); static Node* build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn); + Node* extract_add(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off); public: MaxNode( Node *in1, Node *in2 ) : AddNode(in1,in2) {} virtual int Opcode() const = 0; virtual int max_opcode() const = 0; virtual int min_opcode() const = 0; - Node* IdealI(PhaseGVN* phase, bool can_reshape, int opcode); + Node* IdealI(PhaseGVN* phase, bool can_reshape); + Node* IdentityI(PhaseGVN* phase); static Node* unsigned_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) { return build_min_max(a, b, true, true, t, gvn); @@ -301,6 +303,7 @@ class MaxINode : public MaxNode { int max_opcode() const { return Op_MaxI; } int min_opcode() const { return Op_MinI; } virtual Node* Ideal(PhaseGVN* phase, bool can_reshape); + virtual Node* Identity(PhaseGVN* phase); }; //------------------------------MinINode--------------------------------------- @@ -317,6 +320,7 @@ class MinINode : public MaxNode { int max_opcode() const { return Op_MaxI; } int min_opcode() const { return Op_MinI; } virtual Node *Ideal(PhaseGVN *phase, bool can_reshape); + virtual Node* Identity(PhaseGVN* phase); }; //------------------------------MaxLNode--------------------------------------- From c3ea6f75acdc831472fd764ed7fedb9b9e38d00e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Tue, 30 May 2023 16:29:51 +0200 Subject: [PATCH 18/25] Defer op(x, x) to constant/identity propagation early --- src/hotspot/share/opto/addnode.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 97bc0b8d7882f..6bf6358fcf25e 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1115,6 +1115,10 @@ static ConstAddOperands as_add_with_constant(Node* n) { Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { int opcode = Opcode(); assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); + // Defer handling of op(x, x) to constant/identity propagation. + if (in(1) == in(2)) { + return nullptr; + } // Try to transform the following pattern, in any of its four possible // permutations induced by op's commutativity: // op(op(add(inner, inner_off), inner_other), add(outer, outer_off)) From 29922eabc0c5c125fed883ac86892a247b8bcce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 31 May 2023 18:39:55 +0200 Subject: [PATCH 19/25] Extract MinI/MaxI construction; pass around ConstAddOperands instead of individual components --- src/hotspot/share/opto/addnode.cpp | 41 ++++++++++++++---------------- src/hotspot/share/opto/addnode.hpp | 5 +++- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 9af97d4bd31cf..7c6d7f1de657d 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -33,7 +33,6 @@ #include "opto/mulnode.hpp" #include "opto/phaseX.hpp" #include "opto/subnode.hpp" -#include "utilities/pair.hpp" // Portions of code courtesy of Clifford Click @@ -1031,6 +1030,14 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { return AddNode::Value(phase); } +Node* MaxNode::build_min_max_int(Node* a, Node* b, bool is_max) { + if (is_max) { + return new MaxINode(a, b); + } else { + return new MinINode(a, b); + } +} + Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn) { bool is_int = gvn.type(a)->isa_int(); assert(is_int || gvn.type(a)->isa_long(), "int or long inputs"); @@ -1045,13 +1052,8 @@ Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, co } Node* res = nullptr; if (is_int && !is_unsigned) { - if (is_max) { - res = gvn.transform(new MaxINode(a, b)); - assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); - } else { - Node* res = gvn.transform(new MinINode(a, b)); - assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); - } + res = gvn.transform(build_min_max_int(a, b, is_max)); + assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); } else { Node* cmp = nullptr; if (is_max) { @@ -1104,12 +1106,17 @@ static bool can_overflow(const TypeInt* t, jint c) { (c > 0 && (java_add(t_hi, c) < t_hi))); } +// Let = x_operands and = y_operands. // If x == y and neither add(x, x_off) nor add(y, y_off) overflow, return // add(x, op(x_off, y_off)). Otherwise, return nullptr. -Node* MaxNode::extract_add(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off) { +Node* MaxNode::extract_add(PhaseGVN* phase, ConstAddOperands x_operands, ConstAddOperands y_operands) { + Node* x = x_operands.first; + Node* y = y_operands.first; int opcode = Opcode(); assert(opcode == Op_MaxI || opcode == Op_MinI, "Unexpected opcode"); const TypeInt* tx = phase->type(x)->isa_int(); + jint x_off = x_operands.second; + jint y_off = y_operands.second; if (x == y && tx != nullptr && !can_overflow(tx, x_off) && !can_overflow(tx, y_off)) { @@ -1119,8 +1126,6 @@ Node* MaxNode::extract_add(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y return nullptr; } -typedef const Pair ConstAddOperands; - // If n is an integer addition of x with a non-TOP constant C, return . // Otherwise, return . static ConstAddOperands as_add_with_constant(Node* n) { @@ -1165,8 +1170,6 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { if (outer_add_operands.first == nullptr) { continue; } - Node* outer = outer_add_operands.first; - jint outer_off = outer_add_operands.second; // One operand is a MinI/MaxI and the other is an integer addition with // constant. Test the operands of the inner MinI/MaxI. for (uint inner_add_index = 1; inner_add_index <= 2; inner_add_index++) { @@ -1176,20 +1179,14 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { if (inner_add_operands.first == nullptr) { continue; } - Node* inner = inner_add_operands.first; - jint inner_off = inner_add_operands.second; // Try to extract the inner add. - Node* add_extracted = extract_add(phase, inner, inner_off, outer, outer_off); + Node* add_extracted = extract_add(phase, inner_add_operands, outer_add_operands); if (add_extracted == nullptr) { continue; } Node* add_transformed = phase->transform(add_extracted); Node* inner_other = inner_op->in(inner_add_index == 1 ? 2 : 1); - if (opcode == Op_MinI) { - return new MinINode(add_transformed, inner_other); - } else { - return new MaxINode(add_transformed, inner_other); - } + return build_min_max_int(add_transformed, inner_other, opcode == Op_MaxI); } } // Try to transform @@ -1203,7 +1200,7 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { ConstAddOperands xC = as_add_with_constant(in(1)); ConstAddOperands yC = as_add_with_constant(in(2)); if (xC.first == nullptr || yC.first == nullptr) return nullptr; - return extract_add(phase, xC.first, xC.second, yC.first, yC.second); + return extract_add(phase, xC, yC); } Node* MaxNode::IdentityI(PhaseGVN* phase) { diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 15700b920fe24..7f8cd43cd1043 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -28,10 +28,12 @@ #include "opto/node.hpp" #include "opto/opcodes.hpp" #include "opto/type.hpp" +#include "utilities/pair.hpp" // Portions of code courtesy of Clifford Click class PhaseTransform; +typedef const Pair ConstAddOperands; //------------------------------AddNode---------------------------------------- // Classic Add functionality. This covers all the usual 'add' behaviors for @@ -250,9 +252,10 @@ class XorLNode : public AddNode { // 2 equal inputs to be equal. class MaxNode : public AddNode { private: + static Node* build_min_max_int(Node* a, Node* b, bool is_max); static Node* build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn); static Node* build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn); - Node* extract_add(PhaseGVN* phase, Node* x, jint x_off, Node* y, jint y_off); + Node* extract_add(PhaseGVN* phase, ConstAddOperands x_operands, ConstAddOperands y_operands); public: MaxNode( Node *in1, Node *in2 ) : AddNode(in1,in2) {} From 3adee22c2479f1c0169915397b4c086a33559bc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 31 May 2023 19:22:44 +0200 Subject: [PATCH 20/25] Add tests to exercise the case without inner additions --- .../irTests/MaxMinINodeIdealizationTests.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java index 203feb5ed7fe9..a215a4f78b0db 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java @@ -41,7 +41,9 @@ public static void main(String[] args) { } @Run(test = {"testMax1LL", "testMax1LR", "testMax1RL", "testMax1RR", + "testMax1LLNoInnerAdd", "testMax1LLNoInnerAdd2", "testMax2L", "testMax2R", + "testMax2LNoLeftAdd", "testMax3", "testMin1", "testMin2", @@ -63,8 +65,11 @@ public void assertPositiveResult(int a) { Asserts.assertEQ(testMax1LL(a) , testMax1LR(a)); Asserts.assertEQ(testMax1LL(a) , testMax1RL(a)); Asserts.assertEQ(testMax1LL(a) , testMax1RR(a)); + Asserts.assertEQ(Math.max(Math.max((a >> 1), 200), (a >> 1) + 100) , testMax1LLNoInnerAdd(a)); + Asserts.assertEQ(Math.max(Math.max((a >> 1), (a << 1)), (a >> 1) + 100) , testMax1LLNoInnerAdd2(a)); Asserts.assertEQ(Math.max(((a >> 1) + 10), ((a >> 1) + 11)) , testMax2L(a)); 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(Math.min(((a >> 1) + 100), Math.min(((a >> 1) + 150), 200)), testMin1(a)); @@ -110,6 +115,22 @@ public int testMax1RR(int i) { return Math.max(((i >> 1) + 100), Math.max(200, ((i >> 1) + 150))); } + @Test + @IR(counts = {IRNode.MAX_I, "1", + IRNode.ADD , "1", + }) + public int testMax1LLNoInnerAdd(int i) { + return Math.max(Math.max((i >> 1), 200), (i >> 1) + 100); + } + + @Test + @IR(counts = {IRNode.MAX_I, "1", + IRNode.ADD , "1", + }) + public int testMax1LLNoInnerAdd2(int i) { + return Math.max(Math.max((i >> 1), (i << 1)), (i >> 1) + 100); + } + // Similarly, transform min(x + c0, min(y + c1, z)) to min(add(x, c2), z) if x == y, where c2 = MIN2(c0, c1). @Test @IR(counts = {IRNode.MIN_I, "1", @@ -136,6 +157,13 @@ public int testMax2R(int i) { return Math.max((i >> 1) + 11, (i >> 1) + 10); } + @Test + @IR(failOn = {IRNode.MAX_I}) + @IR(counts = {IRNode.ADD, "1"}) + public int testMax2LNoLeftAdd(int i) { + return Math.max(i >> 1, (i >> 1) + 11); + } + // Similarly, transform min(x + c0, y + c1) to add(x, c2) if x == y, where c2 = MIN2(c0, c1). @Test @IR(failOn = {IRNode.MIN_I}) From efb8ac0e7d9887f77a2c994093a50dd5c4e2f253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Wed, 31 May 2023 20:31:43 +0200 Subject: [PATCH 21/25] Handle case without inner additions by restoring 'as_add_with_constant' to its previous state --- src/hotspot/share/opto/addnode.cpp | 33 +++++++----------------------- src/hotspot/share/opto/addnode.hpp | 3 --- 2 files changed, 7 insertions(+), 29 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 7c6d7f1de657d..e8cc7d6988bb3 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1126,21 +1126,22 @@ Node* MaxNode::extract_add(PhaseGVN* phase, ConstAddOperands x_operands, ConstAd return nullptr; } -// If n is an integer addition of x with a non-TOP constant C, return . -// Otherwise, return . +// Try to cast n as an integer addition with a constant. Return: +// , if n == add(x, C), where 'C' is a non-TOP constant; +// , if n == add(x, C), where 'C' is a TOP constant; or +// , otherwise. static ConstAddOperands as_add_with_constant(Node* n) { - ConstAddOperands null(nullptr, 0); if (n->Opcode() != Op_AddI) { - return null; + return ConstAddOperands(n, 0); } Node* x = n->in(1); Node* c = n->in(2); if (!c->is_Con()) { - return null; + return ConstAddOperands(n, 0); } const Type* c_type = c->bottom_type(); if (c_type == Type::TOP) { - return null; + return ConstAddOperands(nullptr, 0); } return ConstAddOperands(x, c_type->is_int()->get_con()); } @@ -1148,10 +1149,6 @@ static ConstAddOperands as_add_with_constant(Node* n) { Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { int opcode = Opcode(); assert(opcode == Op_MinI || opcode == Op_MaxI, "Unexpected opcode"); - // Defer handling of op(x, x) to constant/identity propagation. - if (in(1) == in(2)) { - return nullptr; - } // Try to transform the following pattern, in any of its four possible // permutations induced by op's commutativity: // op(op(add(inner, inner_off), inner_other), add(outer, outer_off)) @@ -1203,23 +1200,11 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { return extract_add(phase, xC, yC); } -Node* MaxNode::IdentityI(PhaseGVN* phase) { - assert(Opcode() == Op_MinI || Opcode() == Op_MaxI, "Unexpected opcode"); - if (in(1) == in(2)) { - return in(1); - } - return MaxNode::Identity(phase); -} - // Ideal transformations for MaxINode Node* MaxINode::Ideal(PhaseGVN* phase, bool can_reshape) { return IdealI(phase, can_reshape); } -Node* MaxINode::Identity(PhaseGVN* phase) { - return IdentityI(phase); -} - //============================================================================= //------------------------------add_ring--------------------------------------- // Supplied function returns the sum of the inputs. @@ -1239,10 +1224,6 @@ Node* MinINode::Ideal(PhaseGVN* phase, bool can_reshape) { return IdealI(phase, can_reshape); } -Node* MinINode::Identity(PhaseGVN* phase) { - return IdentityI(phase); -} - //------------------------------add_ring--------------------------------------- // Supplied function returns the sum of the inputs. const Type *MinINode::add_ring( const Type *t0, const Type *t1 ) const { diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 7f8cd43cd1043..6fd328b2b730a 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -263,7 +263,6 @@ class MaxNode : public AddNode { virtual int max_opcode() const = 0; virtual int min_opcode() const = 0; Node* IdealI(PhaseGVN* phase, bool can_reshape); - Node* IdentityI(PhaseGVN* phase); static Node* unsigned_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) { return build_min_max(a, b, true, true, t, gvn); @@ -306,7 +305,6 @@ class MaxINode : public MaxNode { int max_opcode() const { return Op_MaxI; } int min_opcode() const { return Op_MinI; } virtual Node* Ideal(PhaseGVN* phase, bool can_reshape); - virtual Node* Identity(PhaseGVN* phase); }; //------------------------------MinINode--------------------------------------- @@ -323,7 +321,6 @@ class MinINode : public MaxNode { int max_opcode() const { return Op_MaxI; } int min_opcode() const { return Op_MinI; } virtual Node *Ideal(PhaseGVN *phase, bool can_reshape); - virtual Node* Identity(PhaseGVN* phase); }; //------------------------------MaxLNode--------------------------------------- From 3b4d79931b8fd9cb37217272d68ac17e449c24db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Thu, 1 Jun 2023 09:01:03 +0200 Subject: [PATCH 22/25] Complete test battery with remaining no-add cases --- .../irTests/MaxMinINodeIdealizationTests.java | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java index a215a4f78b0db..e0ef87f95a908 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/MaxMinINodeIdealizationTests.java @@ -41,7 +41,7 @@ public static void main(String[] args) { } @Run(test = {"testMax1LL", "testMax1LR", "testMax1RL", "testMax1RR", - "testMax1LLNoInnerAdd", "testMax1LLNoInnerAdd2", + "testMax1LLNoInnerAdd", "testMax1LLNoInnerAdd2", "testMax1LLNoOuterAdd", "testMax1LLNoAdd", "testMax2L", "testMax2R", "testMax2LNoLeftAdd", "testMax3", @@ -67,6 +67,8 @@ public void assertPositiveResult(int a) { Asserts.assertEQ(testMax1LL(a) , testMax1RR(a)); Asserts.assertEQ(Math.max(Math.max((a >> 1), 200), (a >> 1) + 100) , testMax1LLNoInnerAdd(a)); Asserts.assertEQ(Math.max(Math.max((a >> 1), (a << 1)), (a >> 1) + 100) , testMax1LLNoInnerAdd2(a)); + Asserts.assertEQ(Math.max(Math.max(((a >> 1) + 150), 200), a >> 1) , testMax1LLNoOuterAdd(a)); + Asserts.assertEQ(Math.max(Math.max((a >> 1), 200), a >> 1) , testMax1LLNoAdd(a)); Asserts.assertEQ(Math.max(((a >> 1) + 10), ((a >> 1) + 11)) , testMax2L(a)); Asserts.assertEQ(testMax2L(a) , testMax2R(a)); Asserts.assertEQ(Math.max(a >> 1, ((a >> 1) + 11)) , testMax2LNoLeftAdd(a)); @@ -131,6 +133,21 @@ public int testMax1LLNoInnerAdd2(int i) { return Math.max(Math.max((i >> 1), (i << 1)), (i >> 1) + 100); } + @Test + @IR(counts = {IRNode.MAX_I, "1", + IRNode.ADD , "1", + }) + public int testMax1LLNoOuterAdd(int i) { + return Math.max(Math.max(((i >> 1) + 150), 200), i >> 1); + } + + @Test + @IR(failOn = {IRNode.ADD}) + @IR(counts = {IRNode.MAX_I, "1"}) + public int testMax1LLNoAdd(int i) { + return Math.max(Math.max((i >> 1), 200), i >> 1); + } + // Similarly, transform min(x + c0, min(y + c1, z)) to min(add(x, c2), z) if x == y, where c2 = MIN2(c0, c1). @Test @IR(counts = {IRNode.MIN_I, "1", From c26f585350f6b1458047e854e6b5a52d927177b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Thu, 1 Jun 2023 14:44:49 +0200 Subject: [PATCH 23/25] Revert extraction of min/max building --- src/hotspot/share/opto/addnode.cpp | 23 ++++++++++++----------- src/hotspot/share/opto/addnode.hpp | 1 - 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index e8cc7d6988bb3..5ee2283e823f1 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1030,14 +1030,6 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { return AddNode::Value(phase); } -Node* MaxNode::build_min_max_int(Node* a, Node* b, bool is_max) { - if (is_max) { - return new MaxINode(a, b); - } else { - return new MinINode(a, b); - } -} - Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn) { bool is_int = gvn.type(a)->isa_int(); assert(is_int || gvn.type(a)->isa_long(), "int or long inputs"); @@ -1052,8 +1044,13 @@ Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, co } Node* res = nullptr; if (is_int && !is_unsigned) { - res = gvn.transform(build_min_max_int(a, b, is_max)); - assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); + if (is_max) { + res = gvn.transform(new MaxINode(a, b)); + assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); + } else { + Node* res = gvn.transform(new MinINode(a, b)); + assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); + } } else { Node* cmp = nullptr; if (is_max) { @@ -1183,7 +1180,11 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { } Node* add_transformed = phase->transform(add_extracted); Node* inner_other = inner_op->in(inner_add_index == 1 ? 2 : 1); - return build_min_max_int(add_transformed, inner_other, opcode == Op_MaxI); + if (opcode == Op_MinI) { + return new MinINode(add_transformed, inner_other); + } else { + return new MaxINode(add_transformed, inner_other); + } } } // Try to transform diff --git a/src/hotspot/share/opto/addnode.hpp b/src/hotspot/share/opto/addnode.hpp index 6fd328b2b730a..709958b6abf25 100644 --- a/src/hotspot/share/opto/addnode.hpp +++ b/src/hotspot/share/opto/addnode.hpp @@ -252,7 +252,6 @@ class XorLNode : public AddNode { // 2 equal inputs to be equal. class MaxNode : public AddNode { private: - static Node* build_min_max_int(Node* a, Node* b, bool is_max); static Node* build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn); static Node* build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn); Node* extract_add(PhaseGVN* phase, ConstAddOperands x_operands, ConstAddOperands y_operands); From 5a9617136a8a94c8dc83d267d93da72338aa1852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Thu, 1 Jun 2023 14:54:40 +0200 Subject: [PATCH 24/25] Abort idealization if any of the adds has a TOP input --- src/hotspot/share/opto/addnode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 5ee2283e823f1..fb4dd36d18311 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1162,7 +1162,7 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { Node* outer_add = in(inner_op_index == 1 ? 2 : 1); ConstAddOperands outer_add_operands = as_add_with_constant(outer_add); if (outer_add_operands.first == nullptr) { - continue; + return nullptr; // outer_add has a TOP input, no need to continue. } // One operand is a MinI/MaxI and the other is an integer addition with // constant. Test the operands of the inner MinI/MaxI. @@ -1171,7 +1171,7 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { Node* inner_add = inner_op->in(inner_add_index); ConstAddOperands inner_add_operands = as_add_with_constant(inner_add); if (inner_add_operands.first == nullptr) { - continue; + return nullptr; // inner_add has a TOP input, no need to continue. } // Try to extract the inner add. Node* add_extracted = extract_add(phase, inner_add_operands, outer_add_operands); From cfcc16fd83d71a8dbc65bfe34b44cfcc912a4d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Casta=C3=B1eda=20Lozano?= Date: Fri, 2 Jun 2023 09:23:00 +0200 Subject: [PATCH 25/25] Re-apply extraction of min/max building after JDK-8309295 --- src/hotspot/share/opto/addnode.cpp | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 5d5d59a02bc81..cf8f58d8e2307 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -1030,6 +1030,14 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { return AddNode::Value(phase); } +Node* build_min_max_int(Node* a, Node* b, bool is_max) { + if (is_max) { + return new MaxINode(a, b); + } else { + return new MinINode(a, b); + } +} + Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn) { bool is_int = gvn.type(a)->isa_int(); assert(is_int || gvn.type(a)->isa_long(), "int or long inputs"); @@ -1044,13 +1052,7 @@ Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, co } Node* res = nullptr; if (is_int && !is_unsigned) { - Node* res_new = nullptr; - if (is_max) { - res_new = new MaxINode(a, b); - } else { - res_new = new MinINode(a, b); - } - res = gvn.transform(res_new); + res = gvn.transform(build_min_max_int(a, b, is_max)); assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match"); } else { Node* cmp = nullptr; @@ -1181,11 +1183,7 @@ Node* MaxNode::IdealI(PhaseGVN* phase, bool can_reshape) { } Node* add_transformed = phase->transform(add_extracted); Node* inner_other = inner_op->in(inner_add_index == 1 ? 2 : 1); - if (opcode == Op_MinI) { - return new MinINode(add_transformed, inner_other); - } else { - return new MaxINode(add_transformed, inner_other); - } + return build_min_max_int(add_transformed, inner_other, opcode == Op_MaxI); } } // Try to transform