Skip to content

Commit 3d454f7

Browse files
zakkakzakkak
authored
Merge pull request #130 from bulasevich/GR-63522
[Backport] [Oracle GraalVM] [GR-63522] Backport to 23.1: Prevent rematerialization of SVM's LoadCompressedObjectConstantOp
2 parents 5016668 + b358b25 commit 3d454f7

File tree

11 files changed

+97
-17
lines changed

11 files changed

+97
-17
lines changed

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotMove.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2025, 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
@@ -30,7 +30,6 @@
3030
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.HINT;
3131
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.ILLEGAL;
3232
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.REG;
33-
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.STACK;
3433

3534
import org.graalvm.compiler.asm.Label;
3635
import org.graalvm.compiler.asm.aarch64.AArch64Assembler;
@@ -52,7 +51,7 @@ public static class LoadHotSpotObjectConstantInline extends AArch64LIRInstructio
5251
public static final LIRInstructionClass<LoadHotSpotObjectConstantInline> TYPE = LIRInstructionClass.create(LoadHotSpotObjectConstantInline.class);
5352

5453
private HotSpotConstant constant;
55-
@Def({REG, STACK}) AllocatableValue result;
54+
@Def({REG}) AllocatableValue result;
5655

5756
public LoadHotSpotObjectConstantInline(HotSpotConstant constant, AllocatableValue result) {
5857
super(TYPE);
@@ -80,6 +79,11 @@ public AllocatableValue getResult() {
8079
public Constant getConstant() {
8180
return constant;
8281
}
82+
83+
@Override
84+
public boolean canRematerializeToStack() {
85+
return false;
86+
}
8387
}
8488

8589
/**

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotMove.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2025, 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
@@ -41,7 +41,6 @@
4141
import org.graalvm.compiler.lir.StandardOp.LoadConstantOp;
4242
import org.graalvm.compiler.lir.amd64.AMD64LIRInstruction;
4343
import org.graalvm.compiler.lir.asm.CompilationResultBuilder;
44-
4544
import jdk.vm.ci.code.Register;
4645
import jdk.vm.ci.hotspot.HotSpotMetaspaceConstant;
4746
import jdk.vm.ci.hotspot.HotSpotObjectConstant;
@@ -104,6 +103,16 @@ public Constant getConstant() {
104103
public AllocatableValue getResult() {
105104
return result;
106105
}
106+
107+
@Override
108+
public boolean canRematerializeToStack() {
109+
/*
110+
* This is slightly too lenient, formally we would also need to check if the target
111+
* allows inlining objects. In practice that is always true, and we do not have access
112+
* to the relevant information here.
113+
*/
114+
return input.isCompressed();
115+
}
107116
}
108117

109118
public static final class BaseMove extends AMD64LIRInstruction {
@@ -167,6 +176,11 @@ public Constant getConstant() {
167176
public AllocatableValue getResult() {
168177
return result;
169178
}
179+
180+
@Override
181+
public boolean canRematerializeToStack() {
182+
return input.isCompressed();
183+
}
170184
}
171185

172186
public static void decodeKlassPointer(AMD64MacroAssembler masm, Register register, Register scratch, AMD64Address address, GraalHotSpotVMConfig config) {

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/StandardOp.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2025, 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
@@ -292,6 +292,14 @@ static boolean isLoadConstantOp(LIRInstruction op) {
292292
return op.isLoadConstantOp();
293293
}
294294

295+
/**
296+
* Determine whether this constant load is safe to rematerialize directly to a stack slot.
297+
* Some constant load operations have special semantics or implementations that only allow
298+
* rematerialization to a register. Such operations must implement this method to return
299+
* {@code false}. Simple constant loads that can be implemented using a move-immediate
300+
* instruction to a stack slot, or similar, should return {@code true}.
301+
*/
302+
boolean canRematerializeToStack();
295303
}
296304

297305
/**

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/aarch64/AArch64Move.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2025, 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
@@ -108,6 +108,11 @@ public Constant getConstant() {
108108
public AllocatableValue getResult() {
109109
return result;
110110
}
111+
112+
@Override
113+
public boolean canRematerializeToStack() {
114+
return true;
115+
}
111116
}
112117

113118
@Opcode("MOVE")

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/alloc/lsra/LinearScanLifetimeAnalysisPhase.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2025, 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
@@ -925,6 +925,15 @@ protected Constant getMaterializedValue(LIRInstruction op, Value operand, Interv
925925
LoadConstantOp move = LoadConstantOp.asLoadConstantOp(op);
926926

927927
if (!allocator.neverSpillConstants()) {
928+
if (!move.canRematerializeToStack()) {
929+
/*
930+
* This allocator cannot ensure that a rematerialized value is always assigned a
931+
* register; sometimes it rematerializes directly to a stack slot. Therefore, we
932+
* must not try to rematerialize constant loads that do not support
933+
* rematerialization to the stack.
934+
*/
935+
return null;
936+
}
928937
/*
929938
* Check if the interval has any uses which would accept an stack location (priority
930939
* == ShouldHaveRegister). Rematerialization of such intervals can result in a

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/amd64/AMD64Move.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import jdk.vm.ci.meta.AllocatableValue;
7272
import jdk.vm.ci.meta.Constant;
7373
import jdk.vm.ci.meta.JavaConstant;
74+
import jdk.vm.ci.meta.JavaKind;
7475
import jdk.vm.ci.meta.Value;
7576

7677
public class AMD64Move {
@@ -171,6 +172,14 @@ public Constant getConstant() {
171172
public AllocatableValue getResult() {
172173
return result;
173174
}
175+
176+
@Override
177+
public boolean canRematerializeToStack() {
178+
if (input.getJavaKind() == JavaKind.Object) {
179+
return input.isNull();
180+
}
181+
return true;
182+
}
174183
}
175184

176185
@Opcode("STACKMOVE")

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/amd64/vector/AMD64VectorMove.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2025, 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
@@ -161,13 +161,18 @@ public Constant getConstant() {
161161
public AllocatableValue getResult() {
162162
return result;
163163
}
164+
165+
@Override
166+
public boolean canRematerializeToStack() {
167+
return true;
168+
}
164169
}
165170

166171
@Opcode("VMOVE")
167172
public static class MoveFromArrayConstOp extends AMD64LIRInstruction implements LoadConstantOp {
168173
public static final LIRInstructionClass<MoveFromArrayConstOp> TYPE = LIRInstructionClass.create(MoveFromArrayConstOp.class);
169174

170-
@Def({REG, STACK}) protected AllocatableValue result;
175+
@Def({REG}) protected AllocatableValue result;
171176
private final DataPointerConstant input;
172177

173178
public MoveFromArrayConstOp(AllocatableValue result, DataPointerConstant input) {
@@ -193,6 +198,11 @@ public Constant getConstant() {
193198
public AllocatableValue getResult() {
194199
return result;
195200
}
201+
202+
@Override
203+
public boolean canRematerializeToStack() {
204+
return false;
205+
}
196206
}
197207

198208
@Opcode("VSTACKMOVE")

‎substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/AArch64LoadMethodPointerConstantOp.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, 2025, 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
@@ -28,14 +28,14 @@
2828
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.HINT;
2929
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.REG;
3030

31+
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
32+
3133
import org.graalvm.compiler.asm.aarch64.AArch64MacroAssembler;
3234
import org.graalvm.compiler.lir.LIRInstructionClass;
3335
import org.graalvm.compiler.lir.StandardOp;
3436
import org.graalvm.compiler.lir.aarch64.AArch64LIRInstruction;
3537
import org.graalvm.compiler.lir.asm.CompilationResultBuilder;
3638

37-
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
38-
3939
import jdk.vm.ci.code.Register;
4040
import jdk.vm.ci.meta.AllocatableValue;
4141
import jdk.vm.ci.meta.Constant;
@@ -67,4 +67,9 @@ public AllocatableValue getResult() {
6767
public Constant getConstant() {
6868
return constant;
6969
}
70+
71+
@Override
72+
public boolean canRematerializeToStack() {
73+
return false;
74+
}
7075
}

‎substratevm/src/com.oracle.svm.core.graal.aarch64/src/com/oracle/svm/core/graal/aarch64/SubstrateAArch64Backend.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, 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
@@ -1184,6 +1184,12 @@ public void emitCode(CompilationResultBuilder crb, AArch64MacroAssembler masm) {
11841184
masm.add(64, resultReg, baseReg, resultReg, ShiftType.LSL, getShift());
11851185
}
11861186
}
1187+
1188+
@Override
1189+
public boolean canRematerializeToStack() {
1190+
/* This operation MUST have a register as its destination. */
1191+
return false;
1192+
}
11871193
}
11881194

11891195
public FrameMapBuilder newFrameMapBuilder(RegisterConfig registerConfig) {

‎substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64LoadMethodPointerConstantOp.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2025, 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
@@ -29,15 +29,14 @@
2929
import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.REG;
3030

3131
import com.oracle.svm.core.FrameAccess;
32+
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
3233
import org.graalvm.compiler.asm.amd64.AMD64Address;
3334
import org.graalvm.compiler.asm.amd64.AMD64MacroAssembler;
3435
import org.graalvm.compiler.lir.LIRInstructionClass;
3536
import org.graalvm.compiler.lir.StandardOp;
3637
import org.graalvm.compiler.lir.amd64.AMD64LIRInstruction;
3738
import org.graalvm.compiler.lir.asm.CompilationResultBuilder;
3839

39-
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
40-
4140
import jdk.vm.ci.code.Register;
4241
import jdk.vm.ci.meta.AllocatableValue;
4342
import org.graalvm.nativeimage.Platform;
@@ -74,4 +73,9 @@ public AllocatableValue getResult() {
7473
public SubstrateMethodPointerConstant getConstant() {
7574
return constant;
7675
}
76+
77+
@Override
78+
public boolean canRematerializeToStack() {
79+
return false;
80+
}
7781
}

0 commit comments

Comments
 (0)