Skip to content

Commit 963dfbc

Browse files
author
Michael Haas
committed
Inserted load and store conversions for unsafe accesses during PEA.
1 parent 69f10d3 commit 963dfbc

File tree

5 files changed

+180
-25
lines changed

5 files changed

+180
-25
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ea/EATestBase.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ public int hashCode() {
114114
}
115115
}
116116

117+
public int getFirstField() {
118+
if (firstFieldIsX) {
119+
return x;
120+
}
121+
return y;
122+
}
123+
117124
public void setFirstField(int v) {
118125
if (firstFieldIsX) {
119126
x = v;

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ea/UnsafeEATest.java‎

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626

2727
import java.nio.ByteBuffer;
2828

29+
import org.junit.Assert;
30+
import org.junit.Test;
31+
2932
import jdk.graal.compiler.api.directives.GraalDirectives;
3033
import jdk.graal.compiler.graph.Graph;
3134
import jdk.graal.compiler.graph.Node;
@@ -37,9 +40,6 @@
3740
import jdk.graal.compiler.nodes.extended.RawStoreNode;
3841
import jdk.graal.compiler.nodes.extended.UnsafeAccessNode;
3942
import jdk.graal.compiler.nodes.java.LoadFieldNode;
40-
import org.junit.Assert;
41-
import org.junit.Test;
42-
4343
import jdk.vm.ci.meta.JavaConstant;
4444
import jdk.vm.ci.meta.JavaKind;
4545
import jdk.vm.ci.meta.ResolvedJavaMethod;
@@ -348,4 +348,67 @@ public static TestClassInt testDeoptLongConstantSnippet() {
348348
return x;
349349
}
350350

351+
352+
static final int VALUE = 0xF0F0F0F0;
353+
354+
public static boolean getBoolean() {
355+
TestClassInt data = new TestClassInt(VALUE, VALUE);
356+
return UNSAFE.getBoolean(data, TestClassInt.fieldOffset1);
357+
}
358+
359+
public static long getByte() {
360+
TestClassInt data = new TestClassInt(VALUE, VALUE);
361+
return UNSAFE.getByte(data, TestClassInt.fieldOffset1);
362+
}
363+
364+
public static long getShort() {
365+
TestClassInt data = new TestClassInt(VALUE, VALUE);
366+
return UNSAFE.getShort(data, TestClassInt.fieldOffset1);
367+
}
368+
369+
public static long getChar() {
370+
TestClassInt data = new TestClassInt(VALUE, VALUE);
371+
return UNSAFE.getShort(data, TestClassInt.fieldOffset1);
372+
}
373+
374+
@Test
375+
public void testUnsafeLoad() {
376+
test("getBoolean");
377+
test("getByte");
378+
test("getShort");
379+
test("getChar");
380+
}
381+
382+
public static long setBoolean() {
383+
TestClassInt data = new TestClassInt(VALUE, VALUE);
384+
UNSAFE.putBoolean(data, TestClassInt.fieldOffset1, true);
385+
return data.getFirstField();
386+
}
387+
388+
public static long setByte() {
389+
TestClassInt data = new TestClassInt(VALUE, VALUE);
390+
UNSAFE.putByte(data, TestClassInt.fieldOffset1, (byte) VALUE);
391+
return data.getFirstField();
392+
}
393+
394+
public static long setShort() {
395+
TestClassInt data = new TestClassInt(VALUE, VALUE);
396+
UNSAFE.putShort(data, TestClassInt.fieldOffset1, (short) VALUE);
397+
return data.getFirstField();
398+
}
399+
400+
public static long setChar() {
401+
TestClassInt data = new TestClassInt(VALUE, VALUE);
402+
UNSAFE.putChar(data, TestClassInt.fieldOffset1, (char) VALUE);
403+
return data.getFirstField();
404+
}
405+
406+
@Test
407+
public void testUnsafeStore() {
408+
test("setBoolean");
409+
test("setByte");
410+
test("setShort");
411+
test("setChar");
412+
}
413+
351414
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/RawLoadNode.java‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_2;
2828
import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_1;
2929

30+
import jdk.graal.compiler.replacements.DefaultJavaLoweringProvider;
3031
import org.graalvm.word.LocationIdentity;
3132

3233
import jdk.graal.compiler.core.common.memory.MemoryOrderMode;
@@ -176,7 +177,27 @@ public void virtualize(VirtualizerTool tool) {
176177
return;
177178
}
178179
}
179-
tool.replaceWith(entry);
180+
JavaKind kind = accessKind();
181+
ValueNode replacement = tool.getAlias(entry);
182+
183+
if (kind.getStackKind() == JavaKind.Int) {
184+
/*
185+
* Get the value as it would be actually stored in memory or in other
186+
* words only the bytes we are interested in. The type is defined by the
187+
* access kind, e.g. for the access kind byte and the value 0xF0F0F0F0
188+
* we actually want to have 0xF0.
189+
*/
190+
ValueNode narrowed = DefaultJavaLoweringProvider.implicitPrimitiveStoreConvert(kind, replacement);
191+
/*
192+
* Expand the value to 32 bits again and perform boolean coercion if
193+
* necessary.
194+
*/
195+
ValueNode extended = DefaultJavaLoweringProvider.implicitPrimitiveLoadConvert(kind, narrowed);
196+
replacement = DefaultJavaLoweringProvider.performBooleanCoercionIfNecessary(extended, kind, graph(), false);
197+
}
198+
199+
tool.ensureAdded(replacement);
200+
tool.replaceWith(replacement);
180201
}
181202
}
182203
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/DefaultJavaLoweringProvider.java‎

Lines changed: 76 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import java.util.BitSet;
4242
import java.util.List;
4343

44+
import jdk.graal.compiler.nodes.calc.AndNode;
45+
import jdk.graal.compiler.nodes.calc.OrNode;
4446
import org.graalvm.word.LocationIdentity;
4547

4648
import jdk.graal.compiler.core.common.memory.BarrierType;
@@ -850,10 +852,18 @@ protected void lowerUnsafeMemoryLoadNode(UnsafeMemoryLoadNode load) {
850852
}
851853

852854
private static ValueNode performBooleanCoercionIfNecessary(ValueNode readValue, JavaKind readKind) {
855+
StructuredGraph graph = readValue.graph();
856+
return performBooleanCoercionIfNecessary(readValue, readKind, graph, true);
857+
}
858+
859+
public static ValueNode performBooleanCoercionIfNecessary(ValueNode readValue, JavaKind readKind, StructuredGraph graph, boolean withGraphAdd) {
853860
if (readKind == JavaKind.Boolean) {
854-
StructuredGraph graph = readValue.graph();
855-
IntegerEqualsNode eq = graph.addOrUnique(new IntegerEqualsNode(readValue, ConstantNode.forInt(0, graph)));
856-
return graph.addOrUnique(new ConditionalNode(eq, ConstantNode.forBoolean(false, graph), ConstantNode.forBoolean(true, graph)));
861+
IntegerEqualsNode eq = new IntegerEqualsNode(readValue, ConstantNode.forInt(0, graph));
862+
ValueNode result = new ConditionalNode(eq, ConstantNode.forBoolean(false, graph), ConstantNode.forBoolean(true, graph));
863+
if (withGraphAdd) {
864+
result = graph.addOrUniqueWithInputs(result);
865+
}
866+
return result;
857867
}
858868
return readValue;
859869
}
@@ -1298,15 +1308,15 @@ protected ValueNode implicitLoadConvert(JavaKind kind, ValueNode value, boolean
12981308
return newCompressionNode(CompressionOp.Uncompress, value);
12991309
}
13001310

1301-
switch (kind) {
1302-
case Byte:
1303-
case Short:
1304-
return new SignExtendNode(value, 32);
1305-
case Boolean:
1306-
case Char:
1307-
return new ZeroExtendNode(value, 32);
1308-
}
1309-
return value;
1311+
return implicitPrimitiveLoadConvert(kind, value);
1312+
}
1313+
1314+
public static ValueNode implicitPrimitiveLoadConvert(JavaKind kind, ValueNode value) {
1315+
return switch (kind) {
1316+
case Byte, Short -> new SignExtendNode(value, 32);
1317+
case Boolean, Char -> new ZeroExtendNode(value, 32);
1318+
default -> value;
1319+
};
13101320
}
13111321

13121322
public ValueNode arrayImplicitStoreConvert(StructuredGraph graph,
@@ -1363,15 +1373,60 @@ protected ValueNode implicitStoreConvert(JavaKind kind, ValueNode value, boolean
13631373
return newCompressionNode(CompressionOp.Compress, value);
13641374
}
13651375

1366-
switch (kind) {
1367-
case Boolean:
1368-
case Byte:
1369-
return new NarrowNode(value, 8);
1370-
case Char:
1371-
case Short:
1372-
return new NarrowNode(value, 16);
1373-
}
1374-
return value;
1376+
return implicitPrimitiveStoreConvert(kind, value);
1377+
}
1378+
1379+
public static ValueNode implicitPrimitiveStoreConvert(JavaKind kind, ValueNode value) {
1380+
return switch (kind) {
1381+
case Boolean, Byte -> new NarrowNode(value, 8);
1382+
case Char, Short -> new NarrowNode(value, 16);
1383+
default -> value;
1384+
};
1385+
}
1386+
1387+
/**
1388+
* Simulate a primitive store.
1389+
*
1390+
* So for code like:
1391+
*
1392+
* <pre>
1393+
* static class Data {
1394+
* int value = 0xF0F0F0F0;
1395+
* }
1396+
*
1397+
* Data data = new Data();
1398+
* UNSAFE.putByte(data, FIELD_OFFSET, 0x0F);
1399+
* </pre>
1400+
*
1401+
* The field value of the data object is 0xF0F0F0F0 before the unsafe write operation and
1402+
* 0xF0F0F00F after the unsafe write operation. We are not allowed to touch the upper 3 bytes.
1403+
* To simulate the write operation we extract the appropriate bytes and combine them.
1404+
* <p>
1405+
* Example for a byte operation, currently stored value 0xF0F0F0F0 and value to store
1406+
* 0x0000000F:
1407+
*
1408+
* <pre>
1409+
* lowerBytesMask = 00000000 00000000 00000000 11111111
1410+
* upperBytesMask = 11111111 11111111 11111111 00000000
1411+
* currentStored = 11110000 11110000 11110000 11110000
1412+
* valueToStore = 00000000 00000000 00000000 00001111
1413+
* newValue = (currentStored & upperBytesMask) | (valueToStore & lowerBytesMask)
1414+
* = 11110000 11110000 11110000 00001111
1415+
* </pre>
1416+
*
1417+
*/
1418+
public static ValueNode simulatePrimitiveStore(JavaKind kind, ValueNode currentValue, ValueNode valueToStore) {
1419+
// compute the masks
1420+
int bitCount = kind.getByteCount() * 8;
1421+
int lowerBytesMask = (int) CodeUtil.mask(bitCount);
1422+
int upperBytesMask = ~lowerBytesMask;
1423+
1424+
// extract the upper bytes from the current entry
1425+
ValueNode upperBytes = AndNode.create(ConstantNode.forInt(upperBytesMask), currentValue, NodeView.DEFAULT);
1426+
// extract the lower bytes from the value
1427+
ValueNode lowerBytes = AndNode.create(ConstantNode.forInt(lowerBytesMask), valueToStore, NodeView.DEFAULT);
1428+
// combine both
1429+
return OrNode.create(upperBytes, lowerBytes, NodeView.DEFAULT);
13751430
}
13761431

13771432
protected abstract ValueNode createReadHub(StructuredGraph graph, ValueNode object, LoweringTool tool);

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/virtual/phases/ea/VirtualizerToolImpl.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import jdk.graal.compiler.nodes.virtual.VirtualInstanceNode;
5151
import jdk.graal.compiler.nodes.virtual.VirtualObjectNode;
5252
import jdk.graal.compiler.options.OptionValues;
53+
import jdk.graal.compiler.replacements.DefaultJavaLoweringProvider;
5354
import jdk.vm.ci.meta.Assumptions;
5455
import jdk.vm.ci.meta.JavaConstant;
5556
import jdk.vm.ci.meta.JavaKind;
@@ -175,6 +176,14 @@ public boolean setVirtualEntry(VirtualObjectNode virtual, int index, ValueNode v
175176

176177
if (canVirtualize) {
177178
getDebug().log(DebugContext.DETAILED_LEVEL, "virtualizing %s for entryKind %s and access kind %s", current, entryKind, accessKind);
179+
if (theAccessKind != null && entryKind != theAccessKind &&
180+
(theAccessKind != JavaKind.Int && theAccessKind.getStackKind() == JavaKind.Int)) {
181+
ValueNode entry = getEntry(virtual, index);
182+
// We can't just set the given value as the new entry but need to simulate a store
183+
// operation
184+
newValue = DefaultJavaLoweringProvider.simulatePrimitiveStore(accessKind, entry, newValue);
185+
ensureAdded(newValue);
186+
}
178187
state.setEntry(virtual.getObjectId(), index, newValue);
179188
if (entryKind == JavaKind.Int) {
180189
if (accessKind.needsTwoSlots()) {

0 commit comments

Comments
 (0)