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

8291669: [REDO] Fix array range check hoisting for some scaled loop iv

Browse Source 
Reviewed-by: roland, thartmann
This commit is contained in:
Pengfei Li 2022-09-14 14:20:39 +00:00
parent 7f3250d71c
commit 211fab8d36
4 changed files with 279 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
src/hotspot/share/opto/loopTransform.cpp
View File

@ -2719,6 +2719,8 @@ bool PhaseIdealLoop::is_iv(Node* exp, Node* iv, BasicType bt) {
// | (LShiftX VIV[iv] ConI)
// | (ConvI2L SIV[iv]) -- a "short-scale" can occur here; note recursion
// | (SubX 0 SIV[iv]) -- same as MulX(iv, -scale); note recursion
// | (AddX SIV[iv] SIV[iv]) -- sum of two scaled iv; note recursion
// | (SubX SIV[iv] SIV[iv]) -- difference of two scaled iv; note recursion
// VIV[iv] = [either iv or its value converted; see is_iv() above]
// On success, the constant scale value is stored back to *p_scale.
// The value (*p_short_scale) reports if such a ConvI2L conversion was present.
@ -2780,25 +2782,74 @@ bool PhaseIdealLoop::is_scaled_iv(Node* exp, Node* iv, BasicType bt, jlong* p_sc
}
return true;
}
} else if (opc == Op_Sub(exp_bt) &&
exp->in(1)->find_integer_as_long(exp_bt, -1) == 0) {
jlong scale = 0;
if (depth == 0 && is_scaled_iv(exp->in(2), iv, exp_bt, &scale, p_short_scale, depth + 1)) {
// SubX(0, iv*K) => iv*(-K)
if (scale == min_signed_integer(exp_bt)) {
// This should work even if -K overflows, but let's not.
} else if (opc == Op_Add(exp_bt)) {
jlong scale_l = 0;
jlong scale_r = 0;
bool short_scale_l = false;
bool short_scale_r = false;
if (depth == 0 &&
is_scaled_iv(exp->in(1), iv, exp_bt, &scale_l, &short_scale_l, depth + 1) &&
is_scaled_iv(exp->in(2), iv, exp_bt, &scale_r, &short_scale_r, depth + 1)) {
// AddX(iv*K1, iv*K2) => iv*(K1+K2)
jlong scale_sum = java_add(scale_l, scale_r);
if (scale_sum > max_signed_integer(exp_bt) || scale_sum <= min_signed_integer(exp_bt)) {
// This logic is shared by int and long. For int, the result may overflow
// as we use jlong to compute so do the check here. Long result may also
// overflow but that's fine because result wraps.
return false;
}
scale = java_multiply(scale, (jlong)-1);
if (p_scale != NULL) {
*p_scale = scale;
*p_scale = scale_sum;
}
if (p_short_scale != NULL) {
// (ConvI2L (MulI iv K)) can be 64-bit linear if iv is kept small enough...
*p_short_scale = *p_short_scale || (exp_bt != bt && scale != 1);
*p_short_scale = short_scale_l && short_scale_r;
}
return true;
}
} else if (opc == Op_Sub(exp_bt)) {
if (exp->in(1)->find_integer_as_long(exp_bt, -1) == 0) {
jlong scale = 0;
if (depth == 0 && is_scaled_iv(exp->in(2), iv, exp_bt, &scale, p_short_scale, depth + 1)) {
// SubX(0, iv*K) => iv*(-K)
if (scale == min_signed_integer(exp_bt)) {
// This should work even if -K overflows, but let's not.
return false;
}
scale = java_multiply(scale, (jlong)-1);
if (p_scale != NULL) {
*p_scale = scale;
}
if (p_short_scale != NULL) {
// (ConvI2L (MulI iv K)) can be 64-bit linear if iv is kept small enough...
*p_short_scale = *p_short_scale || (exp_bt != bt && scale != 1);
}
return true;
}
} else {
jlong scale_l = 0;
jlong scale_r = 0;
bool short_scale_l = false;
bool short_scale_r = false;
if (depth == 0 &&
is_scaled_iv(exp->in(1), iv, exp_bt, &scale_l, &short_scale_l, depth + 1) &&
is_scaled_iv(exp->in(2), iv, exp_bt, &scale_r, &short_scale_r, depth + 1)) {
// SubX(iv*K1, iv*K2) => iv*(K1-K2)
jlong scale_diff = java_subtract(scale_l, scale_r);
if (scale_diff > max_signed_integer(exp_bt) || scale_diff <= min_signed_integer(exp_bt)) {
// This logic is shared by int and long. For int, the result may
// overflow as we use jlong to compute so do the check here. Long
// result may also overflow but that's fine because result wraps.
return false;
}
if (p_scale != NULL) {
*p_scale = scale_diff;
}
if (p_short_scale != NULL) {
*p_short_scale = short_scale_l && short_scale_r;
}
return true;
}
}
}
// We could also recognize (iv*K1)*K2, even with overflow, but let's not.
return false;

64
test/hotspot/jtreg/compiler/rangechecks/RangeCheckEliminationScaleNotOne.java
View File

@ -27,6 +27,7 @@
* @summary C2: range check elimination may allow illegal out of bound access
*
* @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:-UseLoopPredicate RangeCheckEliminationScaleNotOne
* @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:+UseLoopPredicate RangeCheckEliminationScaleNotOne
*
*/
@ -73,6 +74,46 @@ public class RangeCheckEliminationScaleNotOne {
throw new RuntimeException("no AIOOB exception");
}
}
{
int[] array = new int[299];
boolean[] flags = new boolean[100];
Arrays.fill(flags, true);
flags[0] = false;
flags[1] = false;
for (int i = 0; i < 20_000; i++) {
test3(100, array, 1, flags);
}
boolean ex = false;
try {
test3(100, array, -8, flags);
} catch (ArrayIndexOutOfBoundsException aie) {
ex = true;
}
if (!ex) {
throw new RuntimeException("no AIOOB exception");
}
}
{
int[] array = new int[1000];
boolean[] flags = new boolean[100];
Arrays.fill(flags, true);
flags[0] = false;
flags[1] = false;
for (int i = 0; i < 20_000; i++) {
test4(100, array, 302, flags);
}
boolean ex = false;
try {
test4(100, array, 320, flags);
} catch (ArrayIndexOutOfBoundsException aie) {
ex = true;
}
if (!ex) {
throw new RuntimeException("no AIOOB exception");
}
}
}
private static int test1(int stop, int[] array, int offset, boolean[] flags) {
@ -86,7 +127,6 @@ public class RangeCheckEliminationScaleNotOne {
return res;
}
private static int test2(int stop, int[] array, int offset, boolean[] flags) {
if (array == null) {}
int res = 0;
@ -97,4 +137,26 @@ public class RangeCheckEliminationScaleNotOne {
}
return res;
}
private static int test3(int stop, int[] array, int offset, boolean[] flags) {
if (array == null) {}
int res = 0;
for (int i = 0; i < stop; i++) {
if (flags[i]) {
res += array[3 * i + offset];
}
}
return res;
}
private static int test4(int stop, int[] array, int offset, boolean[] flags) {
if (array == null) {}
int res = 0;
for (int i = 0; i < stop; i++) {
if (flags[i]) {
res += array[7 * i + offset];
}
}
return res;
}
}

100
test/hotspot/jtreg/compiler/rangechecks/TestRangeCheckHoistingScaledIV.java Normal file
View File

@ -0,0 +1,100 @@
/*
* 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
* @bug 8289996
* @summary Test range check hoisting for some scaled iv at array index
* @library /test/lib /
* @requires vm.debug & vm.compiler2.enabled & (os.simpleArch == "x64" | os.arch == "aarch64")
* @modules jdk.incubator.vector
* @compile --enable-preview -source ${jdk.version} TestRangeCheckHoistingScaledIV.java
* @run main/othervm --enable-preview compiler.rangechecks.TestRangeCheckHoistingScaledIV
*/
package compiler.rangechecks;
import java.lang.foreign.MemorySegment;
import java.nio.ByteOrder;
import jdk.incubator.vector.ByteVector;
import jdk.incubator.vector.VectorSpecies;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;
public class TestRangeCheckHoistingScaledIV {
// Inner class for test loops
class Launcher {
private static final int SIZE = 16000;
private static final VectorSpecies<Byte> SPECIES = ByteVector.SPECIES_64;
private static final ByteOrder ORDER = ByteOrder.nativeOrder();
private static byte[] ta = new byte[SIZE];
private static byte[] tb = new byte[SIZE];
private static MemorySegment sa = MemorySegment.ofArray(ta);
private static MemorySegment sb = MemorySegment.ofArray(tb);
private static int count = 789;
// Normal array accesses with int range checks
public static void scaledIntIV() {
for (int i = 0; i < count; i += 2) {
tb[7 * i] = ta[3 * i];
}
}
// Memory segment accesses with long range checks
public static void scaledLongIV() {
for (long l = 0; l < count; l += 64) {
ByteVector v = ByteVector.fromMemorySegment(SPECIES, sa, l * 6, ORDER);
v.intoMemorySegment(sb, l * 15, ORDER);
}
}
public static void main(String[] args) {
for (int i = 0; i < 20000; i++) {
scaledIntIV();
scaledLongIV();
}
}
}
public static void main(String[] args) throws Exception {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
"--enable-preview", "--add-modules", "jdk.incubator.vector",
"-Xbatch", "-XX:+TraceLoopPredicate", Launcher.class.getName());
OutputAnalyzer analyzer = new OutputAnalyzer(pb.start());
analyzer.shouldHaveExitValue(0);
analyzer.outputTo(System.out);
// Check if int range checks are hoisted
analyzer.stdoutShouldContain("rc_predicate init * 3");
analyzer.stdoutShouldContain("rc_predicate init * 7");
// Check if long range checks are hoisted
analyzer.stdoutShouldContain("rc_predicate init * 6");
analyzer.stdoutShouldContain("rc_predicate init * 15");
}
}

54
test/micro/org/openjdk/bench/vm/compiler/RangeCheckHoisting.java Normal file
View File

@ -0,0 +1,54 @@
/*
* 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.
*/
package org.openjdk.bench.vm.compiler;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
@State(Scope.Benchmark)
public class RangeCheckHoisting {
private static final int SIZE = 65536;
@Param("6789") private int count;
private static int[] a = new int[SIZE];
private static int[] b = new int[SIZE];
@Benchmark
public void ivScaled3() {
for (int i = 0; i < count; i++) {
b[3 * i] = a[3 * i];
}
}
@Benchmark
public void ivScaled7() {
for (int i = 0; i < count; i++) {
b[7 * i] = a[7 * i];
}
}
}