Skip to content

Commit 1ebc25b

Browse files
mur47x111mur47x111
committed
Address comments.
1 parent c6808b5 commit 1ebc25b

File tree

10 files changed

+137
-143
lines changed

10 files changed

+137
-143
lines changed

‎compiler/src/org.graalvm.compiler.core.aarch64/src/org/graalvm/compiler/core/aarch64/AArch64NodeMatchRules.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it

‎compiler/src/org.graalvm.compiler.jtt/src/org/graalvm/compiler/jtt/optimize/Narrow_byte05.java‎

Lines changed: 0 additions & 74 deletions
This file was deleted.

‎compiler/src/org.graalvm.compiler.nodes.test/src/org/graalvm/compiler/nodes/test/NarrowPreservesOrderTest.java‎

Lines changed: 67 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -26,24 +26,21 @@
2626

2727
import static org.graalvm.compiler.core.common.calc.CanonicalCondition.BT;
2828
import static org.graalvm.compiler.core.common.calc.CanonicalCondition.LT;
29-
import static org.graalvm.compiler.core.test.GraalCompilerTest.getInitialOptions;
3029
import static org.junit.Assert.assertEquals;
30+
import static org.junit.Assert.assertTrue;
3131

32+
import org.graalvm.compiler.core.common.NumUtil;
3233
import org.graalvm.compiler.core.common.calc.CanonicalCondition;
3334
import org.graalvm.compiler.core.common.type.IntegerStamp;
3435
import org.graalvm.compiler.core.common.type.Stamp;
3536
import org.graalvm.compiler.core.common.type.StampFactory;
3637
import org.graalvm.compiler.core.common.type.StampPair;
37-
import org.graalvm.compiler.debug.DebugContext;
3838
import org.graalvm.compiler.graph.test.GraphTest;
3939
import org.graalvm.compiler.nodes.ParameterNode;
40-
import org.graalvm.compiler.nodes.StructuredGraph;
41-
import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions;
4240
import org.graalvm.compiler.nodes.calc.NarrowNode;
43-
import org.graalvm.compiler.options.OptionValues;
44-
import org.junit.Before;
4541
import org.junit.Test;
4642

43+
import jdk.vm.ci.code.CodeUtil;
4744
import jdk.vm.ci.meta.JavaKind;
4845

4946
/**
@@ -52,80 +49,91 @@
5249
*/
5350
public class NarrowPreservesOrderTest extends GraphTest {
5451

55-
private StructuredGraph graph;
52+
private static IntegerStamp signExtend(Stamp stamp, int bits) {
53+
assertTrue(stamp instanceof IntegerStamp);
54+
IntegerStamp integerStamp = (IntegerStamp) stamp;
55+
return IntegerStamp.create(bits, integerStamp.lowerBound(), integerStamp.upperBound());
56+
}
57+
58+
private static IntegerStamp zeroExtend(Stamp stamp, int bits) {
59+
assertTrue(stamp instanceof IntegerStamp);
60+
IntegerStamp integerStamp = (IntegerStamp) stamp;
61+
return IntegerStamp.create(bits, integerStamp.unsignedLowerBound(), integerStamp.unsignedUpperBound());
62+
}
63+
64+
private static IntegerStamp forConstantInt(long cst) {
65+
return IntegerStamp.create(32, cst, cst);
66+
}
5667

57-
@Before
58-
public void before() {
59-
OptionValues options = getInitialOptions();
60-
DebugContext debug = getDebug(options);
61-
graph = new StructuredGraph.Builder(options, debug, AllowAssumptions.YES).build();
68+
private static IntegerStamp forConstantLong(long cst) {
69+
return IntegerStamp.create(64, cst, cst);
6270
}
6371

64-
private void testPreserveOrder(Stamp inputStamp, int resultBits, CanonicalCondition cond, boolean expected) {
72+
private static void testPreserveOrder(Stamp inputStamp, int resultBits, CanonicalCondition cond, boolean expected) {
6573
ParameterNode input = new ParameterNode(0, StampPair.createSingle(inputStamp));
6674
NarrowNode narrow = new NarrowNode(input, resultBits);
6775
assertEquals(expected, narrow.preservesOrder(cond));
6876
}
6977

7078
@Test
7179
public void testBoolean() {
72-
testPreserveOrder(IntegerStamp.create(32, 0, 0), 1, LT, true);
73-
testPreserveOrder(IntegerStamp.create(32, 0, 0), 1, BT, true);
74-
testPreserveOrder(IntegerStamp.create(32, 1, 1), 1, LT, false);
75-
testPreserveOrder(IntegerStamp.create(32, 1, 1), 1, BT, true);
76-
testPreserveOrder(IntegerStamp.create(32, 0, 1), 1, LT, false);
77-
testPreserveOrder(IntegerStamp.create(32, 0, 1), 1, BT, true);
78-
79-
testPreserveOrder(IntegerStamp.create(32, 0xFFFFFF80, 0x7F), 1, LT, false);
80-
testPreserveOrder(IntegerStamp.create(32, 0xFFFFFF80, 0x7F), 1, BT, false);
80+
testPreserveOrder(forConstantInt(0), 1, LT, true);
81+
testPreserveOrder(forConstantInt(0), 1, BT, true);
82+
testPreserveOrder(forConstantInt(1), 1, LT, false);
83+
testPreserveOrder(forConstantInt(1), 1, BT, true);
84+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Boolean), 32), 1, LT, false);
85+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Boolean), 32), 1, BT, true);
86+
87+
testPreserveOrder(StampFactory.forKind(JavaKind.Byte), 1, LT, false);
88+
testPreserveOrder(StampFactory.forKind(JavaKind.Byte), 1, BT, false);
8189
}
8290

8391
@Test
8492
public void testByte() {
85-
testPreserveOrder(IntegerStamp.create(32, 0, 0), 8, LT, true);
86-
testPreserveOrder(IntegerStamp.create(32, 0, 0), 8, BT, true);
87-
testPreserveOrder(IntegerStamp.create(32, 0x7F, 0x7F), 8, LT, true);
88-
testPreserveOrder(IntegerStamp.create(32, 0x7F, 0x7F), 8, BT, true);
89-
testPreserveOrder(IntegerStamp.create(32, 0xFF, 0xFF), 8, LT, false);
90-
testPreserveOrder(IntegerStamp.create(32, 0xFF, 0xFF), 8, BT, true);
91-
testPreserveOrder(IntegerStamp.create(32, 0xFFFFFF80, 0x7F), 8, LT, true);
92-
testPreserveOrder(IntegerStamp.create(32, 0xFFFFFF80, 0x7F), 8, BT, true);
93-
testPreserveOrder(IntegerStamp.create(32, 0, 0xFF), 8, LT, false);
94-
testPreserveOrder(IntegerStamp.create(32, 0, 0xFF), 8, BT, true);
95-
96-
testPreserveOrder(IntegerStamp.create(32, 0xFFFF8000, 0x7FFF), 8, LT, false);
97-
testPreserveOrder(IntegerStamp.create(32, 0xFFFF8000, 0x7FFF), 8, BT, false);
93+
testPreserveOrder(forConstantInt(0), 8, LT, true);
94+
testPreserveOrder(forConstantInt(0), 8, BT, true);
95+
testPreserveOrder(forConstantInt(CodeUtil.maxValue(8)), 8, LT, true);
96+
testPreserveOrder(forConstantInt(CodeUtil.maxValue(8)), 8, BT, true);
97+
testPreserveOrder(forConstantInt(NumUtil.maxValueUnsigned(8)), 8, LT, false);
98+
testPreserveOrder(forConstantInt(NumUtil.maxValueUnsigned(8)), 8, BT, true);
99+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Byte), 32), 8, LT, true);
100+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Byte), 32), 8, BT, true);
101+
testPreserveOrder(zeroExtend(StampFactory.forUnsignedInteger(8), 32), 8, LT, false);
102+
testPreserveOrder(zeroExtend(StampFactory.forUnsignedInteger(8), 32), 8, BT, true);
103+
104+
testPreserveOrder(StampFactory.forKind(JavaKind.Short), 8, LT, false);
105+
testPreserveOrder(StampFactory.forKind(JavaKind.Short), 8, BT, false);
98106
}
99107

100108
@Test
101109
public void testShort() {
102-
testPreserveOrder(IntegerStamp.create(32, 0, 0), 16, LT, true);
103-
testPreserveOrder(IntegerStamp.create(32, 0, 0), 16, BT, true);
104-
testPreserveOrder(IntegerStamp.create(32, 0x7FFF, 0x7FFF), 16, LT, true);
105-
testPreserveOrder(IntegerStamp.create(32, 0x7FFF, 0x7FFF), 16, BT, true);
106-
testPreserveOrder(IntegerStamp.create(32, 0xFFFF, 0xFFFF), 16, LT, false);
107-
testPreserveOrder(IntegerStamp.create(32, 0xFFFF, 0xFFFF), 16, BT, true);
108-
testPreserveOrder(IntegerStamp.create(32, 0xFFFF8000, 0x7FFF), 16, LT, true);
109-
testPreserveOrder(IntegerStamp.create(32, 0xFFFF8000, 0x7FFF), 16, BT, true);
110-
testPreserveOrder(IntegerStamp.create(32, 0, 0xFFFF), 16, LT, false);
111-
testPreserveOrder(IntegerStamp.create(32, 0, 0xFFFF), 16, BT, true);
112-
113-
testPreserveOrder(StampFactory.intValue(), 16, LT, false);
114-
testPreserveOrder(StampFactory.intValue(), 16, BT, false);
110+
testPreserveOrder(forConstantInt(0), 16, LT, true);
111+
testPreserveOrder(forConstantInt(0), 16, BT, true);
112+
testPreserveOrder(forConstantInt(CodeUtil.maxValue(16)), 16, LT, true);
113+
testPreserveOrder(forConstantInt(CodeUtil.maxValue(16)), 16, BT, true);
114+
testPreserveOrder(forConstantInt(NumUtil.maxValueUnsigned(16)), 16, LT, false);
115+
testPreserveOrder(forConstantInt(NumUtil.maxValueUnsigned(16)), 16, BT, true);
116+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Short), 32), 16, LT, true);
117+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Short), 32), 16, BT, true);
118+
testPreserveOrder(zeroExtend(StampFactory.forUnsignedInteger(16), 32), 16, LT, false);
119+
testPreserveOrder(zeroExtend(StampFactory.forUnsignedInteger(16), 32), 16, BT, true);
120+
121+
testPreserveOrder(StampFactory.forKind(JavaKind.Int), 16, LT, false);
122+
testPreserveOrder(StampFactory.forKind(JavaKind.Int), 16, BT, false);
115123
}
116124

117125
@Test
118126
public void testInt() {
119-
testPreserveOrder(IntegerStamp.create(64, 0, 0), 32, LT, true);
120-
testPreserveOrder(IntegerStamp.create(64, 0, 0), 32, BT, true);
121-
testPreserveOrder(IntegerStamp.create(64, 0x7FFFFFFF, 0x7FFFFFFF), 32, LT, true);
122-
testPreserveOrder(IntegerStamp.create(64, 0x7FFFFFFF, 0x7FFFFFFF), 32, BT, true);
123-
testPreserveOrder(IntegerStamp.create(64, 0x00000000FFFFFFFFL, 0x00000000FFFFFFFFL), 32, LT, false);
124-
testPreserveOrder(IntegerStamp.create(64, 0x00000000FFFFFFFFL, 0x00000000FFFFFFFFL), 32, BT, true);
125-
testPreserveOrder(IntegerStamp.create(64, 0x80000000, 0x7FFFFFFF), 32, LT, true);
126-
testPreserveOrder(IntegerStamp.create(64, 0x80000000, 0x7FFFFFFF), 32, BT, true);
127-
testPreserveOrder(IntegerStamp.create(64, 0, 0x00000000FFFFFFFFL), 32, LT, false);
128-
testPreserveOrder(IntegerStamp.create(64, 0, 0x00000000FFFFFFFFL), 32, BT, true);
127+
testPreserveOrder(forConstantLong(0), 32, LT, true);
128+
testPreserveOrder(forConstantLong(0), 32, BT, true);
129+
testPreserveOrder(forConstantLong(CodeUtil.maxValue(32)), 32, LT, true);
130+
testPreserveOrder(forConstantLong(CodeUtil.maxValue(32)), 32, BT, true);
131+
testPreserveOrder(forConstantLong(NumUtil.maxValueUnsigned(32)), 32, LT, false);
132+
testPreserveOrder(forConstantLong(NumUtil.maxValueUnsigned(32)), 32, BT, true);
133+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Int), 64), 32, LT, true);
134+
testPreserveOrder(signExtend(StampFactory.forKind(JavaKind.Int), 64), 32, BT, true);
135+
testPreserveOrder(zeroExtend(StampFactory.forUnsignedInteger(32), 64), 32, LT, false);
136+
testPreserveOrder(zeroExtend(StampFactory.forUnsignedInteger(32), 64), 32, BT, true);
129137

130138
testPreserveOrder(StampFactory.forKind(JavaKind.Long), 32, LT, false);
131139
testPreserveOrder(StampFactory.forKind(JavaKind.Long), 32, BT, false);

‎compiler/src/org.graalvm.compiler.nodes.test/src/org/graalvm/compiler/nodes/test/NarrowTest.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Copyright (c) 2022, 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. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
package org.graalvm.compiler.nodes.test;
26+
27+
import org.graalvm.compiler.api.directives.GraalDirectives;
28+
import org.graalvm.compiler.core.test.GraalCompilerTest;
29+
import org.junit.Test;
30+
31+
public class NarrowTest extends GraalCompilerTest {
32+
33+
@Test
34+
public void testSnippet0() {
35+
test("snippet0", (char) 0);
36+
}
37+
38+
static boolean snippet0(char c0) {
39+
return -2 > (byte) ((byte) c0 / (byte) 2134864769);
40+
}
41+
42+
@Test
43+
public void testSnippet1() {
44+
test("snippet1", 50400L);
45+
}
46+
47+
static boolean snippet1(long l0) {
48+
if ((char) ((byte) l0) >= Character.MAX_VALUE) {
49+
return true;
50+
}
51+
GraalDirectives.blackhole((byte) l0);
52+
return false;
53+
}
54+
}

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/AddNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/IntegerConvertNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/NarrowNode.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -34,6 +34,7 @@
3434
import org.graalvm.compiler.core.common.type.IntegerStamp;
3535
import org.graalvm.compiler.core.common.type.PrimitiveStamp;
3636
import org.graalvm.compiler.core.common.type.Stamp;
37+
import org.graalvm.compiler.debug.GraalError;
3738
import org.graalvm.compiler.graph.NodeClass;
3839
import org.graalvm.compiler.lir.gen.ArithmeticLIRGeneratorTool;
3940
import org.graalvm.compiler.nodeinfo.NodeInfo;
@@ -82,13 +83,14 @@ protected IntegerConvertOp<Narrow> getOp(ArithmeticOpTable table) {
8283

8384
@Override
8485
protected IntegerConvertOp<?> getReverseOp(ArithmeticOpTable table) {
85-
assert isLossless();
86+
assert isSignedLossless() || isUnsignedLossless();
8687
return isSignedLossless() ? table.getSignExtend() : table.getZeroExtend();
8788
}
8889

8990
@Override
9091
public boolean isLossless() {
91-
return isSignedLossless() || isUnsignedLossless();
92+
// This is conservative as we don't know which compare operator is being used.
93+
return isSignedLossless() && isUnsignedLossless();
9294
}
9395

9496
private boolean isSignedLossless() {
@@ -127,8 +129,12 @@ public boolean preservesOrder(CanonicalCondition cond) {
127129
switch (cond) {
128130
case LT:
129131
return isSignedLossless();
132+
case EQ:
133+
case BT:
134+
// We may use signed stamps to represent unsigned integers.
135+
return isSignedLossless() || isUnsignedLossless();
130136
default:
131-
return isLossless();
137+
throw GraalError.shouldNotReachHere("Unsupported canonical condition.");
132138
}
133139
}
134140

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/SignExtendNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/ZeroExtendNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/loop/DerivedOffsetInductionVariable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it

0 commit comments

Comments
 (0)