Skip to content

Commit cb22258

Browse files
teshullteshull
committed
Revert "[GR-43389] [darwin-amd64] Workaround for buggy ld64 versions"
This reverts commit e30698b.
1 parent 3fe9eba commit cb22258

File tree

6 files changed

+25
-66
lines changed

6 files changed

+25
-66
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.movq(resultReg, 0L, true);
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/amd64/AMD64HostedPatcherFeature.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) {
126126
VMConstant constant = ((ConstantReference) ref).getConstant();
127127
Object relocVal = ref;
128128
if (constant instanceof SubstrateMethodPointerConstant) {
129-
VMError.guarantee(!Platform.includedIn(Platform.DARWIN_AMD64.class), "[GR-43389] method pointer relocations should not be inlined.");
130129
MethodPointer pointer = ((SubstrateMethodPointerConstant) constant).pointer();
131130
HostedMethod hMethod = (HostedMethod) pointer.getMethod();
132131
VMError.guarantee(hMethod.isCompiled(), "Method %s is not compiled although there is a method pointer constant created for it.", hMethod);

‎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

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

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.graalvm.compiler.debug.DebugContext;
3838
import org.graalvm.compiler.debug.Indent;
3939
import org.graalvm.nativeimage.ImageSingletons;
40-
import org.graalvm.nativeimage.Platform;
4140
import org.graalvm.nativeimage.c.function.CFunctionPointer;
4241
import org.graalvm.nativeimage.c.function.RelocatedPointer;
4342
import org.graalvm.word.WordBase;
@@ -169,16 +168,10 @@ private void write(RelocatableBuffer buffer, int index, JavaConstant con, Object
169168
private final boolean useHeapBase = NativeImageHeap.useHeapBase();
170169
private final CompressEncoding compressEncoding = ImageSingletons.lookup(CompressEncoding.class);
171170

172-
void writeReference(RelocatableBuffer buffer, int index, Constant constant, Object reason) {
171+
void writeReference(RelocatableBuffer buffer, int index, JavaConstant target, Object reason) {
172+
assert !(heap.hMetaAccess.isInstanceOf(target, WordBase.class)) : "word values are not references";
173173
mustBeReferenceAligned(index);
174-
175-
if (constant instanceof JavaConstant target) {
176-
assert !(heap.hMetaAccess.isInstanceOf(target, WordBase.class)) : "word values are not references";
177-
178-
if (target.isNull()) {
179-
return;
180-
}
181-
174+
if (target.isNonNull()) {
182175
ObjectInfo targetInfo = heap.getConstantInfo(target);
183176
verifyTargetDidNotChange(target, reason, targetInfo);
184177
if (useHeapBase) {
@@ -187,9 +180,6 @@ void writeReference(RelocatableBuffer buffer, int index, Constant constant, Obje
187180
} else {
188181
addDirectRelocationWithoutAddend(buffer, index, referenceSize(), target);
189182
}
190-
} else {
191-
assert Platform.includedIn(Platform.DARWIN_AMD64.class) : "[GR-43389] Workaround for ld64 bug that does not allow direct8 relocations in .text on amd64";
192-
buffer.addRelocationWithoutAddend(index, ObjectFile.RelocationKind.DIRECT_8, ((SubstrateMethodPointerConstant) constant).pointer());
193183
}
194184
}
195185

0 commit comments

Comments
 (0)