Skip to content

Commit 2eeaa57

Browse files
committed
8343944: C2: MinLNode::add_ring() computes _widen wrongly leading to an endless widening/compilation
Reviewed-by: thartmann, rcastanedalo
1 parent e9ede46 commit 2eeaa57

File tree

2 files changed

+81
-1
lines changed

2 files changed

+81
-1
lines changed

src/hotspot/share/opto/addnode.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ const Type* MinLNode::add_ring(const Type* t0, const Type* t1) const {
15461546
const TypeLong* r0 = t0->is_long();
15471547
const TypeLong* r1 = t1->is_long();
15481548

1549-
return TypeLong::make(MIN2(r0->_lo, r1->_lo), MIN2(r0->_hi, r1->_hi), MIN2(r0->_widen, r1->_widen));
1549+
return TypeLong::make(MIN2(r0->_lo, r1->_lo), MIN2(r0->_hi, r1->_hi), MAX2(r0->_widen, r1->_widen));
15501550
}
15511551

15521552
Node* MinLNode::Identity(PhaseGVN* phase) {
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright (c) 2024, 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+
24+
/*
25+
* @test
26+
* @bug 8343944
27+
* @summary Test that _widen is set correctly in MinL::add_ring() to prevent an endless widening in CCP.
28+
* @run main/othervm -Xcomp -XX:CompileCommand=compileonly,compiler.ccp.TestWrongMinLWiden::*
29+
* compiler.ccp.TestWrongMinLWiden
30+
*/
31+
32+
package compiler.ccp;
33+
34+
public class TestWrongMinLWiden {
35+
static long lFld;
36+
static short sFld;
37+
38+
public static void main(String[] strArr) {
39+
Math.min(3,3); // Make sure Math class is loaded.
40+
test();
41+
testWithMathMin();
42+
}
43+
44+
45+
static long test() {
46+
long x = 50398;
47+
for (int i = 1; i < 100; i++) {
48+
long xMinus1 = x - 1; // x is a phi with type #long
49+
// Long comparison:
50+
// ConvI2L(sFld) <= xMinus1
51+
// First converted to
52+
// CMoveL(sFld <= xMinus1, sFld, xMinus1)
53+
// CMoveL(ConvI2L(sFld) <= long_phi, ConvI2L(sFld), long_phi)
54+
// with types
55+
// CMoveL(#short <= #long, #short, #long)
56+
// And then converted in CMoveNode::Ideal() to
57+
// MinL(sFld, xMinus1)
58+
// MinL(ConvI2L(sFld), long_phi)
59+
//
60+
// We wrongly set the _widen of the new type for MinL in MinL::add_ring() to the minimum of both types which
61+
// is always 0 because the _widen of sFld is 0. As a result, we will endlessly widen the involved types
62+
// because the new type for MinL keeps resetting _widen to 0.
63+
// Instead, we should choose the maximum of both _widen fields for the new type for MinL which will then
64+
// select the _widen of xMinus1 which will grow each time we set a new type for the long_phi. Eventually,
65+
// we will saturate the type of long_phi to min_long to avoid an endless widening.
66+
x = sFld <= xMinus1 ? sFld : xMinus1;
67+
}
68+
return x;
69+
}
70+
71+
// Same as test() but with Math.min() which internally uses x <= y ? x : y which allows the CMoveL pattern to be
72+
// replaced with MinL.
73+
static long testWithMathMin() {
74+
long x = 50398;
75+
for (int i = 1; i < 100; i++) {
76+
x = Math.min(sFld, x - 1);
77+
}
78+
return x;
79+
}
80+
}

0 commit comments

Comments
 (0)