Skip to content

Commit 3db9a5a

Browse files
committed
8373502: C2 SuperWord: speculative check uses VPointer variable was pinned after speculative check, leading to bad graph
Reviewed-by: mhaessig, chagedorn Backport-of: 00050f8
1 parent cece06f commit 3db9a5a

File tree

3 files changed

+112
-0
lines changed

3 files changed

+112
-0
lines changed

src/hotspot/share/opto/vectorization.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,29 @@ bool VPointer::can_make_speculative_aliasing_check_with(const VPointer& other) c
10601060
return false;
10611061
}
10621062

1063+
// The speculative check also needs to create the pointer expressions for both
1064+
// VPointers. We must check that we can do that, i.e. that all variables of the
1065+
// VPointers are available at the speculative check (and not just pre-loop invariant).
1066+
if (!this->can_make_pointer_expression_at_speculative_check()) {
1067+
#ifdef ASSERT
1068+
if (_vloop.is_trace_speculative_aliasing_analysis()) {
1069+
tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: not all variables of VPointer are avaialbe at speculative check!");
1070+
this->print_on(tty);
1071+
}
1072+
#endif
1073+
return false;
1074+
}
1075+
1076+
if (!other.can_make_pointer_expression_at_speculative_check()) {
1077+
#ifdef ASSERT
1078+
if (_vloop.is_trace_speculative_aliasing_analysis()) {
1079+
tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: not all variables of VPointer are avaialbe at speculative check!");
1080+
other.print_on(tty);
1081+
}
1082+
#endif
1083+
return false;
1084+
}
1085+
10631086
return true;
10641087
}
10651088

@@ -1147,6 +1170,8 @@ BoolNode* VPointer::make_speculative_aliasing_check_with(const VPointer& other,
11471170
Node* main_init = new ConvL2INode(main_initL);
11481171
phase->register_new_node_with_ctrl_of(main_init, pre_init);
11491172

1173+
assert(vp1.can_make_pointer_expression_at_speculative_check(), "variables must be available early enough to avoid cycles");
1174+
assert(vp2.can_make_pointer_expression_at_speculative_check(), "variables must be available early enough to avoid cycles");
11501175
Node* p1_init = vp1.make_pointer_expression(main_init, ctrl);
11511176
Node* p2_init = vp2.make_pointer_expression(main_init, ctrl);
11521177
Node* size1 = igvn.longcon(vp1.size());

src/hotspot/share/opto/vectorization.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,6 +1188,22 @@ class VPointer : public ArenaObj {
11881188
return true;
11891189
}
11901190

1191+
// We already know that all non-iv summands are pre loop invariant.
1192+
// See init_are_non_iv_summands_pre_loop_invariant
1193+
// That is good enough for alignment computations in the pre-loop limit. But it is not
1194+
// sufficient if we want to use the variables of the VPointer at the speculative check,
1195+
// which is further up before the pre-loop.
1196+
bool can_make_pointer_expression_at_speculative_check() const {
1197+
bool success = true;
1198+
mem_pointer().for_each_non_empty_summand([&] (const MemPointerSummand& s) {
1199+
Node* variable = s.variable();
1200+
if (variable != _vloop.iv() && !_vloop.is_available_for_speculative_check(variable)) {
1201+
success = false;
1202+
}
1203+
});
1204+
return success;
1205+
}
1206+
11911207
// In the pointer analysis, and especially the AlignVector analysis, we assume that
11921208
// stride and scale are not too large. For example, we multiply "iv_scale * iv_stride",
11931209
// and assume that this does not overflow the int range. We also take "abs(iv_scale)"
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Copyright (c) 2025, 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 id=with-flags
26+
* @bug 8373502
27+
* @summary Test where a VPointer variable was pinned at the pre-loop, but not available at the
28+
* Auto_Vectorization_Check, and so it should not be used for the auto vectorization
29+
* aliasing check, to avoid a bad (circular) graph.
30+
* @run main/othervm
31+
* -XX:CompileCommand=compileonly,*TestAliasingCheckVPointerVariablesNotAvailable::test
32+
* -XX:-TieredCompilation
33+
* -Xcomp
34+
* ${test.main.class}
35+
*/
36+
37+
/*
38+
* @test id=vanilla
39+
* @bug 8373502
40+
* @run main ${test.main.class}
41+
*/
42+
43+
package compiler.loopopts.superword;
44+
45+
public class TestAliasingCheckVPointerVariablesNotAvailable {
46+
static int iFld;
47+
48+
public static void main(String[] strArr) {
49+
test();
50+
}
51+
52+
static void test() {
53+
int iArr[] = new int[400];
54+
boolean flag = false;
55+
for (int i = 6; i < 50000; i++) { // Trigger OSR
56+
try {
57+
int x = 234 / iFld;
58+
iFld = iArr[3];
59+
} catch (ArithmeticException a_e) {
60+
}
61+
for (int j = i; j < 2; j++) {
62+
if (flag) {
63+
iArr[j] = 117;
64+
} else {
65+
iArr[1] = 34;
66+
}
67+
iArr[1] += i;
68+
}
69+
}
70+
}
71+
}

0 commit comments

Comments
 (0)