Skip to content

Commit 432c387

Browse files
robcaslozVladimir Ivanov
authored andcommitted
8254317: C2: Resource consumption of ConvI2LNode::Ideal() grows exponentially
Prevent exponential number of calls to ConvI2LNode::Ideal() when AddIs are used multiple times by other AddIs in the optimization ConvI2L(AddI(x, y)) -> AddL(ConvI2L(x), ConvI2L(y)). This is achieved by (1) reusing existing ConvI2Ls if possible rather than eagerly creating new ones and (2) postponing the optimization of newly created ConvI2Ls. Remove hook node solution introduced in 8217359, since this is subsumed by (2). Use phase->is_IterGVN() rather than can_reshape to check if ConvI2LNode::Ideal() is called within iterative GVN, for clarity. Add regression tests that cover different shapes and sizes of AddI subgraphs, implicitly checking (by not timing out) that there is no combinatorial explosion. Co-authored-by: Vladimir Ivanov <vlivanov@openjdk.org> Reviewed-by: vlivanov, kvn
1 parent 79ac041 commit 432c387

File tree

2 files changed

+186
-10
lines changed

2 files changed

+186
-10
lines changed

src/hotspot/share/opto/convertnode.cpp

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,30 @@ static inline bool long_ranges_overlap(jlong lo1, jlong hi1,
258258
// Two ranges overlap iff one range's low point falls in the other range.
259259
return (lo2 <= lo1 && lo1 <= hi2) || (lo1 <= lo2 && lo2 <= hi1);
260260
}
261+
262+
// If there is an existing ConvI2L node with the given parent and type, return
263+
// it. Otherwise, create and return a new one. Both reusing existing ConvI2L
264+
// nodes and postponing the idealization of new ones are needed to avoid an
265+
// explosion of recursive Ideal() calls when compiling long AddI chains.
266+
static Node* find_or_make_convI2L(PhaseIterGVN* igvn, Node* parent,
267+
const TypeLong* type) {
268+
Node* n = new ConvI2LNode(parent, type);
269+
Node* existing = igvn->hash_find_insert(n);
270+
if (existing != NULL) {
271+
n->destruct(igvn);
272+
return existing;
273+
}
274+
return igvn->register_new_node_with_optimizer(n);
275+
}
261276
#endif
262277

263278
//------------------------------Ideal------------------------------------------
264279
Node *ConvI2LNode::Ideal(PhaseGVN *phase, bool can_reshape) {
280+
PhaseIterGVN *igvn = phase->is_IterGVN();
265281
const TypeLong* this_type = this->type()->is_long();
266282
Node* this_changed = NULL;
267283

268-
if (can_reshape) {
284+
if (igvn != NULL) {
269285
// Do NOT remove this node's type assertion until no more loop ops can happen.
270286
if (phase->C->post_loop_opts_phase()) {
271287
const TypeInt* in_type = phase->type(in(1))->isa_int();
@@ -334,10 +350,9 @@ Node *ConvI2LNode::Ideal(PhaseGVN *phase, bool can_reshape) {
334350
Node* z = in(1);
335351
int op = z->Opcode();
336352
if (op == Op_AddI || op == Op_SubI) {
337-
if (!can_reshape) {
338-
// Postpone this optimization to after parsing because with deep AddNode
339-
// chains a large amount of dead ConvI2L nodes might be created that are
340-
// not removed during parsing. As a result, we might hit the node limit.
353+
if (igvn == NULL) {
354+
// Postpone this optimization to iterative GVN, where we can handle deep
355+
// AddI chains without an exponential number of recursive Ideal() calls.
341356
phase->record_for_igvn(this);
342357
return this_changed;
343358
}
@@ -399,11 +414,8 @@ Node *ConvI2LNode::Ideal(PhaseGVN *phase, bool can_reshape) {
399414
}
400415
assert(rxlo == (int)rxlo && rxhi == (int)rxhi, "x should not overflow");
401416
assert(rylo == (int)rylo && ryhi == (int)ryhi, "y should not overflow");
402-
Node* cx = phase->C->constrained_convI2L(phase, x, TypeInt::make(rxlo, rxhi, widen), NULL);
403-
Node *hook = new Node(1);
404-
hook->init_req(0, cx); // Add a use to cx to prevent him from dying
405-
Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), NULL);
406-
hook->destruct(phase);
417+
Node* cx = find_or_make_convI2L(igvn, x, TypeLong::make(rxlo, rxhi, widen));
418+
Node* cy = find_or_make_convI2L(igvn, y, TypeLong::make(rylo, ryhi, widen));
407419
switch (op) {
408420
case Op_AddI: return new AddLNode(cx, cy);
409421
case Op_SubI: return new SubLNode(cx, cy);
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/*
2+
* Copyright (c) 2020, 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+
package compiler.conversions;
25+
26+
import java.util.Random;
27+
import jdk.test.lib.Asserts;
28+
29+
/*
30+
* @test
31+
* @bug 8254317
32+
* @requires vm.compiler2.enabled
33+
* @summary Exercises the optimization that moves integer-to-long conversions
34+
* upwards through different shapes of integer addition
35+
* subgraphs. Contains three small functional tests and two stress
36+
* tests that resulted in a compilation time and memory explosion
37+
* before fixing bug 8254317. The stress tests run with -Xbatch to wait
38+
* for C2, so that a timeout or an out-of-memory error is triggered if
39+
* there was an explosion. These tests use a timeout of 30s to catch
40+
* the explosion earlier.
41+
* @library /test/lib /
42+
* @run main/othervm
43+
* compiler.conversions.TestMoveConvI2LThroughAddIs functional
44+
* @run main/othervm/timeout=30 -Xbatch
45+
* compiler.conversions.TestMoveConvI2LThroughAddIs stress1
46+
* @run main/othervm/timeout=30 -Xbatch
47+
* compiler.conversions.TestMoveConvI2LThroughAddIs stress2
48+
*/
49+
50+
public class TestMoveConvI2LThroughAddIs {
51+
52+
// Number of repetitions of each test. Should be sufficiently large for the
53+
// method under test to be compiled with C2.
54+
static final int N = 100_000;
55+
56+
// Chain-shaped functional test.
57+
static long testChain(boolean cnd) {
58+
int a = cnd ? 1 : 2;
59+
int b = a + a;
60+
int c = b + b;
61+
int d = c + c;
62+
return d;
63+
}
64+
65+
// Tree-shaped functional test.
66+
static long testTree(boolean cnd) {
67+
int a0 = cnd ? 1 : 2;
68+
int a1 = cnd ? 1 : 2;
69+
int a2 = cnd ? 1 : 2;
70+
int a3 = cnd ? 1 : 2;
71+
int a4 = cnd ? 1 : 2;
72+
int a5 = cnd ? 1 : 2;
73+
int a6 = cnd ? 1 : 2;
74+
int a7 = cnd ? 1 : 2;
75+
int b0 = a0 + a1;
76+
int b1 = a2 + a3;
77+
int b2 = a4 + a5;
78+
int b3 = a6 + a7;
79+
int c0 = b0 + b1;
80+
int c1 = b2 + b3;
81+
int d = c0 + c1;
82+
return d;
83+
}
84+
85+
// DAG-shaped functional test.
86+
static long testDAG(boolean cnd) {
87+
int a0 = cnd ? 1 : 2;
88+
int a1 = cnd ? 1 : 2;
89+
int a2 = cnd ? 1 : 2;
90+
int a3 = cnd ? 1 : 2;
91+
int b0 = a0 + a1;
92+
int b1 = a1 + a2;
93+
int b2 = a2 + a3;
94+
int c0 = b0 + b1;
95+
int c1 = b1 + b2;
96+
int d = c0 + c1;
97+
return d;
98+
}
99+
100+
// Chain-shaped stress test. Before fixing bug 8254317, this test would
101+
// result in an out-of-memory error after minutes running.
102+
static long testStress1(boolean cnd) {
103+
// C2 infers a finite, small value range for a. Note that there are
104+
// different ways to achieve this, for example a might take the value of
105+
// the induction variable in an outer counted loop.
106+
int a = cnd ? 1 : 2;
107+
// C2 fully unrolls this loop, creating a long chain of AddIs.
108+
for (int i = 0; i < 28; i++) {
109+
a = a + a;
110+
}
111+
// C2 places a ConvI2L at the end of the AddI chain.
112+
return a;
113+
}
114+
115+
// DAG-shaped stress test. Before fixing bug 8254317, this test would result
116+
// in an out-of-memory error after minutes running.
117+
static long testStress2(boolean cnd) {
118+
int a = cnd ? 1 : 2;
119+
int b = a;
120+
int c = a + a;
121+
for (int i = 0; i < 20; i++) {
122+
b = b + c;
123+
c = b + c;
124+
}
125+
int d = b + c;
126+
return d;
127+
}
128+
129+
public static void main(String[] args) {
130+
// We use a random number generator to avoid constant propagation in C2
131+
// and produce a variable ("a" in the different tests) with a finite,
132+
// small value range.
133+
Random rnd = new Random();
134+
switch(args[0]) {
135+
case "functional":
136+
// Small, functional tests.
137+
for (int i = 0; i < N; i++) {
138+
boolean cnd = rnd.nextBoolean();
139+
Asserts.assertEQ(testChain(cnd), cnd ? 8L : 16L);
140+
Asserts.assertEQ(testTree(cnd), cnd ? 8L : 16L);
141+
Asserts.assertEQ(testDAG(cnd), cnd ? 8L : 16L);
142+
}
143+
break;
144+
case "stress1":
145+
// Chain-shaped stress test.
146+
for (int i = 0; i < N; i++) {
147+
boolean cnd = rnd.nextBoolean();
148+
Asserts.assertEQ(testStress1(cnd),
149+
cnd ? 268435456L : 536870912L);
150+
}
151+
break;
152+
case "stress2":
153+
// DAG-shaped stress test.
154+
for (int i = 0; i < N; i++) {
155+
boolean cnd = rnd.nextBoolean();
156+
Asserts.assertEQ(testStress2(cnd),
157+
cnd ? 701408733L : 1402817466L);
158+
}
159+
break;
160+
default:
161+
System.out.println("invalid mode");
162+
}
163+
}
164+
}

0 commit comments

Comments
 (0)