Skip to content

Commit 5234041

Browse files
merykittymerykitty
committed
8375653: C2: CmpUNode::sub is not monotonic
Reviewed-by: chagedorn, thartmann Backport-of: 30675fa
1 parent 6ac3ce0 commit 5234041

3 files changed

Lines changed: 310 additions & 96 deletions

File tree

‎src/hotspot/share/opto/subnode.cpp‎

Lines changed: 35 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -745,71 +745,40 @@ const Type* CmpINode::Value(PhaseGVN* phase) const {
745745

746746
// Simplify a CmpU (compare 2 integers) node, based on local information.
747747
// If both inputs are constants, compare them.
748-
const Type *CmpUNode::sub( const Type *t1, const Type *t2 ) const {
749-
assert(!t1->isa_ptr(), "obsolete usage of CmpU");
748+
const Type* CmpUNode::sub(const Type* t1, const Type* t2) const {
749+
const TypeInt* r0 = t1->is_int();
750+
const TypeInt* r1 = t2->is_int();
750751

751-
// comparing two unsigned ints
752-
const TypeInt *r0 = t1->is_int(); // Handy access
753-
const TypeInt *r1 = t2->is_int();
754-
755-
// Current installed version
756-
// Compare ranges for non-overlap
757-
juint lo0 = r0->_lo;
758-
juint hi0 = r0->_hi;
759-
juint lo1 = r1->_lo;
760-
juint hi1 = r1->_hi;
761-
762-
// If either one has both negative and positive values,
763-
// it therefore contains both 0 and -1, and since [0..-1] is the
764-
// full unsigned range, the type must act as an unsigned bottom.
765-
bool bot0 = ((jint)(lo0 ^ hi0) < 0);
766-
bool bot1 = ((jint)(lo1 ^ hi1) < 0);
767-
768-
if (bot0 || bot1) {
769-
// All unsigned values are LE -1 and GE 0.
770-
if (lo0 == 0 && hi0 == 0) {
771-
return TypeInt::CC_LE; // 0 <= bot
772-
} else if ((jint)lo0 == -1 && (jint)hi0 == -1) {
773-
return TypeInt::CC_GE; // -1 >= bot
774-
} else if (lo1 == 0 && hi1 == 0) {
775-
return TypeInt::CC_GE; // bot >= 0
776-
} else if ((jint)lo1 == -1 && (jint)hi1 == -1) {
777-
return TypeInt::CC_LE; // bot <= -1
778-
}
779-
} else {
780-
// We can use ranges of the form [lo..hi] if signs are the same.
781-
assert(lo0 <= hi0 && lo1 <= hi1, "unsigned ranges are valid");
782-
// results are reversed, '-' > '+' for unsigned compare
783-
if (hi0 < lo1) {
784-
return TypeInt::CC_LT; // smaller
785-
} else if (lo0 > hi1) {
786-
return TypeInt::CC_GT; // greater
787-
} else if (hi0 == lo1 && lo0 == hi1) {
788-
return TypeInt::CC_EQ; // Equal results
789-
} else if (lo0 >= hi1) {
790-
return TypeInt::CC_GE;
791-
} else if (hi0 <= lo1) {
792-
// Check for special case in Hashtable::get. (See below.)
793-
if ((jint)lo0 >= 0 && (jint)lo1 >= 0 && is_index_range_check())
794-
return TypeInt::CC_LT;
795-
return TypeInt::CC_LE;
796-
}
797-
}
798752
// Check for special case in Hashtable::get - the hash index is
799753
// mod'ed to the table size so the following range check is useless.
800754
// Check for: (X Mod Y) CmpU Y, where the mod result and Y both have
801755
// to be positive.
802756
// (This is a gross hack, since the sub method never
803757
// looks at the structure of the node in any other case.)
804-
if ((jint)lo0 >= 0 && (jint)lo1 >= 0 && is_index_range_check())
758+
if (r0->_lo >= 0 && r1->_lo >= 0 && is_index_range_check()) {
805759
return TypeInt::CC_LT;
760+
}
761+
762+
if (r0->_uhi < r1->_ulo) {
763+
return TypeInt::CC_LT;
764+
} else if (r0->_ulo > r1->_uhi) {
765+
return TypeInt::CC_GT;
766+
} else if (r0->is_con() && r1->is_con()) {
767+
// Since r0->_ulo == r0->_uhi == r0->get_con(), we only reach here if the constants are equal
768+
assert(r0->get_con() == r1->get_con(), "must reach a previous branch otherwise");
769+
return TypeInt::CC_EQ;
770+
} else if (r0->_uhi == r1->_ulo) {
771+
return TypeInt::CC_LE;
772+
} else if (r0->_ulo == r1->_uhi) {
773+
return TypeInt::CC_GE;
774+
}
806775

807776
const Type* joined = r0->join(r1);
808777
if (joined == Type::TOP) {
809778
return TypeInt::CC_NE;
810779
}
811780

812-
return TypeInt::CC; // else use worst case results
781+
return TypeInt::CC;
813782
}
814783

815784
const Type* CmpUNode::Value(PhaseGVN* phase) const {
@@ -963,59 +932,29 @@ const Type *CmpLNode::sub( const Type *t1, const Type *t2 ) const {
963932
// Simplify a CmpUL (compare 2 unsigned longs) node, based on local information.
964933
// If both inputs are constants, compare them.
965934
const Type* CmpULNode::sub(const Type* t1, const Type* t2) const {
966-
assert(!t1->isa_ptr(), "obsolete usage of CmpUL");
967-
968-
// comparing two unsigned longs
969-
const TypeLong* r0 = t1->is_long(); // Handy access
935+
const TypeLong* r0 = t1->is_long();
970936
const TypeLong* r1 = t2->is_long();
971937

972-
// Current installed version
973-
// Compare ranges for non-overlap
974-
julong lo0 = r0->_lo;
975-
julong hi0 = r0->_hi;
976-
julong lo1 = r1->_lo;
977-
julong hi1 = r1->_hi;
978-
979-
// If either one has both negative and positive values,
980-
// it therefore contains both 0 and -1, and since [0..-1] is the
981-
// full unsigned range, the type must act as an unsigned bottom.
982-
bool bot0 = ((jlong)(lo0 ^ hi0) < 0);
983-
bool bot1 = ((jlong)(lo1 ^ hi1) < 0);
984-
985-
if (bot0 || bot1) {
986-
// All unsigned values are LE -1 and GE 0.
987-
if (lo0 == 0 && hi0 == 0) {
988-
return TypeInt::CC_LE; // 0 <= bot
989-
} else if ((jlong)lo0 == -1 && (jlong)hi0 == -1) {
990-
return TypeInt::CC_GE; // -1 >= bot
991-
} else if (lo1 == 0 && hi1 == 0) {
992-
return TypeInt::CC_GE; // bot >= 0
993-
} else if ((jlong)lo1 == -1 && (jlong)hi1 == -1) {
994-
return TypeInt::CC_LE; // bot <= -1
995-
}
996-
} else {
997-
// We can use ranges of the form [lo..hi] if signs are the same.
998-
assert(lo0 <= hi0 && lo1 <= hi1, "unsigned ranges are valid");
999-
// results are reversed, '-' > '+' for unsigned compare
1000-
if (hi0 < lo1) {
1001-
return TypeInt::CC_LT; // smaller
1002-
} else if (lo0 > hi1) {
1003-
return TypeInt::CC_GT; // greater
1004-
} else if (hi0 == lo1 && lo0 == hi1) {
1005-
return TypeInt::CC_EQ; // Equal results
1006-
} else if (lo0 >= hi1) {
1007-
return TypeInt::CC_GE;
1008-
} else if (hi0 <= lo1) {
1009-
return TypeInt::CC_LE;
1010-
}
938+
if (r0->_uhi < r1->_ulo) {
939+
return TypeInt::CC_LT;
940+
} else if (r0->_ulo > r1->_uhi) {
941+
return TypeInt::CC_GT;
942+
} else if (r0->is_con() && r1->is_con()) {
943+
// Since r0->_ulo == r0->_uhi == r0->get_con(), we only reach here if the constants are equal
944+
assert(r0->get_con() == r1->get_con(), "must reach a previous branch otherwise");
945+
return TypeInt::CC_EQ;
946+
} else if (r0->_uhi == r1->_ulo) {
947+
return TypeInt::CC_LE;
948+
} else if (r0->_ulo == r1->_uhi) {
949+
return TypeInt::CC_GE;
1011950
}
1012951

1013952
const Type* joined = r0->join(r1);
1014953
if (joined == Type::TOP) {
1015954
return TypeInt::CC_NE;
1016955
}
1017956

1018-
return TypeInt::CC; // else use worst case results
957+
return TypeInt::CC;
1019958
}
1020959

1021960
//=============================================================================

‎test/hotspot/jtreg/compiler/c2/gvn/CmpUNodeValueTests.java‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
/*
2+
* Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
package compiler.c2.gvn;
24+
25+
import compiler.lib.generators.Generators;
26+
import compiler.lib.ir_framework.*;
27+
import jdk.test.lib.Asserts;
28+
29+
/*
30+
* @test
31+
* @bug 8375653
32+
* @summary Test that Value computations of CmpUNode are being performed as expected.
33+
* @library /test/lib /
34+
* @run driver ${test.main.class}
35+
*/
36+
public class CmpUNodeValueTests {
37+
public static void main(String[] args) {
38+
TestFramework.run();
39+
}
40+
41+
@DontInline
42+
public static int one() {
43+
return 1;
44+
}
45+
46+
// Move to a separate method to prevent javac folding the constant comparison
47+
@ForceInline
48+
public static int oneInline() {
49+
return 1;
50+
}
51+
52+
@Run(test = {"testIntControl", "testIntLT", "testIntLE", "testIntGT", "testIntGE", "testIntEQ", "testIntNE",
53+
"testLongControl", "testLongLT", "testLongLE", "testLongGT", "testLongGE", "testLongEQ", "testLongNE"})
54+
public void run() {
55+
var stream = Generators.G.longs();
56+
long x = stream.next();
57+
long y = stream.next();
58+
59+
Asserts.assertEQ(0, testIntControl(false, false));
60+
Asserts.assertEQ(0, testIntControl(false, true));
61+
Asserts.assertEQ(0, testIntControl(true, false));
62+
Asserts.assertEQ(1, testIntControl(true, true));
63+
Asserts.assertEQ(0, testIntLT((int) x));
64+
Asserts.assertEQ(0, testIntLE(false, false));
65+
Asserts.assertEQ(0, testIntLE(false, true));
66+
Asserts.assertEQ(0, testIntLE(true, false));
67+
Asserts.assertEQ(0, testIntLE(true, true));
68+
Asserts.assertEQ(0, testIntGT(false, false));
69+
Asserts.assertEQ(0, testIntGT(false, true));
70+
Asserts.assertEQ(0, testIntGT(true, false));
71+
Asserts.assertEQ(0, testIntGT(true, true));
72+
Asserts.assertEQ(0, testIntGE((int) y));
73+
Asserts.assertEQ(0, testIntEQ());
74+
Asserts.assertEQ(0, testIntNE(false, false));
75+
Asserts.assertEQ(0, testIntNE(false, true));
76+
Asserts.assertEQ(0, testIntNE(true, false));
77+
Asserts.assertEQ(0, testIntNE(true, true));
78+
79+
Asserts.assertEQ(0, testLongControl(false, false));
80+
Asserts.assertEQ(0, testLongControl(false, true));
81+
Asserts.assertEQ(0, testLongControl(true, false));
82+
Asserts.assertEQ(1, testLongControl(true, true));
83+
Asserts.assertEQ(0, testLongLT((int) x, false));
84+
Asserts.assertEQ(0, testLongLT((int) x, true));
85+
Asserts.assertEQ(0, testLongLE(false, false));
86+
Asserts.assertEQ(0, testLongLE(false, true));
87+
Asserts.assertEQ(0, testLongLE(true, false));
88+
Asserts.assertEQ(0, testLongLE(true, true));
89+
Asserts.assertEQ(0, testLongGT(false, false));
90+
Asserts.assertEQ(0, testLongGT(false, true));
91+
Asserts.assertEQ(0, testLongGT(true, false));
92+
Asserts.assertEQ(0, testLongGT(true, true));
93+
Asserts.assertEQ(0, testLongGE((int) y));
94+
Asserts.assertEQ(0, testLongEQ());
95+
Asserts.assertEQ(0, testLongNE(false, false));
96+
Asserts.assertEQ(0, testLongNE(false, true));
97+
Asserts.assertEQ(0, testLongNE(true, false));
98+
Asserts.assertEQ(0, testLongNE(true, true));
99+
}
100+
101+
@Test
102+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, counts = {IRNode.CMP_U, "1"})
103+
int testIntControl(boolean b1, boolean b2) {
104+
int v1 = b1 ? 1 : -1;
105+
int v2 = b2 ? 1 : 0;
106+
return Integer.compareUnsigned(v1, v2) > 0 ? 0 : one();
107+
}
108+
109+
@Test
110+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
111+
int testIntLT(int x) {
112+
int v1 = x & Integer.MAX_VALUE; // Unset the highest bit, make v1 non-negative
113+
int v2 = Integer.MIN_VALUE;
114+
return Integer.compareUnsigned(v1, v2) < 0 ? 0 : one();
115+
}
116+
117+
@Test
118+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
119+
int testIntLE(boolean b1, boolean b2) {
120+
int v1 = b1 ? 2 : 0;
121+
int v2 = b2 ? -1 : 2;
122+
return Integer.compareUnsigned(v1, v2) <= 0 ? 0 : one();
123+
}
124+
125+
@Test
126+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
127+
int testIntGT(boolean b1, boolean b2) {
128+
int v1 = b1 ? Integer.MIN_VALUE : Integer.MAX_VALUE;
129+
int v2 = b2 ? 0 : 2;
130+
return Integer.compareUnsigned(v1, v2) > 0 ? 0 : one();
131+
}
132+
133+
@Test
134+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
135+
int testIntGE(int y) {
136+
int v2 = y & Integer.MAX_VALUE; // Unset the highest bit, make v2 non-negative
137+
return Integer.compareUnsigned(Integer.MIN_VALUE, v2) >= 0 ? 0 : one();
138+
}
139+
140+
@Test
141+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
142+
int testIntEQ() {
143+
return Integer.compareUnsigned(oneInline(), 1) == 0 ? 0 : one();
144+
}
145+
146+
@Test
147+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
148+
int testIntNE(boolean b1, boolean b2) {
149+
int v1 = b1 ? 1 : -1;
150+
int v2 = b2 ? 0 : 2;
151+
return Integer.compareUnsigned(v1, v2) != 0 ? 0 : one();
152+
}
153+
154+
@Test
155+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, counts = {IRNode.CMP_UL, "1"})
156+
int testLongControl(boolean b1, boolean b2) {
157+
long v1 = b1 ? 1 : -1;
158+
long v2 = b2 ? 1 : 0;
159+
return Long.compareUnsigned(v1, v2) > 0 ? 0 : one();
160+
}
161+
162+
163+
@Test
164+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
165+
int testLongLT(int x, boolean b2) {
166+
long v1 = Integer.toUnsignedLong(x);
167+
long v2 = Integer.MIN_VALUE;
168+
return Long.compareUnsigned(v1, v2) < 0 ? 0 : one();
169+
}
170+
171+
@Test
172+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
173+
int testLongLE(boolean b1, boolean b2) {
174+
long v1 = b1 ? 2 : 0;
175+
long v2 = b2 ? -1 : 2;
176+
return Long.compareUnsigned(v1, v2) <= 0 ? 0 : one();
177+
}
178+
179+
@Test
180+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
181+
int testLongGT(boolean b1, boolean b2) {
182+
long v1 = b1 ? Long.MIN_VALUE : Long.MAX_VALUE;
183+
long v2 = b2 ? 0 : 2;
184+
return Long.compareUnsigned(v1, v2) > 0 ? 0 : one();
185+
}
186+
187+
@Test
188+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
189+
int testLongGE(int y) {
190+
long v2 = y & Long.MAX_VALUE; // Unset the highest bit, make v2 non-negative
191+
return Long.compareUnsigned(Long.MIN_VALUE, v2) >= 0 ? 0 : one();
192+
}
193+
194+
@Test
195+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
196+
int testLongEQ() {
197+
return Long.compareUnsigned(oneInline(), 1L) == 0 ? 0 : one();
198+
}
199+
200+
@Test
201+
@IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
202+
int testLongNE(boolean b1, boolean b2) {
203+
long v1 = b1 ? 1 : -1;
204+
long v2 = b2 ? 0 : 2;
205+
return Long.compareUnsigned(v1, v2) != 0 ? 0 : one();
206+
}
207+
}

0 commit comments

Comments
 (0)