Skip to content

Commit ce97031

Browse files
tkrodrigueztkrodriguez
committed
Refactor implicit load/store conversions
1 parent 963dfbc commit ce97031

File tree

5 files changed

+58
-51
lines changed

5 files changed

+58
-51
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,6 @@ public static TestClassInt testDeoptLongConstantSnippet() {
348348
return x;
349349
}
350350

351-
352351
static final int VALUE = 0xF0F0F0F0;
353352

354353
public static boolean getBoolean() {

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
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;
3130
import org.graalvm.word.LocationIdentity;
3231

3332
import jdk.graal.compiler.core.common.memory.MemoryOrderMode;
@@ -56,6 +55,7 @@
5655
import jdk.graal.compiler.nodes.type.StampTool;
5756
import jdk.graal.compiler.nodes.virtual.VirtualArrayNode;
5857
import jdk.graal.compiler.nodes.virtual.VirtualObjectNode;
58+
import jdk.graal.compiler.replacements.DefaultJavaLoweringProvider;
5959
import jdk.vm.ci.meta.JavaKind;
6060
import jdk.vm.ci.meta.ResolvedJavaField;
6161

@@ -192,8 +192,7 @@ public void virtualize(VirtualizerTool tool) {
192192
* Expand the value to 32 bits again and perform boolean coercion if
193193
* necessary.
194194
*/
195-
ValueNode extended = DefaultJavaLoweringProvider.implicitPrimitiveLoadConvert(kind, narrowed);
196-
replacement = DefaultJavaLoweringProvider.performBooleanCoercionIfNecessary(extended, kind, graph(), false);
195+
replacement = DefaultJavaLoweringProvider.implicitUnsafePrimitiveLoadConvert(kind, narrowed);
197196
}
198197

199198
tool.ensureAdded(replacement);

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

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@
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;
4644
import org.graalvm.word.LocationIdentity;
4745

4846
import jdk.graal.compiler.core.common.memory.BarrierType;
@@ -82,6 +80,7 @@
8280
import jdk.graal.compiler.nodes.ValueNode;
8381
import jdk.graal.compiler.nodes.ValuePhiNode;
8482
import jdk.graal.compiler.nodes.calc.AddNode;
83+
import jdk.graal.compiler.nodes.calc.AndNode;
8584
import jdk.graal.compiler.nodes.calc.ConditionalNode;
8685
import jdk.graal.compiler.nodes.calc.FloatingIntegerDivRemNode;
8786
import jdk.graal.compiler.nodes.calc.IntegerBelowNode;
@@ -91,6 +90,7 @@
9190
import jdk.graal.compiler.nodes.calc.IsNullNode;
9291
import jdk.graal.compiler.nodes.calc.LeftShiftNode;
9392
import jdk.graal.compiler.nodes.calc.NarrowNode;
93+
import jdk.graal.compiler.nodes.calc.OrNode;
9494
import jdk.graal.compiler.nodes.calc.ReinterpretNode;
9595
import jdk.graal.compiler.nodes.calc.RightShiftNode;
9696
import jdk.graal.compiler.nodes.calc.SignExtendNode;
@@ -831,7 +831,7 @@ protected ReadNode createUnsafeRead(StructuredGraph graph, RawLoadNode load, Gua
831831
} else {
832832
memoryRead.setGuard(guard);
833833
}
834-
ValueNode readValue = performBooleanCoercionIfNecessary(implicitLoadConvert(graph, readKind, memoryRead, compressible), readKind);
834+
ValueNode readValue = implicitUnsafeLoadConvert(graph, readKind, memoryRead, compressible);
835835
load.replaceAtUsages(readValue);
836836
return memoryRead;
837837
}
@@ -846,26 +846,18 @@ protected void lowerUnsafeMemoryLoadNode(UnsafeMemoryLoadNode load) {
846846
// An unsafe read must not float otherwise it may float above
847847
// a test guaranteeing the read is safe.
848848
memoryRead.setForceFixed(true);
849-
ValueNode readValue = performBooleanCoercionIfNecessary(implicitLoadConvert(graph, readKind, memoryRead, false), readKind);
849+
ValueNode readValue = implicitUnsafeLoadConvert(graph, readKind, memoryRead, false);
850850
load.replaceAtUsages(readValue);
851851
graph.replaceFixedWithFixed(load, memoryRead);
852852
}
853853

854-
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) {
860-
if (readKind == JavaKind.Boolean) {
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;
867-
}
868-
return readValue;
854+
/**
855+
* Coerce integer values into a boolean 0 or 1 to match Java semantics. The returned nodes have
856+
* not been added to the graph.
857+
*/
858+
private static ValueNode performBooleanCoercion(ValueNode readValue) {
859+
IntegerEqualsNode eq = new IntegerEqualsNode(readValue, ConstantNode.forInt(0));
860+
return new ConditionalNode(eq, ConstantNode.forBoolean(false), ConstantNode.forBoolean(true));
869861
}
870862

871863
protected void lowerUnsafeStoreNode(RawStoreNode store) {
@@ -1278,39 +1270,49 @@ protected Stamp loadStamp(Stamp stamp, JavaKind kind, boolean compressible) {
12781270
return stamp;
12791271
}
12801272

1281-
public final ValueNode implicitLoadConvertWithBooleanCoercionIfNecessary(StructuredGraph graph, JavaKind kind, ValueNode value) {
1282-
return performBooleanCoercionIfNecessary(implicitLoadConvert(graph, kind, value), kind);
1273+
protected abstract ValueNode newCompressionNode(CompressionOp op, ValueNode value);
1274+
1275+
/**
1276+
* Perform sign or zero extensions for subword types, and convert potentially unsafe 8 bit
1277+
* boolean values into 0 or 1. The nodes have already been added to the graph.
1278+
*/
1279+
public final ValueNode implicitUnsafeLoadConvert(StructuredGraph graph, JavaKind kind, ValueNode value, boolean compressible) {
1280+
if (compressible && kind.isObject()) {
1281+
return implicitLoadConvert(graph, kind, value, compressible);
1282+
} else {
1283+
ValueNode ret = implicitUnsafePrimitiveLoadConvert(kind, value);
1284+
if (!ret.isAlive()) {
1285+
ret = graph.addOrUniqueWithInputs(ret);
1286+
}
1287+
return ret;
1288+
}
12831289
}
12841290

12851291
public final ValueNode implicitLoadConvert(StructuredGraph graph, JavaKind kind, ValueNode value) {
12861292
return implicitLoadConvert(graph, kind, value, true);
12871293
}
12881294

1289-
public ValueNode implicitLoadConvert(JavaKind kind, ValueNode value) {
1290-
return implicitLoadConvert(kind, value, true);
1291-
}
1292-
1295+
/**
1296+
* Perform sign or zero extensions for subword types and add the nodes to the graph.
1297+
*/
12931298
protected final ValueNode implicitLoadConvert(StructuredGraph graph, JavaKind kind, ValueNode value, boolean compressible) {
1294-
ValueNode ret = implicitLoadConvert(kind, value, compressible);
1299+
ValueNode ret;
1300+
if (useCompressedOops(kind, compressible)) {
1301+
ret = newCompressionNode(CompressionOp.Uncompress, value);
1302+
} else {
1303+
ret = implicitPrimitiveLoadConvert(kind, value);
1304+
}
1305+
12951306
if (!ret.isAlive()) {
1296-
ret = graph.addOrUnique(ret);
1307+
ret = graph.addOrUniqueWithInputs(ret);
12971308
}
12981309
return ret;
12991310
}
13001311

1301-
protected abstract ValueNode newCompressionNode(CompressionOp op, ValueNode value);
1302-
13031312
/**
1304-
* @param compressible whether the convert should be compressible
1313+
* Perform sign or zero extensions for subword types. The caller is expected to add an resulting
1314+
* nodes to the graph.
13051315
*/
1306-
protected ValueNode implicitLoadConvert(JavaKind kind, ValueNode value, boolean compressible) {
1307-
if (useCompressedOops(kind, compressible)) {
1308-
return newCompressionNode(CompressionOp.Uncompress, value);
1309-
}
1310-
1311-
return implicitPrimitiveLoadConvert(kind, value);
1312-
}
1313-
13141316
public static ValueNode implicitPrimitiveLoadConvert(JavaKind kind, ValueNode value) {
13151317
return switch (kind) {
13161318
case Byte, Short -> new SignExtendNode(value, 32);
@@ -1319,6 +1321,17 @@ public static ValueNode implicitPrimitiveLoadConvert(JavaKind kind, ValueNode va
13191321
};
13201322
}
13211323

1324+
/**
1325+
* Perform sign or zero extensions for subword types, and convert potentially unsafe 8 bit
1326+
* boolean values into 0 or 1. The caller is expected to add an resulting * nodes to the graph.
1327+
*/
1328+
public static ValueNode implicitUnsafePrimitiveLoadConvert(JavaKind kind, ValueNode value) {
1329+
if (kind == JavaKind.Boolean) {
1330+
return performBooleanCoercion(new ZeroExtendNode(value, 32));
1331+
}
1332+
return implicitPrimitiveLoadConvert(kind, value);
1333+
}
1334+
13221335
public ValueNode arrayImplicitStoreConvert(StructuredGraph graph,
13231336
JavaKind entryKind,
13241337
ValueNode value,

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateLoweringProvider.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,17 @@
2626

2727
import java.util.Map;
2828

29+
import com.oracle.svm.core.graal.GraalConfiguration;
30+
import com.oracle.svm.core.graal.snippets.NodeLoweringProvider;
31+
2932
import jdk.graal.compiler.core.common.spi.ForeignCallsProvider;
3033
import jdk.graal.compiler.core.common.spi.MetaAccessExtensionProvider;
3134
import jdk.graal.compiler.core.common.type.Stamp;
3235
import jdk.graal.compiler.graph.Node;
33-
import jdk.graal.compiler.nodes.StructuredGraph;
34-
import jdk.graal.compiler.nodes.ValueNode;
3536
import jdk.graal.compiler.nodes.spi.LoweringProvider;
3637
import jdk.graal.compiler.nodes.spi.PlatformConfigurationProvider;
3738
import jdk.graal.compiler.options.OptionValues;
3839
import jdk.graal.compiler.phases.util.Providers;
39-
40-
import com.oracle.svm.core.graal.GraalConfiguration;
41-
import com.oracle.svm.core.graal.snippets.NodeLoweringProvider;
42-
4340
import jdk.vm.ci.meta.JavaKind;
4441
import jdk.vm.ci.meta.MetaAccessProvider;
4542

@@ -49,8 +46,6 @@ public interface SubstrateLoweringProvider extends LoweringProvider {
4946

5047
Map<Class<? extends Node>, NodeLoweringProvider<?>> getLowerings();
5148

52-
ValueNode implicitLoadConvertWithBooleanCoercionIfNecessary(StructuredGraph graph, JavaKind kind, ValueNode value);
53-
5449
Stamp loadStamp(Stamp stamp, JavaKind kind);
5550

5651
static LoweringProvider createForHosted(MetaAccessProvider metaAccess, ForeignCallsProvider foreignCalls, PlatformConfigurationProvider platformConfig,

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
import jdk.graal.compiler.nodes.java.StoreIndexedNode;
8080
import jdk.graal.compiler.phases.common.inlining.InliningUtil;
8181
import jdk.graal.compiler.phases.util.Providers;
82+
import jdk.graal.compiler.replacements.DefaultJavaLoweringProvider;
8283
import jdk.graal.compiler.replacements.GraphKit;
8384
import jdk.vm.ci.code.BytecodeFrame;
8485
import jdk.vm.ci.code.CallingConvention;
@@ -239,7 +240,7 @@ public ValueNode createCFunctionCallWithCapture(ValueNode targetAddress, List<Va
239240

240241
// Sign or zero extend to get a clean int value. If a boolean result is expected, the int
241242
// value is coerced to true or false.
242-
return getLoweringProvider().implicitLoadConvertWithBooleanCoercionIfNecessary(getGraph(), asKind(returnType), result);
243+
return DefaultJavaLoweringProvider.implicitUnsafePrimitiveLoadConvert(asKind(returnType), result);
243244
}
244245

245246
public InvokeNode createIndirectCall(ValueNode targetAddress, List<ValueNode> args, Signature signature, SubstrateCallingConventionKind callKind) {

0 commit comments

Comments
 (0)