Skip to content

Commit 4f3b7d8

Browse files
mur47x111mur47x111
committed
Address comments.
1 parent fed776e commit 4f3b7d8

File tree

4 files changed

+358
-23
lines changed

4 files changed

+358
-23
lines changed

‎compiler/src/org.graalvm.compiler.core.amd64/src/org/graalvm/compiler/core/amd64/AMD64NodeMatchRules.java‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -581,32 +581,34 @@ public ComplexMatchResult xorMemory(ValueNode value, LIRLowerableAccess access)
581581
}
582582

583583
private ComplexMatchResult emitMemoryConsumer(WriteNode write, AMD64Assembler.AMD64BinaryArithmetic arithmeticOp, ReadNode read, ValueNode value) {
584-
OperandSize size = getMemorySize(write);
585-
if (!size.isXmmType() && write.getAddress() == read.getAddress()) {
586-
if (value.isJavaConstant()) {
587-
long valueCst = value.asJavaConstant().asLong();
588-
if (NumUtil.isInt(valueCst)) {
589-
AMD64Assembler.AMD64MOp mop = AMD64ArithmeticLIRGenerator.getMOp(arithmeticOp, (int) valueCst);
590-
if (mop != null) {
591-
return builder -> {
592-
AMD64AddressValue addressValue = (AMD64AddressValue) operand(write.getAddress());
593-
builder.append(new AMD64UnaryConsumer.MemoryOp(mop, size, addressValue));
594-
return null;
595-
};
596-
} else {
597-
return builder -> {
598-
AMD64AddressValue addressValue = (AMD64AddressValue) operand(write.getAddress());
599-
builder.append(new AMD64BinaryConsumer.MemoryConstOp(arithmeticOp.getMIOpcode(size, NumUtil.isByte(valueCst)), size, addressValue, (int) valueCst, state(write)));
600-
return null;
601-
};
584+
if (getMemoryKind(write).isInteger() && !write.canDeoptimize() && !read.canDeoptimize()) {
585+
OperandSize size = getMemorySize(write);
586+
if (write.getAddress() == read.getAddress()) {
587+
if (value.isJavaConstant()) {
588+
long valueCst = value.asJavaConstant().asLong();
589+
if (NumUtil.isInt(valueCst)) {
590+
AMD64Assembler.AMD64MOp mop = AMD64ArithmeticLIRGenerator.getMOp(arithmeticOp, (int) valueCst);
591+
if (mop != null) {
592+
return builder -> {
593+
AMD64AddressValue addressValue = (AMD64AddressValue) operand(write.getAddress());
594+
builder.append(new AMD64UnaryConsumer.MemoryOp(mop, size, addressValue));
595+
return null;
596+
};
597+
} else {
598+
return builder -> {
599+
AMD64AddressValue addressValue = (AMD64AddressValue) operand(write.getAddress());
600+
builder.append(new AMD64BinaryConsumer.MemoryConstOp(arithmeticOp.getMIOpcode(size, NumUtil.isByte(valueCst)), size, addressValue, (int) valueCst, state(write)));
601+
return null;
602+
};
603+
}
602604
}
603605
}
606+
return builder -> {
607+
AMD64AddressValue addressValue = (AMD64AddressValue) operand(write.getAddress());
608+
builder.append(new AMD64BinaryConsumer.MemoryMROp(arithmeticOp.getMROpcode(size), size, addressValue, builder.getLIRGeneratorTool().asAllocatable(operand(value)), state(write)));
609+
return null;
610+
};
604611
}
605-
return builder -> {
606-
AMD64AddressValue addressValue = (AMD64AddressValue) operand(write.getAddress());
607-
builder.append(new AMD64BinaryConsumer.MemoryMROp(arithmeticOp.getMROpcode(size), size, addressValue, builder.getLIRGeneratorTool().asAllocatable(operand(value)), state(write)));
608-
return null;
609-
};
610612
}
611613
return null;
612614
}

‎compiler/src/org.graalvm.compiler.hotspot.amd64.test/src/org/graalvm/compiler/hotspot/amd64/test/AMD64ReadAfterWriteMatchTest.java‎

Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,325 @@
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.hotspot.amd64.test;
26+
27+
import static org.graalvm.compiler.phases.common.UseTrappingNullChecksPhase.Options.UseTrappingNullChecks;
28+
import static org.junit.Assume.assumeTrue;
29+
30+
import java.util.function.Predicate;
31+
32+
import org.graalvm.compiler.core.common.cfg.AbstractBlockBase;
33+
import org.graalvm.compiler.core.test.MatchRuleTest;
34+
import org.graalvm.compiler.lir.LIR;
35+
import org.graalvm.compiler.lir.LIRInstruction;
36+
import org.graalvm.compiler.lir.amd64.AMD64BinaryConsumer.MemoryConstOp;
37+
import org.graalvm.compiler.lir.amd64.AMD64BinaryConsumer.MemoryMROp;
38+
import org.graalvm.compiler.lir.amd64.AMD64UnaryConsumer.MemoryOp;
39+
import org.graalvm.compiler.options.OptionValues;
40+
import org.junit.Before;
41+
import org.junit.Test;
42+
43+
import jdk.vm.ci.amd64.AMD64;
44+
import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod;
45+
46+
public class AMD64ReadAfterWriteMatchTest extends MatchRuleTest {
47+
48+
@Before
49+
public void checkAMD64() {
50+
assumeTrue("skipping AMD64 specific test", getTarget().arch instanceof AMD64);
51+
((HotSpotResolvedJavaMethod) getResolvedJavaMethod("reset")).setNotInlinableOrCompilable();
52+
((HotSpotResolvedJavaMethod) getResolvedJavaMethod("getValue")).setNotInlinableOrCompilable();
53+
}
54+
55+
static int staticIntField = 0xBAADCAFE;
56+
57+
public static class A {
58+
int intField = 0xBAADCAFE;
59+
}
60+
61+
@BytecodeParserNeverInline
62+
static void reset() {
63+
staticIntField = 0xBAADCAFE;
64+
}
65+
66+
@BytecodeParserNeverInline
67+
static int getValue() {
68+
return staticIntField;
69+
}
70+
71+
@BytecodeParserNeverInline
72+
static void resetInstance(A a) {
73+
a.intField = 0xBAADCAFE;
74+
}
75+
76+
@BytecodeParserNeverInline
77+
static int getInstanceValue(A a) {
78+
return a.intField;
79+
}
80+
81+
public int compileAndCount(String method, OptionValues options, Predicate<LIRInstruction> filter) {
82+
compile(getResolvedJavaMethod(method), null, options != null ? options : getInitialOptions());
83+
LIR lir = getLIR();
84+
int count = 0;
85+
for (AbstractBlockBase<?> block : lir.codeEmittingOrder()) {
86+
for (LIRInstruction ins : lir.getLIRforBlock(block)) {
87+
if (filter.test(ins)) {
88+
count++;
89+
}
90+
}
91+
}
92+
return count;
93+
}
94+
95+
public static int snippetAdd1() {
96+
reset();
97+
staticIntField++;
98+
return getValue();
99+
}
100+
101+
@Test
102+
public void testSnippetAdd1() {
103+
int cnt = compileAndCount("snippetAdd1", null, ins -> ins instanceof MemoryOp && "INC".equals(((MemoryOp) ins).getOpcode().toString()));
104+
assumeTrue("INC is expected once in the LIR", cnt == 1);
105+
test("snippetAdd1");
106+
}
107+
108+
public static int snippetAddLargeConstant() {
109+
reset();
110+
staticIntField += 0xDEADBEEF;
111+
return getValue();
112+
}
113+
114+
@Test
115+
public void testSnippetAddLargeConstant() {
116+
int cnt = compileAndCount("snippetAddLargeConstant", null, ins -> ins instanceof MemoryConstOp && "ADD".equals(((MemoryConstOp) ins).getOpcode().toString()));
117+
assumeTrue("ADD is expected once in the LIR", cnt == 1);
118+
test("snippetAddLargeConstant");
119+
}
120+
121+
public static int snippetAddVariable(int i) {
122+
reset();
123+
staticIntField += i;
124+
return getValue();
125+
}
126+
127+
@Test
128+
public void testSnippetAddVariable() {
129+
int cnt = compileAndCount("snippetAddVariable", null, ins -> ins instanceof MemoryMROp && "ADD".equals(((MemoryMROp) ins).getOpcode().toString()));
130+
assumeTrue("ADD is expected once in the LIR", cnt == 1);
131+
test("snippetAddVariable", 1);
132+
test("snippetAddVariable", 0xDEADBEEF);
133+
}
134+
135+
public static int snippetInstanceFieldAdd1(A a) {
136+
resetInstance(a);
137+
a.intField++;
138+
return getInstanceValue(a);
139+
}
140+
141+
@Test
142+
public void testSnippetInstanceFieldAdd1() {
143+
OptionValues options = getInitialOptions();
144+
assumeTrue("UseTrappingNullChecks must be on", UseTrappingNullChecks.getValue(options));
145+
Predicate<LIRInstruction> predicate = ins -> ins instanceof MemoryOp && "INC".equals(((MemoryOp) ins).getOpcode().toString());
146+
int cnt = compileAndCount("snippetInstanceFieldAdd1", null, predicate);
147+
assumeTrue("INC is not expected in the LIR", cnt == 0);
148+
OptionValues disableUseTrappingNullChecks = new OptionValues(options, UseTrappingNullChecks, false);
149+
cnt = compileAndCount("snippetInstanceFieldAdd1", disableUseTrappingNullChecks, predicate);
150+
assumeTrue("INC is expected once in the LIR", cnt == 1);
151+
test("snippetInstanceFieldAdd1", disableUseTrappingNullChecks, new A());
152+
}
153+
154+
public static int snippetSub1() {
155+
reset();
156+
staticIntField--;
157+
return getValue();
158+
}
159+
160+
@Test
161+
public void testSnippetSub1() {
162+
int cnt = compileAndCount("snippetSub1", null, ins -> ins instanceof MemoryOp && "DEC".equals(((MemoryOp) ins).getOpcode().toString()));
163+
assumeTrue("DEC is expected once in the LIR", cnt == 1);
164+
test("snippetSub1");
165+
}
166+
167+
public static int snippetSubLargeConstant() {
168+
reset();
169+
staticIntField -= 0xDEADBEEF;
170+
return getValue();
171+
}
172+
173+
@Test
174+
public void testSnippetSubLargeConstant() {
175+
int cnt = compileAndCount("snippetSubLargeConstant", null, ins -> ins instanceof MemoryConstOp && "SUB".equals(((MemoryConstOp) ins).getOpcode().toString()));
176+
assumeTrue("SUB is expected once in the LIR", cnt == 1);
177+
test("snippetSubLargeConstant");
178+
}
179+
180+
public static int snippetSubVariable(int i) {
181+
reset();
182+
staticIntField -= i;
183+
return getValue();
184+
}
185+
186+
@Test
187+
public void testSnippetSubVariable() {
188+
int cnt = compileAndCount("snippetSubVariable", null, ins -> ins instanceof MemoryMROp && "SUB".equals(((MemoryMROp) ins).getOpcode().toString()));
189+
assumeTrue("SUB is expected once in the LIR", cnt == 1);
190+
test("snippetSubVariable", 1);
191+
test("snippetSubVariable", 0xDEADBEEF);
192+
}
193+
194+
public static int snippetInstanceFieldSub1(A a) {
195+
resetInstance(a);
196+
a.intField--;
197+
return getInstanceValue(a);
198+
}
199+
200+
@Test
201+
public void testSnippetInstanceFieldSub1() {
202+
OptionValues options = getInitialOptions();
203+
assumeTrue("UseTrappingNullChecks must be on", UseTrappingNullChecks.getValue(options));
204+
Predicate<LIRInstruction> predicate = ins -> ins instanceof MemoryOp && "DEC".equals(((MemoryOp) ins).getOpcode().toString());
205+
int cnt = compileAndCount("snippetInstanceFieldSub1", null, predicate);
206+
assumeTrue("DEC is not expected in the LIR", cnt == 0);
207+
OptionValues disableUseTrappingNullChecks = new OptionValues(options, UseTrappingNullChecks, false);
208+
cnt = compileAndCount("snippetInstanceFieldSub1", disableUseTrappingNullChecks, predicate);
209+
assumeTrue("DEC is expected once in the LIR", cnt == 1);
210+
test("snippetInstanceFieldSub1", disableUseTrappingNullChecks, new A());
211+
}
212+
213+
public static int snippetOr1() {
214+
reset();
215+
staticIntField |= 1;
216+
return getValue();
217+
}
218+
219+
public static int snippetOrLargeConstant() {
220+
reset();
221+
staticIntField |= 0xDEADBEEF;
222+
return getValue();
223+
}
224+
225+
@Test
226+
public void testSnippetOrConstant() {
227+
Predicate<LIRInstruction> predicate = ins -> ins instanceof MemoryConstOp && "OR".equals(((MemoryConstOp) ins).getOpcode().toString());
228+
int cnt = compileAndCount("snippetOr1", null, predicate);
229+
assumeTrue("OR is expected once in the LIR", cnt == 1);
230+
test("snippetOr1");
231+
232+
cnt = compileAndCount("snippetOrLargeConstant", null, predicate);
233+
assumeTrue("OR is expected once in the LIR", cnt == 1);
234+
test("snippetOrLargeConstant");
235+
}
236+
237+
public static int snippetOrVariable(int i) {
238+
reset();
239+
staticIntField |= i;
240+
return getValue();
241+
}
242+
243+
@Test
244+
public void testSnippetOrVariable() {
245+
int cnt = compileAndCount("snippetOrVariable", null, ins -> ins instanceof MemoryMROp && "OR".equals(((MemoryMROp) ins).getOpcode().toString()));
246+
assumeTrue("OR is expected once in the LIR", cnt == 1);
247+
test("snippetOrVariable", 1);
248+
test("snippetOrVariable", 0xDEADBEEF);
249+
}
250+
251+
public static int snippetInstanceFieldOr(A a) {
252+
resetInstance(a);
253+
a.intField |= 0xDEADBEEF;
254+
return getInstanceValue(a);
255+
}
256+
257+
@Test
258+
public void testSnippetInstanceFieldOr() {
259+
OptionValues options = getInitialOptions();
260+
assumeTrue("UseTrappingNullChecks must be on", UseTrappingNullChecks.getValue(options));
261+
Predicate<LIRInstruction> predicate = ins -> ins instanceof MemoryConstOp && "OR".equals(((MemoryConstOp) ins).getOpcode().toString());
262+
int cnt = compileAndCount("snippetInstanceFieldOr", null, predicate);
263+
assumeTrue("OR is not expected in the LIR", cnt == 0);
264+
OptionValues disableUseTrappingNullChecks = new OptionValues(options, UseTrappingNullChecks, false);
265+
cnt = compileAndCount("snippetInstanceFieldOr", disableUseTrappingNullChecks, predicate);
266+
assumeTrue("OR is expected once in the LIR", cnt == 1);
267+
test("snippetInstanceFieldOr", disableUseTrappingNullChecks, new A());
268+
}
269+
270+
public static int snippetXor1() {
271+
reset();
272+
staticIntField ^= 1;
273+
return getValue();
274+
}
275+
276+
public static int snippetXorLargeConstant() {
277+
reset();
278+
staticIntField ^= 0xDEADBEEF;
279+
return getValue();
280+
}
281+
282+
@Test
283+
public void testSnippetXorConstant() {
284+
Predicate<LIRInstruction> predicate = ins -> ins instanceof MemoryConstOp && "XOR".equals(((MemoryConstOp) ins).getOpcode().toString());
285+
int cnt = compileAndCount("snippetXor1", null, predicate);
286+
assumeTrue("XOR is expected once in the LIR", cnt == 1);
287+
cnt = compileAndCount("snippetXorLargeConstant", null, predicate);
288+
assumeTrue("XOR is expected once in the LIR", cnt == 1);
289+
test("snippetXor1");
290+
test("snippetXorLargeConstant");
291+
}
292+
293+
public static int snippetXorVariable(int i) {
294+
reset();
295+
staticIntField ^= i;
296+
return getValue();
297+
}
298+
299+
@Test
300+
public void testSnippetXorVariable() {
301+
int cnt = compileAndCount("snippetXorVariable", null, ins -> ins instanceof MemoryMROp && "XOR".equals(((MemoryMROp) ins).getOpcode().toString()));
302+
assumeTrue("XOR is expected once in the LIR", cnt == 1);
303+
test("snippetXorVariable", 1);
304+
test("snippetXorVariable", 0xDEADBEEF);
305+
}
306+
307+
public static int snippetInstanceFieldXor(A a) {
308+
resetInstance(a);
309+
a.intField ^= 0xDEADBEEF;
310+
return getInstanceValue(a);
311+
}
312+
313+
@Test
314+
public void testSnippetInstanceFieldXor() {
315+
OptionValues options = getInitialOptions();
316+
assumeTrue("UseTrappingNullChecks must be on", UseTrappingNullChecks.getValue(options));
317+
Predicate<LIRInstruction> predicate = ins -> ins instanceof MemoryConstOp && "XOR".equals(((MemoryConstOp) ins).getOpcode().toString());
318+
int cnt = compileAndCount("snippetInstanceFieldXor", null, predicate);
319+
assumeTrue("OR is not expected in the LIR", cnt == 0);
320+
OptionValues disableUseTrappingNullChecks = new OptionValues(options, UseTrappingNullChecks, false);
321+
cnt = compileAndCount("snippetInstanceFieldXor", disableUseTrappingNullChecks, predicate);
322+
assumeTrue("OR is expected once in the LIR", cnt == 1);
323+
test("snippetInstanceFieldXor", disableUseTrappingNullChecks, new A());
324+
}
325+
}

‎compiler/src/org.graalvm.compiler.lir.amd64/src/org/graalvm/compiler/lir/amd64/AMD64BinaryConsumer.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ public boolean makeNullCheckFor(Value value, LIRFrameState nullCheckState, int i
285285
}
286286
return false;
287287
}
288+
289+
public AMD64MROp getOpcode() {
290+
return opcode;
291+
}
288292
}
289293

290294
/**

0 commit comments

Comments
 (0)