From 51d0a5d1a62283f4b314c98506c30a2253ad806a Mon Sep 17 00:00:00 2001 From: SirYwell Date: Wed, 2 Apr 2025 15:58:17 +0200 Subject: [PATCH 1/6] Implement constant folding for ReverseBytes*Nodes --- src/hotspot/share/opto/subnode.cpp | 30 ++++ src/hotspot/share/opto/subnode.hpp | 4 + .../irTests/ReverseBytesConstantsTests.java | 170 ++++++++++++++++++ 3 files changed, 204 insertions(+) create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java diff --git a/src/hotspot/share/opto/subnode.cpp b/src/hotspot/share/opto/subnode.cpp index 24d1f820ffbc0..67a195cdc08cd 100644 --- a/src/hotspot/share/opto/subnode.cpp +++ b/src/hotspot/share/opto/subnode.cpp @@ -2021,6 +2021,36 @@ const Type* SqrtHFNode::Value(PhaseGVN* phase) const { return TypeH::make((float)sqrt((double)f)); } +template +const Type* reverse_bytes(const Node* node, PhaseGVN* phase) { + const Type* type = phase->type(node->in(1)); + if (type == Type::TOP) { + return Type::TOP; + } + const TypeInteger* typeInteger = type->isa_integer(B); + if (typeInteger != nullptr && typeInteger->is_con()) { + const T res = byteswap(static_cast(typeInteger->get_con_as_long(B))); + return TypeInteger::make(res, B); + } + return node->bottom_type(); +} + +const Type* ReverseBytesINode::Value(PhaseGVN* phase) const { + return reverse_bytes(this, phase); +} + +const Type* ReverseBytesSNode::Value(PhaseGVN* phase) const { + return reverse_bytes(this, phase); +} + +const Type* ReverseBytesUSNode::Value(PhaseGVN* phase) const { + return reverse_bytes(this, phase); +} + +const Type* ReverseBytesLNode::Value(PhaseGVN* phase) const { + return reverse_bytes(this, phase); +} + const Type* ReverseINode::Value(PhaseGVN* phase) const { const Type *t1 = phase->type( in(1) ); if (t1 == Type::TOP) { diff --git a/src/hotspot/share/opto/subnode.hpp b/src/hotspot/share/opto/subnode.hpp index 0899a2c13fcff..74c949bf15aa5 100644 --- a/src/hotspot/share/opto/subnode.hpp +++ b/src/hotspot/share/opto/subnode.hpp @@ -569,6 +569,7 @@ class ReverseBytesINode : public InvolutionNode { virtual int Opcode() const; const Type* bottom_type() const { return TypeInt::INT; } virtual uint ideal_reg() const { return Op_RegI; } + virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseBytesLNode-------------------------------- @@ -579,6 +580,7 @@ class ReverseBytesLNode : public InvolutionNode { virtual int Opcode() const; const Type* bottom_type() const { return TypeLong::LONG; } virtual uint ideal_reg() const { return Op_RegL; } + virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseBytesUSNode-------------------------------- @@ -589,6 +591,7 @@ class ReverseBytesUSNode : public InvolutionNode { virtual int Opcode() const; const Type* bottom_type() const { return TypeInt::CHAR; } virtual uint ideal_reg() const { return Op_RegI; } + virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseBytesSNode-------------------------------- @@ -599,6 +602,7 @@ class ReverseBytesSNode : public InvolutionNode { virtual int Opcode() const; const Type* bottom_type() const { return TypeInt::SHORT; } virtual uint ideal_reg() const { return Op_RegI; } + virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseINode-------------------------------- diff --git a/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java b/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java new file mode 100644 index 0000000000000..7f0e30dd6bfb8 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java @@ -0,0 +1,170 @@ +/* + * Copyright (c) 2025, 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. + */ +package compiler.c2.irTests; + +import compiler.lib.generators.Generators; +import compiler.lib.generators.RestrictableGenerator; +import compiler.lib.ir_framework.DontCompile; +import compiler.lib.ir_framework.IR; +import compiler.lib.ir_framework.IRNode; +import compiler.lib.ir_framework.Run; +import compiler.lib.ir_framework.Test; +import compiler.lib.ir_framework.TestFramework; +import jdk.test.lib.Asserts; + +/* + * @test + * @bug 8353551 + * @summary Test that ReverseBytes operations constant-fold. + * @library /test/lib / + * @run driver compiler.c2.irTests.ReverseBytesConstantsTests + */ +public class ReverseBytesConstantsTests { + + private static final RestrictableGenerator GEN_CHAR = Generators.G.safeRestrict(Generators.G.ints(), Character.MIN_VALUE, Character.MAX_VALUE); + private static final char C_CHAR = (char) GEN_CHAR.next().intValue(); + private static final RestrictableGenerator GEN_SHORT = Generators.G.safeRestrict(Generators.G.ints(), Short.MIN_VALUE, Short.MAX_VALUE); + private static final short C_SHORT = GEN_SHORT.next().shortValue(); + private static final RestrictableGenerator GEN_LONG = Generators.G.longs(); + private static final long C_LONG = GEN_LONG.next(); + private static final RestrictableGenerator GEN_INT = Generators.G.ints(); + private static final int C_INT = GEN_INT.next(); + + public static void main(String[] args) { + TestFramework.run(); + } + + @Run(test = { + "testI1", "testI2", "testI3", + "testL1", "testL2", "testL3", + "testS1", "testS2", "testS3", + "testUS1", "testUS2", "testUS3", + }) + public void runMethod() { + assertResultI(); + assertResultL(); + assertResultS(); + assertResultUS(); + } + + @DontCompile + public void assertResultI() { + Asserts.assertEQ(Integer.reverseBytes(0x04030201), testI1()); + Asserts.assertEQ(Integer.reverseBytes(0x50607080), testI2()); + Asserts.assertEQ(Integer.reverseBytes(C_INT), testI3()); + } + + @DontCompile + public void assertResultL() { + Asserts.assertEQ(Long.reverseBytes(0x0807060504030201L), testL1()); + Asserts.assertEQ(Long.reverseBytes(0x1020304050607080L), testL2()); + Asserts.assertEQ(Long.reverseBytes(C_LONG), testL3()); + } + + @DontCompile + public void assertResultS() { + Asserts.assertEQ(Short.reverseBytes((short) 0x0201), testS1()); + Asserts.assertEQ(Short.reverseBytes((short) 0x7080), testS2()); + Asserts.assertEQ(Short.reverseBytes(C_SHORT), testS3()); + } + + @DontCompile + public void assertResultUS() { + Asserts.assertEQ(Character.reverseBytes((char) 0x0201), testUS1()); + Asserts.assertEQ(Character.reverseBytes((char) 0x7080), testUS2()); + Asserts.assertEQ(Character.reverseBytes(C_CHAR), testUS3()); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_I}) + public int testI1() { + return Integer.reverseBytes(0x04030201); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_I}) + public int testI2() { + return Integer.reverseBytes(0x50607080); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_I}) + public int testI3() { + return Integer.reverseBytes(C_INT); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_L}) + public long testL1() { + return Long.reverseBytes(0x0807060504030201L); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_L}) + public long testL2() { + return Long.reverseBytes(0x1020304050607080L); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_L}) + public long testL3() { + return Long.reverseBytes(C_LONG); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_S}) + public short testS1() { + return Short.reverseBytes((short) 0x0201); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_S}) + public short testS2() { + return Short.reverseBytes((short) 0x7080); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_S}) + public short testS3() { + return Short.reverseBytes(C_SHORT); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_US}) + public char testUS1() { + return Character.reverseBytes((char) 0x0201); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_US}) + public char testUS2() { + return Character.reverseBytes((char) 0x7080); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_US}) + public char testUS3() { + return Character.reverseBytes(C_CHAR); + } + +} From dc046768ca4638c29179f5e4ff0ed06c6315b4cb Mon Sep 17 00:00:00 2001 From: SirYwell Date: Tue, 8 Apr 2025 10:11:47 +0200 Subject: [PATCH 2/6] add test cases with negative -> non-negative --- .../irTests/ReverseBytesConstantsTests.java | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java b/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java index 7f0e30dd6bfb8..35d2900dfd883 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java @@ -71,28 +71,32 @@ public void runMethod() { public void assertResultI() { Asserts.assertEQ(Integer.reverseBytes(0x04030201), testI1()); Asserts.assertEQ(Integer.reverseBytes(0x50607080), testI2()); - Asserts.assertEQ(Integer.reverseBytes(C_INT), testI3()); + Asserts.assertEQ(Integer.reverseBytes(0x80706050), testI3()); + Asserts.assertEQ(Integer.reverseBytes(C_INT), testI4()); } @DontCompile public void assertResultL() { Asserts.assertEQ(Long.reverseBytes(0x0807060504030201L), testL1()); Asserts.assertEQ(Long.reverseBytes(0x1020304050607080L), testL2()); - Asserts.assertEQ(Long.reverseBytes(C_LONG), testL3()); + Asserts.assertEQ(Long.reverseBytes(0x8070605040302010L), testL3()); + Asserts.assertEQ(Long.reverseBytes(C_LONG), testL4()); } @DontCompile public void assertResultS() { Asserts.assertEQ(Short.reverseBytes((short) 0x0201), testS1()); Asserts.assertEQ(Short.reverseBytes((short) 0x7080), testS2()); - Asserts.assertEQ(Short.reverseBytes(C_SHORT), testS3()); + Asserts.assertEQ(Short.reverseBytes((short) 0x8070), testS3()); + Asserts.assertEQ(Short.reverseBytes(C_SHORT), testS4()); } @DontCompile public void assertResultUS() { Asserts.assertEQ(Character.reverseBytes((char) 0x0201), testUS1()); Asserts.assertEQ(Character.reverseBytes((char) 0x7080), testUS2()); - Asserts.assertEQ(Character.reverseBytes(C_CHAR), testUS3()); + Asserts.assertEQ(Character.reverseBytes((char) 0x8070), testUS3()); + Asserts.assertEQ(Character.reverseBytes(C_CHAR), testUS4()); } @Test @@ -110,6 +114,12 @@ public int testI2() { @Test @IR(failOn = {IRNode.REVERSE_BYTES_I}) public int testI3() { + return Integer.reverseBytes(0x80706050); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_I}) + public int testI4() { return Integer.reverseBytes(C_INT); } @@ -128,6 +138,12 @@ public long testL2() { @Test @IR(failOn = {IRNode.REVERSE_BYTES_L}) public long testL3() { + return Long.reverseBytes(0x8070605040302010L); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_L}) + public long testL4() { return Long.reverseBytes(C_LONG); } @@ -146,6 +162,12 @@ public short testS2() { @Test @IR(failOn = {IRNode.REVERSE_BYTES_S}) public short testS3() { + return Short.reverseBytes((short) 0x8070); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_S}) + public short testS4() { return Short.reverseBytes(C_SHORT); } @@ -164,6 +186,12 @@ public char testUS2() { @Test @IR(failOn = {IRNode.REVERSE_BYTES_US}) public char testUS3() { + return Character.reverseBytes((char) 0x8070); + } + + @Test + @IR(failOn = {IRNode.REVERSE_BYTES_US}) + public char testUS4() { return Character.reverseBytes(C_CHAR); } From 90f2e9de8c42f32ce197a797de75ca34c4a41105 Mon Sep 17 00:00:00 2001 From: SirYwell Date: Thu, 10 Apr 2025 10:51:27 +0200 Subject: [PATCH 3/6] introduce common ReverseBytesNode --- src/hotspot/share/opto/subnode.cpp | 39 ++++++++++++------------------ src/hotspot/share/opto/subnode.hpp | 28 ++++++++++++--------- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/hotspot/share/opto/subnode.cpp b/src/hotspot/share/opto/subnode.cpp index 67a195cdc08cd..b668fda325035 100644 --- a/src/hotspot/share/opto/subnode.cpp +++ b/src/hotspot/share/opto/subnode.cpp @@ -2021,34 +2021,25 @@ const Type* SqrtHFNode::Value(PhaseGVN* phase) const { return TypeH::make((float)sqrt((double)f)); } -template -const Type* reverse_bytes(const Node* node, PhaseGVN* phase) { - const Type* type = phase->type(node->in(1)); +const Type* reverse_bytes(int opcode, const Type* con) { + switch (opcode) { + case Op_ReverseBytesS: return TypeInt::make(byteswap(checked_cast(con->is_int()->get_con()))); + case Op_ReverseBytesUS: return TypeInt::make(byteswap(checked_cast(con->is_int()->get_con()))); + case Op_ReverseBytesI: return TypeInt::make(byteswap(checked_cast(con->is_int()->get_con()))); + case Op_ReverseBytesL: return TypeLong::make(byteswap(checked_cast(con->is_long()->get_con()))); + default: ShouldNotReachHere(); + } +} + +const Type* ReverseBytesNode::Value(PhaseGVN* phase) const { + const Type* type = phase->type(in(1)); if (type == Type::TOP) { return Type::TOP; } - const TypeInteger* typeInteger = type->isa_integer(B); - if (typeInteger != nullptr && typeInteger->is_con()) { - const T res = byteswap(static_cast(typeInteger->get_con_as_long(B))); - return TypeInteger::make(res, B); + if (type->singleton()) { + return reverse_bytes(Opcode(), type); } - return node->bottom_type(); -} - -const Type* ReverseBytesINode::Value(PhaseGVN* phase) const { - return reverse_bytes(this, phase); -} - -const Type* ReverseBytesSNode::Value(PhaseGVN* phase) const { - return reverse_bytes(this, phase); -} - -const Type* ReverseBytesUSNode::Value(PhaseGVN* phase) const { - return reverse_bytes(this, phase); -} - -const Type* ReverseBytesLNode::Value(PhaseGVN* phase) const { - return reverse_bytes(this, phase); + return bottom_type(); } const Type* ReverseINode::Value(PhaseGVN* phase) const { diff --git a/src/hotspot/share/opto/subnode.hpp b/src/hotspot/share/opto/subnode.hpp index 74c949bf15aa5..9a411f3d20e6b 100644 --- a/src/hotspot/share/opto/subnode.hpp +++ b/src/hotspot/share/opto/subnode.hpp @@ -561,48 +561,52 @@ class SqrtHFNode : public Node { virtual const Type* Value(PhaseGVN* phase) const; }; + +class ReverseBytesNode : public InvolutionNode { +public: + ReverseBytesNode(Node* in) : InvolutionNode(in) {} + virtual const Type* Value(PhaseGVN* phase) const; +}; //-------------------------------ReverseBytesINode-------------------------------- // reverse bytes of an integer -class ReverseBytesINode : public InvolutionNode { +class ReverseBytesINode : public ReverseBytesNode { public: - ReverseBytesINode(Node* in) : InvolutionNode(in) {} + ReverseBytesINode(Node* in) : ReverseBytesNode(in) { + } + virtual int Opcode() const; const Type* bottom_type() const { return TypeInt::INT; } virtual uint ideal_reg() const { return Op_RegI; } - virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseBytesLNode-------------------------------- // reverse bytes of a long -class ReverseBytesLNode : public InvolutionNode { +class ReverseBytesLNode : public ReverseBytesNode { public: - ReverseBytesLNode(Node* in) : InvolutionNode(in) {} + ReverseBytesLNode(Node* in) : ReverseBytesNode(in) {} virtual int Opcode() const; const Type* bottom_type() const { return TypeLong::LONG; } virtual uint ideal_reg() const { return Op_RegL; } - virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseBytesUSNode-------------------------------- // reverse bytes of an unsigned short / char -class ReverseBytesUSNode : public InvolutionNode { +class ReverseBytesUSNode : public ReverseBytesNode { public: - ReverseBytesUSNode(Node* in1) : InvolutionNode(in1) {} + ReverseBytesUSNode(Node* in1) : ReverseBytesNode(in1) {} virtual int Opcode() const; const Type* bottom_type() const { return TypeInt::CHAR; } virtual uint ideal_reg() const { return Op_RegI; } - virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseBytesSNode-------------------------------- // reverse bytes of a short -class ReverseBytesSNode : public InvolutionNode { +class ReverseBytesSNode : public ReverseBytesNode { public: - ReverseBytesSNode(Node* in) : InvolutionNode(in) {} + ReverseBytesSNode(Node* in) : ReverseBytesNode(in) {} virtual int Opcode() const; const Type* bottom_type() const { return TypeInt::SHORT; } virtual uint ideal_reg() const { return Op_RegI; } - virtual const Type* Value(PhaseGVN* phase) const; }; //-------------------------------ReverseINode-------------------------------- From 278a2a7c9ea0ebadb309bce03c97e9e456dda89e Mon Sep 17 00:00:00 2001 From: SirYwell Date: Mon, 14 Apr 2025 08:42:00 +0200 Subject: [PATCH 4/6] make function static --- src/hotspot/share/opto/subnode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/subnode.cpp b/src/hotspot/share/opto/subnode.cpp index b668fda325035..d547cbffe9813 100644 --- a/src/hotspot/share/opto/subnode.cpp +++ b/src/hotspot/share/opto/subnode.cpp @@ -2021,7 +2021,7 @@ const Type* SqrtHFNode::Value(PhaseGVN* phase) const { return TypeH::make((float)sqrt((double)f)); } -const Type* reverse_bytes(int opcode, const Type* con) { +static const Type* reverse_bytes(int opcode, const Type* con) { switch (opcode) { case Op_ReverseBytesS: return TypeInt::make(byteswap(checked_cast(con->is_int()->get_con()))); case Op_ReverseBytesUS: return TypeInt::make(byteswap(checked_cast(con->is_int()->get_con()))); From f0ec0e4a2490dbabde3aa5d5fd14def23e0897ee Mon Sep 17 00:00:00 2001 From: SirYwell Date: Fri, 25 Apr 2025 10:42:43 +0200 Subject: [PATCH 5/6] move test --- .../c2/{irTests => gvn}/ReverseBytesConstantsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/hotspot/jtreg/compiler/c2/{irTests => gvn}/ReverseBytesConstantsTests.java (99%) diff --git a/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java b/test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java similarity index 99% rename from test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java rename to test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java index 35d2900dfd883..741951803e4d0 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/ReverseBytesConstantsTests.java +++ b/test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java @@ -20,7 +20,7 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package compiler.c2.irTests; +package compiler.c2.gvn; import compiler.lib.generators.Generators; import compiler.lib.generators.RestrictableGenerator; From 3a94bbe6d63b91171c948ea4a1496c3b918f1d8a Mon Sep 17 00:00:00 2001 From: SirYwell Date: Fri, 25 Apr 2025 10:46:50 +0200 Subject: [PATCH 6/6] correct driver path --- .../jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java b/test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java index 741951803e4d0..13461771cc93a 100644 --- a/test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java +++ b/test/hotspot/jtreg/compiler/c2/gvn/ReverseBytesConstantsTests.java @@ -37,7 +37,7 @@ * @bug 8353551 * @summary Test that ReverseBytes operations constant-fold. * @library /test/lib / - * @run driver compiler.c2.irTests.ReverseBytesConstantsTests + * @run driver compiler.c2.gvn.ReverseBytesConstantsTests */ public class ReverseBytesConstantsTests {