Skip to content

Commit a27caf3

Browse files
teshullteshull
committed
[GR-47886] Directly patch LoadMethod Constants.
PullRequest: graal/15723
2 parents 07e130b + 946090a commit a27caf3

File tree

8 files changed

+57
-103
lines changed

8 files changed

+57
-103
lines changed

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
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.FrameAccess;
32-
import org.graalvm.compiler.asm.amd64.AMD64Address;
3331
import org.graalvm.compiler.asm.amd64.AMD64MacroAssembler;
3432
import org.graalvm.compiler.lir.LIRInstructionClass;
3533
import org.graalvm.compiler.lir.StandardOp;
@@ -40,7 +38,6 @@
4038

4139
import jdk.vm.ci.code.Register;
4240
import jdk.vm.ci.meta.AllocatableValue;
43-
import org.graalvm.nativeimage.Platform;
4441

4542
public final class AMD64LoadMethodPointerConstantOp extends AMD64LIRInstruction implements StandardOp.LoadConstantOp {
4643
public static final LIRInstructionClass<AMD64LoadMethodPointerConstantOp> TYPE = LIRInstructionClass.create(AMD64LoadMethodPointerConstantOp.class);
@@ -56,13 +53,8 @@ public final class AMD64LoadMethodPointerConstantOp extends AMD64LIRInstruction
5653
@Override
5754
public void emitCode(CompilationResultBuilder crb, AMD64MacroAssembler masm) {
5855
Register resultReg = asRegister(result);
59-
if (!Platform.includedIn(Platform.DARWIN_AMD64.class)) {
60-
crb.recordInlineDataInCode(constant);
61-
masm.movq(resultReg, 0L, true);
62-
} else {
63-
/* [GR-43389] ld64 bug does not allow direct8 relocations in .text on darwin */
64-
masm.movq(resultReg, (AMD64Address) crb.recordDataReferenceInCode(constant, FrameAccess.wordSize()));
65-
}
56+
crb.recordInlineDataInCode(constant);
57+
masm.leaq(resultReg, masm.getPlaceholder(masm.position()));
6658
}
6759

6860
@Override

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/code/SubstrateDataBuilder.java‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
import java.nio.ByteBuffer;
3030

31-
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
3231
import org.graalvm.compiler.code.DataSection.Data;
3332
import org.graalvm.compiler.code.DataSection.Patches;
3433
import org.graalvm.compiler.core.common.type.CompressibleConstant;
@@ -51,11 +50,8 @@ public class SubstrateDataBuilder extends DataBuilder {
5150
@Override
5251
public Data createDataItem(Constant constant) {
5352
int size;
54-
if (constant instanceof SubstrateMethodPointerConstant methodPointerConstant) {
55-
size = FrameAccess.wordSize();
56-
return new ObjectData(size, size, methodPointerConstant);
57-
} else if (constant instanceof VMConstant vmConstant) {
58-
assert constant instanceof CompressibleConstant && constant instanceof TypedConstant : constant;
53+
if (constant instanceof VMConstant vmConstant) {
54+
assert constant instanceof JavaConstant && constant instanceof CompressibleConstant && constant instanceof TypedConstant : constant;
5955
return new ObjectData(vmConstant);
6056
} else if (JavaConstant.isNull(constant)) {
6157
if (SubstrateObjectConstant.isCompressed((JavaConstant) constant)) {
@@ -64,8 +60,9 @@ public Data createDataItem(Constant constant) {
6460
size = FrameAccess.uncompressedReferenceSize();
6561
}
6662
return createZeroData(size, size);
67-
} else if (constant instanceof SerializableConstant serializableConstant) {
68-
return createSerializableData(serializableConstant);
63+
} else if (constant instanceof SerializableConstant) {
64+
SerializableConstant s = (SerializableConstant) constant;
65+
return createSerializableData(s);
6966
} else {
7067
throw new JVMCIError(String.valueOf(constant));
7168
}
@@ -74,19 +71,15 @@ public Data createDataItem(Constant constant) {
7471
public static class ObjectData extends Data {
7572
private final VMConstant constant;
7673

77-
protected ObjectData(int alignment, int size, VMConstant constant) {
78-
super(alignment, size);
79-
this.constant = constant;
80-
}
81-
8274
protected ObjectData(VMConstant constant) {
83-
this(ConfigurationValues.getObjectLayout().getReferenceSize(), ConfigurationValues.getObjectLayout().getReferenceSize(), constant);
75+
super(ConfigurationValues.getObjectLayout().getReferenceSize(), ConfigurationValues.getObjectLayout().getReferenceSize());
8476
assert ((CompressibleConstant) constant).isCompressed() == ReferenceAccess.singleton()
8577
.haveCompressedReferences() : "Constant object references in compiled code must be compressed (base-relative)";
78+
this.constant = constant;
8679
}
8780

88-
public VMConstant getConstant() {
89-
return constant;
81+
public JavaConstant getConstant() {
82+
return (JavaConstant) constant;
9083
}
9184

9285
@Override
@@ -99,7 +92,7 @@ public static void emit(ByteBuffer buffer, Patches patches, int size, VMConstant
9992
if (size == Integer.BYTES) {
10093
buffer.putInt(0);
10194
} else if (size == Long.BYTES) {
102-
buffer.putLong(0);
95+
buffer.putLong(0L);
10396
} else {
10497
shouldNotReachHere("Unsupported object constant reference size: " + size);
10598
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import java.util.concurrent.ConcurrentMap;
4040
import java.util.concurrent.ForkJoinPool;
4141

42-
import com.oracle.svm.core.graal.code.SubstrateDataBuilder;
4342
import org.graalvm.collections.EconomicMap;
4443
import org.graalvm.compiler.api.replacements.Fold;
4544
import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
@@ -123,6 +122,7 @@
123122
import com.oracle.svm.core.graal.phases.OptimizeExceptionPathsPhase;
124123
import com.oracle.svm.core.heap.RestrictHeapAccess;
125124
import com.oracle.svm.core.heap.RestrictHeapAccessCallees;
125+
import com.oracle.svm.core.meta.MethodPointer;
126126
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
127127
import com.oracle.svm.core.util.InterruptImageBuilding;
128128
import com.oracle.svm.core.util.VMError;
@@ -151,7 +151,6 @@
151151
import jdk.vm.ci.meta.MetaAccessProvider;
152152
import jdk.vm.ci.meta.ResolvedJavaMethod;
153153
import jdk.vm.ci.meta.VMConstant;
154-
import org.graalvm.nativeimage.Platform;
155154

156155
public class CompileQueue {
157156

@@ -1369,29 +1368,15 @@ protected void removeDeoptTargetOptimizations(LIRSuites lirSuites) {
13691368
DeoptimizationUtils.removeDeoptTargetOptimizations(lirSuites);
13701369
}
13711370

1372-
private void ensureCompiledForMethodPointerConstant(HostedMethod method, CompileReason reason, SubstrateMethodPointerConstant methodPointerConstant) {
1373-
HostedMethod referencedMethod = (HostedMethod) methodPointerConstant.pointer().getMethod();
1374-
ensureCompiled(referencedMethod, new MethodPointerConstantReason(method, referencedMethod, reason));
1375-
}
1376-
13771371
protected final void ensureCompiledForMethodPointerConstants(HostedMethod method, CompileReason reason, CompilationResult result) {
13781372
for (DataPatch dataPatch : result.getDataPatches()) {
13791373
Reference reference = dataPatch.reference;
1380-
if (reference instanceof ConstantReference constantReference) {
1381-
VMConstant vmConstant = constantReference.getConstant();
1382-
if (vmConstant instanceof SubstrateMethodPointerConstant methodPointerConstant) {
1383-
ensureCompiledForMethodPointerConstant(method, reason, methodPointerConstant);
1384-
}
1385-
}
1386-
}
1387-
1388-
for (DataSection.Data data : result.getDataSection()) {
1389-
if (data instanceof SubstrateDataBuilder.ObjectData objectData) {
1390-
VMConstant vmConstant = objectData.getConstant();
1391-
if (vmConstant instanceof SubstrateMethodPointerConstant methodPointerConstant) {
1392-
/* [GR-43389] Only reachable with ld64 workaround on */
1393-
VMError.guarantee(Platform.includedIn(Platform.DARWIN_AMD64.class));
1394-
ensureCompiledForMethodPointerConstant(method, reason, methodPointerConstant);
1374+
if (reference instanceof ConstantReference) {
1375+
VMConstant constant = ((ConstantReference) reference).getConstant();
1376+
if (constant instanceof SubstrateMethodPointerConstant) {
1377+
MethodPointer pointer = ((SubstrateMethodPointerConstant) constant).pointer();
1378+
HostedMethod referencedMethod = (HostedMethod) pointer.getMethod();
1379+
ensureCompiled(referencedMethod, new MethodPointerConstantReason(method, referencedMethod, reason));
13951380
}
13961381
}
13971382
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcherFeature.java‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,14 @@
4141
import com.oracle.svm.core.feature.InternalFeature;
4242
import com.oracle.svm.core.graal.code.CGlobalDataReference;
4343
import com.oracle.svm.core.graal.code.PatchConsumerFactory;
44-
import com.oracle.svm.core.meta.MethodPointer;
4544
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
4645
import com.oracle.svm.core.util.VMError;
4746
import com.oracle.svm.hosted.code.HostedPatcher;
4847
import com.oracle.svm.hosted.image.RelocatableBuffer;
49-
import com.oracle.svm.hosted.meta.HostedMethod;
5048

5149
import jdk.vm.ci.code.site.ConstantReference;
5250
import jdk.vm.ci.code.site.DataSectionReference;
5351
import jdk.vm.ci.code.site.Reference;
54-
import jdk.vm.ci.meta.VMConstant;
5552

5653
@AutomaticallyRegisteredFeature
5754
@Platforms({Platform.AARCH64.class})
@@ -172,22 +169,17 @@ class AdrpAddMacroInstructionHostedPatcher extends CompilationResult.CodeAnnotat
172169

173170
@Override
174171
public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) {
175-
Object relocVal = ref;
176-
if (ref instanceof ConstantReference) {
177-
VMConstant constant = ((ConstantReference) ref).getConstant();
178-
if (constant instanceof SubstrateMethodPointerConstant) {
179-
MethodPointer pointer = ((SubstrateMethodPointerConstant) constant).pointer();
180-
HostedMethod hMethod = (HostedMethod) pointer.getMethod();
181-
VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod);
182-
relocVal = pointer;
183-
}
172+
if (ref instanceof ConstantReference constantRef) {
173+
VMError.guarantee(!(constantRef.getConstant() instanceof SubstrateMethodPointerConstant), "SubstrateMethodPointerConstants should not be relocated %s", constantRef);
174+
} else {
175+
VMError.guarantee(ref instanceof DataSectionReference || ref instanceof CGlobalDataReference, "Unexpected reference: %s", ref);
184176
}
185177

186178
int siteOffset = compStart + macroInstruction.instructionPosition;
187-
relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADR_PREL_PG_HI21, relocVal);
179+
relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADR_PREL_PG_HI21, ref);
188180

189181
siteOffset += 4;
190-
relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADD_ABS_LO12_NC, relocVal);
182+
relocs.addRelocationWithoutAddend(siteOffset, RelocationKind.AARCH64_R_AARCH64_ADD_ABS_LO12_NC, ref);
191183
}
192184

193185
@Uninterruptible(reason = ".")
@@ -221,8 +213,8 @@ public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) {
221213
*/
222214
int siteOffset = compStart + annotation.instructionPosition;
223215
if (ref instanceof DataSectionReference || ref instanceof CGlobalDataReference || ref instanceof ConstantReference) {
224-
if (ref instanceof ConstantReference) {
225-
assert !(((ConstantReference) ref).getConstant() instanceof SubstrateMethodPointerConstant);
216+
if (ref instanceof ConstantReference constantRef) {
217+
VMError.guarantee(!(constantRef.getConstant() instanceof SubstrateMethodPointerConstant), "SubstrateMethodPointerConstants should not be relocated %s", constantRef);
226218
}
227219
/*
228220
* calculating the last mov index. This is necessary ensure the proper overflow checks

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/amd64/AMD64HostedPatcherFeature.java‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,16 @@
4040
import com.oracle.svm.core.feature.InternalFeature;
4141
import com.oracle.svm.core.graal.code.CGlobalDataReference;
4242
import com.oracle.svm.core.graal.code.PatchConsumerFactory;
43-
import com.oracle.svm.core.meta.MethodPointer;
4443
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
4544
import com.oracle.svm.core.util.VMError;
4645
import com.oracle.svm.hosted.code.HostedImageHeapConstantPatch;
4746
import com.oracle.svm.hosted.code.HostedPatcher;
4847
import com.oracle.svm.hosted.image.RelocatableBuffer;
49-
import com.oracle.svm.hosted.meta.HostedMethod;
5048

5149
import jdk.vm.ci.code.site.ConstantReference;
5250
import jdk.vm.ci.code.site.DataSectionReference;
5351
import jdk.vm.ci.code.site.Reference;
5452
import jdk.vm.ci.meta.JavaConstant;
55-
import jdk.vm.ci.meta.VMConstant;
5653

5754
@AutomaticallyRegisteredFeature
5855
@Platforms({Platform.AMD64.class})
@@ -122,17 +119,9 @@ public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) {
122119
*/
123120
long addend = (annotation.nextInstructionPosition - annotation.operandPosition);
124121
relocs.addRelocationWithAddend((int) siteOffset, ObjectFile.RelocationKind.getPCRelative(annotation.operandSize), addend, ref);
125-
} else if (ref instanceof ConstantReference) {
126-
VMConstant constant = ((ConstantReference) ref).getConstant();
127-
Object relocVal = ref;
128-
if (constant instanceof SubstrateMethodPointerConstant) {
129-
VMError.guarantee(!Platform.includedIn(Platform.DARWIN_AMD64.class), "[GR-43389] method pointer relocations should not be inlined.");
130-
MethodPointer pointer = ((SubstrateMethodPointerConstant) constant).pointer();
131-
HostedMethod hMethod = (HostedMethod) pointer.getMethod();
132-
VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod);
133-
relocVal = pointer;
134-
}
135-
relocs.addRelocationWithoutAddend((int) siteOffset, ObjectFile.RelocationKind.getDirect(annotation.operandSize), relocVal);
122+
} else if (ref instanceof ConstantReference constantRef) {
123+
VMError.guarantee(!(constantRef.getConstant() instanceof SubstrateMethodPointerConstant), "SubstrateMethodPointerConstants should not be relocated %s", constantRef);
124+
relocs.addRelocationWithoutAddend((int) siteOffset, ObjectFile.RelocationKind.getDirect(annotation.operandSize), ref);
136125
} else {
137126
throw VMError.shouldNotReachHere("Unknown type of reference in code");
138127
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/LIRNativeImageCodeCache.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.oracle.objectfile.ObjectFile;
4646
import com.oracle.svm.core.SubstrateOptions;
4747
import com.oracle.svm.core.config.ConfigurationValues;
48+
import com.oracle.svm.core.meta.SubstrateMethodPointerConstant;
4849
import com.oracle.svm.core.util.VMError;
4950
import com.oracle.svm.hosted.code.HostedDirectCallTrampolineSupport;
5051
import com.oracle.svm.hosted.code.HostedImageHeapConstantPatch;
@@ -54,6 +55,7 @@
5455

5556
import jdk.vm.ci.code.TargetDescription;
5657
import jdk.vm.ci.code.site.Call;
58+
import jdk.vm.ci.code.site.ConstantReference;
5759
import jdk.vm.ci.code.site.DataPatch;
5860
import jdk.vm.ci.code.site.Infopoint;
5961
import jdk.vm.ci.code.site.Reference;
@@ -343,7 +345,7 @@ public void patchMethods(DebugContext debug, RelocatableBuffer relocs, ObjectFil
343345
// the codecache-relative offset of the compilation
344346
int compStart = method.getCodeAddressOffset();
345347

346-
// Build an index of PatchingAnnoations
348+
// Build an index of PatchingAnnotations
347349
Map<Integer, HostedPatcher> patches = new HashMap<>();
348350
ByteBuffer targetCode = null;
349351
for (CodeAnnotation codeAnnotation : compilation.getCodeAnnotations()) {
@@ -395,12 +397,25 @@ public void patchMethods(DebugContext debug, RelocatableBuffer relocs, ObjectFil
395397
}
396398
}
397399
for (DataPatch dataPatch : compilation.getDataPatches()) {
400+
assert dataPatch.note == null : "Unexpected note: " + dataPatch.note;
398401
Reference ref = dataPatch.reference;
399-
/*
400-
* Constants are allocated offsets in a separate space, which can be emitted as
401-
* read-only (.rodata) section.
402-
*/
403-
patches.get(dataPatch.pcOffset).relocate(ref, relocs, compStart);
402+
var patcher = patches.get(dataPatch.pcOffset);
403+
if (ref instanceof ConstantReference constant && constant.getConstant() instanceof SubstrateMethodPointerConstant methodPtrConstant) {
404+
/*
405+
* We directly patch SubstrateMethodPointerConstants.
406+
*/
407+
HostedMethod hMethod = (HostedMethod) methodPtrConstant.pointer().getMethod();
408+
VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod);
409+
int targetOffset = hMethod.getCodeAddressOffset();
410+
int pcDisplacement = targetOffset - (compStart + dataPatch.pcOffset);
411+
patcher.patch(compStart, pcDisplacement, compilation.getTargetCode());
412+
} else {
413+
/*
414+
* Constants are allocated offsets in a separate space, which can be emitted as
415+
* read-only (.rodata) section.
416+
*/
417+
patcher.relocate(ref, relocs, compStart);
418+
}
404419
boolean noPriorMatch = patchedOffsets.add(dataPatch.pcOffset);
405420
VMError.guarantee(noPriorMatch, "Patching same offset twice.");
406421
patchesHandled++;

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public void layoutConstants() {
197197
CompilationResult compilation = pair.getRight();
198198
for (DataSection.Data data : compilation.getDataSection()) {
199199
if (data instanceof SubstrateDataBuilder.ObjectData) {
200-
VMConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
200+
JavaConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
201201
constantReasons.put(constant, compilation.getName());
202202
}
203203
}
@@ -217,7 +217,7 @@ public void layoutConstants() {
217217
public void addConstantsToHeap() {
218218
for (DataSection.Data data : dataSection) {
219219
if (data instanceof SubstrateDataBuilder.ObjectData) {
220-
VMConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
220+
JavaConstant constant = ((SubstrateDataBuilder.ObjectData) data).getConstant();
221221
addConstantToHeap(constant, NativeImageHeap.HeapInclusionReason.DataSection);
222222
}
223223
}
@@ -599,7 +599,7 @@ protected boolean verifyMethods(HostedUniverse hUniverse, ForkJoinPool threadPoo
599599
public void writeConstants(NativeImageHeapWriter writer, RelocatableBuffer buffer) {
600600
ByteBuffer bb = buffer.getByteBuffer();
601601
dataSection.buildDataSection(bb, (position, constant) -> {
602-
writer.writeReference(buffer, position, constant, "VMConstant: " + constant);
602+
writer.writeReference(buffer, position, (JavaConstant) constant, "VMConstant: " + constant);
603603
});
604604
}
605605

0 commit comments

Comments
 (0)