stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8290910: Wrong memory state is picked in SuperWord::co_locate_pack()

Browse Source 
Reviewed-by: chagedorn, kvn
This commit is contained in:
Fei Gao 2022-09-23 01:26:21 +00:00 committed by Ningsheng Jian
parent f3ba332fdc
commit a4dc035a97
3 changed files with 213 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/hotspot/share/opto/superword.cpp
View File

@ -2363,7 +2363,8 @@ void SuperWord::co_locate_pack(Node_List* pk) {
// pick the memory state of the last load.
Node* SuperWord::pick_mem_state(Node_List* pk) {
Node* first_mem = find_first_mem_state(pk);
Node* last_mem = find_last_mem_state(pk, first_mem);
bool is_dependent = false;
Node* last_mem = find_last_mem_state(pk, first_mem, is_dependent);
for (uint i = 0; i < pk->size(); i++) {
Node* ld = pk->at(i);
@ -2371,12 +2372,18 @@ Node* SuperWord::pick_mem_state(Node_List* pk) {
assert(current->is_Mem() && in_bb(current), "unexpected memory");
assert(current != first_mem, "corrupted memory graph");
if (!independent(current, ld)) {
// A later store depends on this load, pick the memory state of the first load. This can happen, for example,
// if a load pack has interleaving stores that are part of a store pack which, however, is removed at the pack
// filtering stage. This leaves us with only a load pack for which we cannot take the memory state of the
// last load as the remaining unvectorized stores could interfere since they have a dependency to the loads.
// A later unvectorized store depends on this load, pick the memory state of the first load. This can happen,
// for example, if a load pack has interleaving stores that are part of a store pack which, however, is removed
// at the pack filtering stage. This leaves us with only a load pack for which we cannot take the memory state
// of the last load as the remaining unvectorized stores could interfere since they have a dependency to the loads.
// Some stores could be executed before the load vector resulting in a wrong result. We need to take the
// memory state of the first load to prevent this.
if (my_pack(current) != NULL && is_dependent) {
// For vectorized store pack, when the load pack depends on
// some memory operations locating after first_mem, we still
// take the memory state of the last load.
continue;
}
return first_mem;
}
}
@ -2407,13 +2414,17 @@ Node* SuperWord::find_first_mem_state(Node_List* pk) {
// the memory graph from the loads of the pack to the memory of
// the first load. If we encounter the memory of the current last
// load, then we started from further down in the memory graph and
// the load we started from is the last load.
Node* SuperWord::find_last_mem_state(Node_List* pk, Node* first_mem) {
// the load we started from is the last load. At the same time, the
// function also helps determine if some loads in the pack depend on
// early memory operations which locate after first_mem.
Node* SuperWord::find_last_mem_state(Node_List* pk, Node* first_mem, bool &is_dependent) {
Node* last_mem = pk->at(0)->in(MemNode::Memory);
for (uint i = 0; i < pk->size(); i++) {
Node* ld = pk->at(i);
for (Node* current = ld->in(MemNode::Memory); current != first_mem; current = current->in(MemNode::Memory)) {
assert(current->is_Mem() && in_bb(current), "unexpected memory");
// Determine if the load pack is dependent on some memory operations locating after first_mem.
is_dependent |= !independent(current, ld);
if (current->in(MemNode::Memory) == last_mem) {
last_mem = ld->in(MemNode::Memory);
}

2
src/hotspot/share/opto/superword.hpp
View File

@ -548,7 +548,7 @@ class SuperWord : public ResourceObj {
void co_locate_pack(Node_List* p);
Node* pick_mem_state(Node_List* pk);
Node* find_first_mem_state(Node_List* pk);
Node* find_last_mem_state(Node_List* pk, Node* first_mem);
Node* find_last_mem_state(Node_List* pk, Node* first_mem, bool &is_dependent);
// Convert packs into vector node operations
bool output();

194
test/hotspot/jtreg/compiler/loopopts/superword/TestPickLastMemoryState.java Normal file
View File

@ -0,0 +1,194 @@
/*
* Copyright (c) 2022, Arm Limited. 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.
*/
/**
* @test
* @requires vm.compiler2.enabled
* @bug 8290910 8293216
* @summary Test which needs to select the memory state of the last load in a load pack in SuperWord::co_locate_pack.
*
* @run main/othervm -Xcomp -XX:CompileOnly=compiler/loopopts/superword/TestPickLastMemoryState
* -Xbatch -XX:MaxVectorSize=16 compiler.loopopts.superword.TestPickLastMemoryState
* @run main/othervm -Xcomp -XX:CompileOnly=compiler/loopopts/superword/TestPickLastMemoryState
* -Xbatch -XX:MaxVectorSize=32 compiler.loopopts.superword.TestPickLastMemoryState
* @run main/othervm -Xcomp -XX:CompileOnly=compiler/loopopts/superword/TestPickLastMemoryState
* -Xbatch compiler.loopopts.superword.TestPickLastMemoryState
*/
package compiler.loopopts.superword;
public class TestPickLastMemoryState {
static final int N = 400;
static final int M = 32;
static long lArrFld[] = new long[N];
static long iMeth_check_sum;
static float a[] = new float[M];
static void f() {
int b[] = new int[M];
for (int h = 1; h < 32; h++) {
a[h] = b[h - 1]--;
b[h]--;
}
boolean c[] = new boolean[M];
}
static void test0() throws Exception {
f();
double s = checkSum(a);
System.out.println(s);
if (s < -31 || s > -29) {
throw new Exception("expected s: -30, actual s: " + s);
}
}
static void test1() {
int i1 , i2 = -222, iArr[] = new int[N];
init(iArr, 212);
// For the following loop, statement 1 can be vectorized but statement 2 can't. When
// finding the memory state for the LoadI pack, we cannot pick the memory state from
// the first load as the LoadI vector operation must load the memory after iArr writes
// 'iArr[i1 + 1] - (i2++)' to 'iArr[i1 + 1]'. We must take the memory state of the last
// load where we have assigned new values ('iArr[i1 + 1] - (i2++)') to the iArr array.
for (i1 = 6; i1 < 227; i1++) {
iArr[i1] += lArrFld[i1]++; // statement 1
iArr[i1 + 1] -= (i2++); // statement 2
}
iMeth_check_sum += checkSum(iArr);
}
static void test2() {
int i1 , i2 = -222, iArr[] = new int[N];
init(iArr, 212);
for (i1 = 6; i1 < 227; i1++) {
iArr[i1] += lArrFld[i1]++;
iArr[i1 + 2] -= (i2++);
}
iMeth_check_sum += checkSum(iArr);
}
static void test3() {
int i1 , i2 = -222, iArr[] = new int[N];
init(iArr, 212);
for (i1 = 6; i1 < 227; i1++) {
iArr[i1-2] += lArrFld[i1]++;
iArr[i1] -= (i2++);
}
iMeth_check_sum += checkSum(iArr);
}
static void test4() {
int i1 , i2 = -222, iArr[] = new int[N];
init(iArr, 212);
for (i1 = 6; i1 < 227; i1++) {
iArr[i1-3] += lArrFld[i1]++;
iArr[i1] -= (i2++);
}
iMeth_check_sum += checkSum(iArr);
}
static void test5() {
int i1 , i2 = -222, i3 = -100, iArr[] = new int[N];
init(iArr, 212);
for (i1 = 6; i1 < 227; i1++) {
iArr[i1] += lArrFld[i1]++;
iArr[i1+2] -= (i3++);
iArr[i1+16] -= (i2++);
}
iMeth_check_sum += checkSum(iArr);
}
static void test6() {
int i1 , i2 = -222, i3 = -100, iArr[] = new int[N];
init(iArr, 212);
for (i1 = 6; i1 < 227; i1++) {
iArr[i1] += lArrFld[i1]++;
iArr[i1+1] -= (i3++);
iArr[i1+32] -= (i2++);
}
iMeth_check_sum += checkSum(iArr);
}
static void init(int[] a, int seed) {
for (int j = 0; j < a.length; j++) {
a[j] = (j % 2 == 0) ? seed + j : seed - j;
}
}
static long checkSum(int[] a) {
long sum = 0;
for (int j = 0; j < a.length; j++) {
sum += (a[j] / (j + 1) + a[j] % (j + 1));
}
return sum;
}
static double checkSum(float[] a) {
double sum = 0;
for (int j = 0; j < a.length; j++) {
sum += a[j];
}
return sum;
}
static void reset() {
for (int i = 0; i < N; i++) {
lArrFld[i] = 0;
}
iMeth_check_sum = 0;
}
public static void main(String[] strArr) throws Exception {
test0();
test0();
for (int i = 0; i < 5_000; i++) {
reset();
test1();
if (iMeth_check_sum != 22154) {
throw new RuntimeException("iMeth wrong result at test1: " + iMeth_check_sum);
}
test2();
if (iMeth_check_sum != 44246) {
throw new RuntimeException("iMeth wrong result at test2: " + iMeth_check_sum);
}
test3();
if (iMeth_check_sum != 66171) {
throw new RuntimeException("iMeth wrong result at test3: " + iMeth_check_sum);
}
test4();
if (iMeth_check_sum != 88309) {
throw new RuntimeException("iMeth wrong result at test4: " + iMeth_check_sum);
}
test5();
if (iMeth_check_sum != 109251) {
throw new RuntimeException("iMeth wrong result at test5: " + iMeth_check_sum);
}
test6();
if (iMeth_check_sum != 130073) {
throw new RuntimeException("iMeth wrong result at test6: " + iMeth_check_sum);
}
}
}
}