Skip to content

[GR-68664] Disallow floating reads unless FloatingReadsPhase has been run #11942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
lateMacroInvoke.setTemplateProducer(new Supplier<SnippetTemplate>() {
@Override
public SnippetTemplate get() {
Arguments args = new Arguments(transplantTestSnippets.producer, GuardsStage.AFTER_FSA, LoweringTool.StandardLoweringStage.LOW_TIER);
Arguments args = new Arguments(transplantTestSnippets.producer, GuardsStage.AFTER_FSA, true, LoweringTool.StandardLoweringStage.LOW_TIER);
// no args
SnippetTemplate template = transplantTestSnippets.template(getProviders(), lateMacroInvoke, args);
return template;
Expand All @@ -116,7 +116,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
lateMacroInvoke.setTemplateProducer(new Supplier<SnippetTemplate>() {
@Override
public SnippetTemplate get() {
Arguments args = new Arguments(transplantTestSnippets.producerWithArgs, GuardsStage.AFTER_FSA, LoweringTool.StandardLoweringStage.LOW_TIER);
Arguments args = new Arguments(transplantTestSnippets.producerWithArgs, GuardsStage.AFTER_FSA, true, LoweringTool.StandardLoweringStage.LOW_TIER);
args.add("a", arg0);
args.add("b", arg1);
SnippetTemplate template = transplantTestSnippets.template(getProviders(), lateMacroInvoke, args);
Expand All @@ -140,7 +140,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
lateMacroInvoke.setTemplateProducer(new Supplier<SnippetTemplate>() {
@Override
public SnippetTemplate get() {
Arguments args = new Arguments(transplantTestSnippets.producerWithDeopt, GuardsStage.AFTER_FSA, LoweringTool.StandardLoweringStage.LOW_TIER);
Arguments args = new Arguments(transplantTestSnippets.producerWithDeopt, GuardsStage.AFTER_FSA, true, LoweringTool.StandardLoweringStage.LOW_TIER);
args.add("a", arg0);
args.add("b", arg1);
SnippetTemplate template = transplantTestSnippets.template(getProviders(), lateMacroInvoke, args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,25 +390,17 @@ public ComplexMatchResult testBitAndBranch(IfNode root, ValueNode value, Constan
}

@MatchRule("(If (IntegerTest Read=access value))")
@MatchRule("(If (IntegerTest FloatingRead=access value))")
public ComplexMatchResult integerTestBranchMemory(IfNode root, LIRLowerableAccess access, ValueNode value) {
return emitIntegerTestBranchMemory(root, value, access);
}

@MatchRule("(If (IntegerEquals=compare value Read=access))")
@MatchRule("(If (IntegerLessThan=compare value Read=access))")
@MatchRule("(If (IntegerBelow=compare value Read=access))")
@MatchRule("(If (IntegerEquals=compare value FloatingRead=access))")
@MatchRule("(If (IntegerLessThan=compare value FloatingRead=access))")
@MatchRule("(If (IntegerBelow=compare value FloatingRead=access))")
@MatchRule("(If (FloatEquals=compare value Read=access))")
@MatchRule("(If (FloatEquals=compare value FloatingRead=access))")
@MatchRule("(If (FloatLessThan=compare value Read=access))")
@MatchRule("(If (FloatLessThan=compare value FloatingRead=access))")
@MatchRule("(If (PointerEquals=compare value Read=access))")
@MatchRule("(If (PointerEquals=compare value FloatingRead=access))")
@MatchRule("(If (ObjectEquals=compare value Read=access))")
@MatchRule("(If (ObjectEquals=compare value FloatingRead=access))")
public ComplexMatchResult ifCompareMemory(IfNode root, CompareNode compare, ValueNode value, LIRLowerableAccess access) {
return emitCompareBranchMemory(root, compare, value, access);
}
Expand Down Expand Up @@ -468,7 +460,6 @@ public ComplexMatchResult ifCompareLogicCas(IfNode root, CompareNode compare, Va
return null;
}

@MatchRule("(If (ObjectEquals=compare value FloatingRead=access))")
public ComplexMatchResult ifLogicCas(IfNode root, CompareNode compare, ValueNode value, LIRLowerableAccess access) {
return emitCompareBranchMemory(root, compare, value, access);
}
Expand Down Expand Up @@ -516,7 +507,6 @@ private ComplexMatchResult binaryRead(AMD64Assembler.VexRVMOp op, OperandSize si
}

@MatchRule("(Add value Read=access)")
@MatchRule("(Add value FloatingRead=access)")
public ComplexMatchResult addMemory(ValueNode value, LIRLowerableAccess access) {
OperandSize size = getMemorySize(access);
if (size.isXmmType()) {
Expand All @@ -531,7 +521,6 @@ public ComplexMatchResult addMemory(ValueNode value, LIRLowerableAccess access)
}

@MatchRule("(Sub value Read=access)")
@MatchRule("(Sub value FloatingRead=access)")
public ComplexMatchResult subMemory(ValueNode value, LIRLowerableAccess access) {
OperandSize size = getMemorySize(access);
if (size.isXmmType()) {
Expand All @@ -546,7 +535,6 @@ public ComplexMatchResult subMemory(ValueNode value, LIRLowerableAccess access)
}

@MatchRule("(Mul value Read=access)")
@MatchRule("(Mul value FloatingRead=access)")
public ComplexMatchResult mulMemory(ValueNode value, LIRLowerableAccess access) {
OperandSize size = getMemorySize(access);
if (size.isXmmType()) {
Expand All @@ -561,7 +549,6 @@ public ComplexMatchResult mulMemory(ValueNode value, LIRLowerableAccess access)
}

@MatchRule("(And value Read=access)")
@MatchRule("(And value FloatingRead=access)")
public ComplexMatchResult andMemory(ValueNode value, LIRLowerableAccess access) {
OperandSize size = getMemorySize(access);
if (size.isXmmType()) {
Expand All @@ -572,7 +559,6 @@ public ComplexMatchResult andMemory(ValueNode value, LIRLowerableAccess access)
}

@MatchRule("(Or value Read=access)")
@MatchRule("(Or value FloatingRead=access)")
public ComplexMatchResult orMemory(ValueNode value, LIRLowerableAccess access) {
OperandSize size = getMemorySize(access);
if (size.isXmmType()) {
Expand All @@ -583,7 +569,6 @@ public ComplexMatchResult orMemory(ValueNode value, LIRLowerableAccess access) {
}

@MatchRule("(Xor value Read=access)")
@MatchRule("(Xor value FloatingRead=access)")
public ComplexMatchResult xorMemory(ValueNode value, LIRLowerableAccess access) {
OperandSize size = getMemorySize(access);
if (size.isXmmType()) {
Expand Down Expand Up @@ -660,20 +645,17 @@ public ComplexMatchResult writeNarrow(WriteNode root, NarrowNode narrow) {
}

@MatchRule("(SignExtend Read=access)")
@MatchRule("(SignExtend FloatingRead=access)")
public ComplexMatchResult signExtend(SignExtendNode root, LIRLowerableAccess access) {
return emitSignExtendMemory(access, root.getInputBits(), root.getResultBits(), null);
}

@MatchRule("(ZeroExtend Read=access)")
@MatchRule("(ZeroExtend FloatingRead=access)")
public ComplexMatchResult zeroExtend(ZeroExtendNode root, LIRLowerableAccess access) {
AMD64Kind memoryKind = getMemoryKind(access);
return builder -> getArithmeticLIRGenerator().emitZeroExtendMemory(memoryKind, root.getResultBits(), (AMD64AddressValue) operand(access.getAddress()), getState(access));
}

@MatchRule("(Narrow Read=access)")
@MatchRule("(Narrow FloatingRead=access)")
public ComplexMatchResult narrowRead(NarrowNode root, LIRLowerableAccess access) {
return new ComplexMatchResult() {
@Override
Expand All @@ -690,14 +672,12 @@ public Value evaluate(NodeLIRBuilder builder) {
}

@MatchRule("(SignExtend (Narrow=narrow Read=access))")
@MatchRule("(SignExtend (Narrow=narrow FloatingRead=access))")
public ComplexMatchResult signExtendNarrowRead(SignExtendNode root, NarrowNode narrow, LIRLowerableAccess access) {
LIRKind kind = getLIRGeneratorTool().getLIRKind(narrow.stamp(NodeView.DEFAULT));
return emitSignExtendMemory(access, narrow.getResultBits(), root.getResultBits(), kind);
}

@MatchRule("(FloatConvert Read=access)")
@MatchRule("(FloatConvert FloatingRead=access)")
public ComplexMatchResult floatConvert(FloatConvertNode root, LIRLowerableAccess access) {
if (root.getFloatConvert().getCategory().equals(FloatConvertCategory.FloatingPointToInteger) && (root.inputCanBeNaN() || root.canOverflow())) {
/* We need to fix up the result of the conversion, the input should be in a register. */
Expand Down Expand Up @@ -730,7 +710,6 @@ public ComplexMatchResult floatConvert(FloatConvertNode root, LIRLowerableAccess
}

@MatchRule("(Reinterpret Read=access)")
@MatchRule("(Reinterpret FloatingRead=access)")
public ComplexMatchResult reinterpret(ReinterpretNode root, LIRLowerableAccess access) {
return builder -> {
LIRKind kind = getLIRGeneratorTool().getLIRKind(root.stamp(NodeView.DEFAULT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ protected void updatePredecessor(Node oldSuccessor, Node newSuccessor) {
}
}

void initialize(Graph newGraph) {
final void initialize(Graph newGraph) {
assertTrue(id == INITIAL_ID, "unexpected id: %d", id);
this.graph = newGraph;
newGraph.register(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void lower(UnaryMathIntrinsicNode mathIntrinsicNode, LoweringTool tool) {
throw GraalError.shouldNotReachHere("Snippet not found for math intrinsic " + mathIntrinsicNode.getOperation().name()); // ExcludeFromJacocoGeneratedReport
}

Arguments args = new Arguments(info, mathIntrinsicNode.graph().getGuardsStage(), tool.getLoweringStage());
Arguments args = new Arguments(info, mathIntrinsicNode.graph(), tool.getLoweringStage());
args.add("input", mathIntrinsicNode.getValue());
template(tool, mathIntrinsicNode, args).instantiate(tool.getMetaAccess(), mathIntrinsicNode, DEFAULT_REPLACER, tool, args);
mathIntrinsicNode.safeDelete();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,27 @@ private void lowerKlassLayoutHelperNode(KlassLayoutHelperNode n, LoweringTool to
}
StructuredGraph graph = n.graph();
assert !n.getHub().isConstant();
AddressNode address = createOffsetAddress(graph, n.getHub(), runtime.getVMConfig().klassLayoutHelperOffset);
ReadNode memoryRead = graph.add(new ReadNode(address, HotSpotReplacementsUtil.KLASS_LAYOUT_HELPER_LOCATION, null, n.stamp(NodeView.DEFAULT), null, BarrierType.NONE));
graph.addAfterFixed(tool.lastFixedNode(), memoryRead);
ValueNode memoryRead = createRead(graph, createOffsetAddress(graph, n.getHub(), runtime.getVMConfig().klassLayoutHelperOffset), HotSpotReplacementsUtil.KLASS_LAYOUT_HELPER_LOCATION,
n.stamp(NodeView.DEFAULT), BarrierType.NONE, tool.lastFixedNode());
n.replaceAtUsagesAndDelete(memoryRead);
}

private static ValueNode createRead(StructuredGraph graph, AddressNode address, LocationIdentity location, Stamp stamp, BarrierType barrierType, FixedWithNextNode insertAfter) {
return createRead(graph, address, location, stamp, null, barrierType, insertAfter);
}

private static ValueNode createRead(StructuredGraph graph, AddressNode address, LocationIdentity location, Stamp stamp, GuardingNode guard, BarrierType barrierType,
FixedWithNextNode insertAfter) {
if (graph.getGraphState().allowsFloatingReads()) {
return graph.unique(new FloatingReadNode(address, location, null, stamp, guard, barrierType));
} else {
ReadNode memoryRead = graph.add(new ReadNode(address, location, null, stamp, guard, barrierType));
graph.addAfterFixed(insertAfter, memoryRead);
assert memoryRead.canFloat() : "this path is only useable for floatable reads: " + memoryRead;
return memoryRead;
}
}

private void lowerHubGetClassNode(HubGetClassNode n, LoweringTool tool) {
if (tool.getLoweringStage() == LoweringTool.StandardLoweringStage.HIGH_TIER) {
return;
Expand All @@ -809,13 +824,16 @@ private void lowerHubGetClassNode(HubGetClassNode n, LoweringTool tool) {
assert !hub.isConstant();
AddressNode mirrorAddress = createOffsetAddress(graph, hub, vmConfig.classMirrorOffset);
Stamp loadStamp = n.stamp(NodeView.DEFAULT);
FloatingReadNode read = graph.unique(
new FloatingReadNode(mirrorAddress, HotSpotReplacementsUtil.CLASS_MIRROR_LOCATION, null, StampFactory.forKind(target.wordJavaKind),
null, BarrierType.NONE));
FixedWithNextNode insertAfter = tool.lastFixedNode();
ValueNode read = createRead(graph, mirrorAddress, HotSpotReplacementsUtil.CLASS_MIRROR_LOCATION, StampFactory.forKind(target.wordJavaKind),
BarrierType.NONE, insertAfter);
if (read instanceof FixedWithNextNode fixed) {
insertAfter = fixed;
}
// Read the Object from the OopHandle
AddressNode address = createOffsetAddress(graph, read, 0);
read = graph.unique(new FloatingReadNode(address, HotSpotReplacementsUtil.HOTSPOT_OOP_HANDLE_LOCATION, null, loadStamp, null,
barrierSet.readBarrierType(HotSpotReplacementsUtil.HOTSPOT_OOP_HANDLE_LOCATION, address, loadStamp)));
read = createRead(graph, address, HotSpotReplacementsUtil.HOTSPOT_OOP_HANDLE_LOCATION, loadStamp,
barrierSet.readBarrierType(HotSpotReplacementsUtil.HOTSPOT_OOP_HANDLE_LOCATION, address, loadStamp), insertAfter);
n.replaceAtUsagesAndDelete(read);
}

Expand All @@ -827,7 +845,7 @@ private void lowerClassGetHubNode(ClassGetHubNode n, LoweringTool tool) {
StructuredGraph graph = n.graph();
assert !n.getValue().isConstant();
AddressNode address = createOffsetAddress(graph, n.getValue(), runtime.getVMConfig().klassOffset);
FloatingReadNode read = graph.unique(new FloatingReadNode(address, HotSpotReplacementsUtil.CLASS_KLASS_LOCATION, null, n.stamp(NodeView.DEFAULT), null, BarrierType.NONE));
ValueNode read = createRead(graph, address, HotSpotReplacementsUtil.CLASS_KLASS_LOCATION, n.stamp(NodeView.DEFAULT), BarrierType.NONE, tool.lastFixedNode());
n.replaceAtUsagesAndDelete(read);
}

Expand Down Expand Up @@ -920,9 +938,7 @@ protected ValueNode createReadArrayComponentHub(StructuredGraph graph, ValueNode
guard = AbstractBeginNode.prevBegin(anchor);
}
AddressNode address = createOffsetAddress(graph, arrayHub, runtime.getVMConfig().arrayClassElementOffset);
ReadNode read = graph.add(new ReadNode(address, HotSpotReplacementsUtil.OBJ_ARRAY_KLASS_ELEMENT_KLASS_LOCATION, null, KlassPointerStamp.klassNonNull(), guard, BarrierType.NONE));
graph.addAfterFixed(insertAfter, read);
return read;
return createRead(graph, address, HotSpotReplacementsUtil.OBJ_ARRAY_KLASS_ELEMENT_KLASS_LOCATION, KlassPointerStamp.klassNonNull(), guard, BarrierType.NONE, insertAfter);
}

private void lowerLoadMethodNode(LoadMethodNode loadMethodNode) {
Expand Down Expand Up @@ -1157,22 +1173,17 @@ protected ValueNode createReadHub(StructuredGraph graph, ValueNode object, Lower
hubStamp = hubStamp.compressed(config.getKlassEncoding());
}

FixedWithNextNode effectiveInsertAfter = insertAfter;

if (config.useCompactObjectHeaders) {
AddressNode address = createOffsetAddress(graph, object, config.markOffset);
ReadNode memoryRead = graph.add(new ReadNode(address, COMPACT_HUB_LOCATION, null, StampFactory.forKind(JavaKind.Long), null, BarrierType.NONE));
graph.addAfterFixed(insertAfter, memoryRead);
effectiveInsertAfter = memoryRead;
ValueNode memoryRead = createRead(graph, address, COMPACT_HUB_LOCATION, StampFactory.forKind(JavaKind.Long), BarrierType.NONE, insertAfter);
ValueNode rawCompressedHubWordSize = graph.addOrUnique(UnsignedRightShiftNode.create(memoryRead, ConstantNode.forInt(config.markWordKlassShift, graph), NodeView.DEFAULT));
ValueNode rawCompressedHub = graph.addOrUnique(NarrowNode.create(rawCompressedHubWordSize, 32, NodeView.DEFAULT));
ValueNode compressedKlassPointer = graph.addOrUnique(PointerCastNode.create(hubStamp, rawCompressedHub));
return HotSpotCompressionNode.uncompress(graph, compressedKlassPointer, config.getKlassEncoding());
}
AddressNode address = createOffsetAddress(graph, object, config.hubOffset);
LocationIdentity hubLocation = config.useCompressedClassPointers ? COMPRESSED_HUB_LOCATION : HUB_LOCATION;
ReadNode memoryRead = graph.add(new ReadNode(address, hubLocation, null, hubStamp, null, BarrierType.NONE));
graph.addAfterFixed(effectiveInsertAfter, memoryRead);
ValueNode memoryRead = createRead(graph, address, hubLocation, hubStamp, BarrierType.NONE, insertAfter);
if (config.useCompressedClassPointers) {
return HotSpotCompressionNode.uncompress(graph, memoryRead, config.getKlassEncoding());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public Templates(OptionValues options, HotSpotProviders providers) {

public void lower(AssertionNode assertionNode, LoweringTool tool) {
StructuredGraph graph = assertionNode.graph();
Arguments args = new Arguments(graph.start() instanceof StubStartNode ? stubAssertion : assertion, graph.getGuardsStage(), tool.getLoweringStage());
Arguments args = new Arguments(graph.start() instanceof StubStartNode ? stubAssertion : assertion, graph, tool.getLoweringStage());
args.add("condition", assertionNode.condition());
args.add("message",
graph.unique(new ConstantNode(new CStringConstant("failed runtime assertion in snippet/stub: " + assertionNode.message() + " (" + graph.method() + ")"),
Expand Down
Loading
Loading