Skip to content

Commit a21c01d

Browse files
tzezulatzezula
committed
Resolved review comments.
1 parent e7c684f commit a21c01d

File tree

7 files changed

+63
-60
lines changed

7 files changed

+63
-60
lines changed

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/HotSpotGraalVMEventListener.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
*/
2525
package jdk.graal.compiler.hotspot;
2626

27-
import jdk.graal.compiler.code.CompilationResult;
2827
import jdk.graal.compiler.debug.DebugContext;
2928
import jdk.graal.compiler.debug.DebugOptions;
3029
import jdk.vm.ci.code.CompiledCode;
@@ -56,8 +55,6 @@ public void notifyShutdown() {
5655
public void notifyInstall(HotSpotCodeCacheProvider codeCache, InstalledCode installedCode, CompiledCode compiledCode) {
5756
DebugContext debug = DebugContext.forCurrentThread();
5857
if (debug.isDumpEnabled(DebugContext.BASIC_LEVEL)) {
59-
CompilationResult compResult = debug.contextLookup(CompilationResult.class);
60-
assert compResult != null : "can't dump installed code properly without CompilationResult";
6158
debug.dump(DebugContext.BASIC_LEVEL, installedCode, "After code installation");
6259
}
6360
if (debug.isLogEnabled()) {

‎substratevm/mx.substratevm/mx_substratevm.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,8 @@ def truffle_unittest_task(extra_build_args=None):
412412
if match and int(match.group(1), 16) != 0:
413413
success = True
414414
break
415+
if not success:
416+
mx.abort(f"Failed to find expected PrintCompilation output in log file: {logfile.name}.")
415417
finally:
416418
if success:
417419
os.unlink(logfile.name)

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolateAwareCodeCacheProvider.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ public InstalledCode installCode(ResolvedJavaMethod method, CompiledCode compile
7272
installBridge.setSubstrateInstalledCodeHandle(installedCode);
7373
DebugContext debug = DebugContext.forCurrentThread();
7474
if (debug.isDumpEnabled(DebugContext.BASIC_LEVEL)) {
75-
assert debug.contextLookup(CompilationResult.class) != null : "can't dump installed code properly without CompilationResult";
7675
debug.dump(DebugContext.BASIC_LEVEL, installBridge, "After code installation");
7776
}
7877
return installBridge;

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/isolated/IsolatedCodeInstallBridge.java

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.graalvm.nativeimage.c.function.CEntryPoint;
4040
import org.graalvm.nativeimage.c.function.CodePointer;
4141
import org.graalvm.nativeimage.c.type.CTypeConversion;
42-
import org.graalvm.word.WordFactory;
4342

4443
/**
4544
* A helper to pass information for installing code in the compilation client through a Truffle
@@ -76,6 +75,10 @@ public String getName() {
7675
throw VMError.shouldNotReachHere(DO_NOT_CALL_REASON);
7776
}
7877

78+
/**
79+
* This method is used by the compiler debugging feature
80+
* {@code jdk.graal.PrintCompilation=true}.
81+
*/
7982
@Override
8083
public long getStart() {
8184
ClientHandle<? extends SubstrateInstalledCode> handle = installedCodeHandle;
@@ -86,6 +89,10 @@ public long getStart() {
8689
}
8790
}
8891

92+
/**
93+
* This method is used by the compiler debugging feature {@code jdk.graal.Dump=CodeInstall} to
94+
* dump a code at the point of code installation.
95+
*/
8996
@Override
9097
public boolean isValid() {
9198
ClientHandle<? extends SubstrateInstalledCode> handle = installedCodeHandle;
@@ -101,6 +108,10 @@ public boolean isAlive() {
101108
throw VMError.shouldNotReachHere(DO_NOT_CALL_REASON);
102109
}
103110

111+
/**
112+
* This method is used by the compiler debugging feature {@code jdk.graal.Dump=CodeInstall} to
113+
* dump a code at the point of code installation.
114+
*/
104115
@Override
105116
public byte[] getCode() {
106117
ClientHandle<? extends SubstrateInstalledCode> handle = installedCodeHandle;
@@ -140,31 +151,31 @@ private static boolean isValid0(@SuppressWarnings("unused") ClientIsolateThread
140151
@CEntryPoint(include = CEntryPoint.NotIncludedAutomatically.class, publishAs = CEntryPoint.Publish.NotPublished)
141152
private static CompilerHandle<byte[]> getCode0(@SuppressWarnings("unused") ClientIsolateThread client, ClientHandle<? extends SubstrateInstalledCode> installedCodeHandle) {
142153
SubstrateInstalledCode installedCode = IsolatedCompileClient.get().unhand(installedCodeHandle);
143-
CodeInfo codeInfo = lookupCodeInfo(installedCode);
144-
if (codeInfo.isNull()) {
145-
return IsolatedHandles.nullHandle();
146-
}
147-
CodePointer codeStart = CodeInfoAccess.getCodeStart(codeInfo);
148-
int codeSize = (int) CodeInfoAccess.getCodeSize(codeInfo).rawValue();
149-
return copyCode0(IsolatedCompileClient.get().getCompiler(), codeStart, codeSize);
154+
return getCodeUninterruptible(Word.pointer(installedCode.getEntryPoint()));
150155
}
151156

152-
@Uninterruptible(reason = "Looks up code info.")
153-
private static CodeInfo lookupCodeInfo(SubstrateInstalledCode installedCode) {
154-
UntetheredCodeInfo untetheredCodeInfo = CodeInfoTable.lookupCodeInfo(Word.pointer(installedCode.getEntryPoint()));
157+
@Uninterruptible(reason = "Accesses code info.")
158+
private static CompilerHandle<byte[]> getCodeUninterruptible(CodePointer entryPointAddress) {
159+
UntetheredCodeInfo untetheredCodeInfo = CodeInfoTable.lookupCodeInfo(entryPointAddress);
155160
if (untetheredCodeInfo.isNull()) {
156-
return WordFactory.nullPointer();
161+
return IsolatedHandles.nullHandle();
157162
}
158163
Object tether = CodeInfoAccess.acquireTether(untetheredCodeInfo);
159164
try {
160-
return CodeInfoAccess.convert(untetheredCodeInfo, tether);
165+
CodeInfo codeInfo = CodeInfoAccess.convert(untetheredCodeInfo, tether);
166+
return copyCode0(codeInfo);
161167
} finally {
162168
CodeInfoAccess.releaseTether(untetheredCodeInfo, tether);
163169
}
164170
}
165171

172+
@Uninterruptible(reason = "Wrap the now safe call to code info.", calleeMustBe = false)
173+
private static CompilerHandle<byte[]> copyCode0(CodeInfo codeInfo) {
174+
return copyCodeInCompilerIsolate0(IsolatedCompileClient.get().getCompiler(), CodeInfoAccess.getCodeStart(codeInfo), (int) CodeInfoAccess.getCodeSize(codeInfo).rawValue());
175+
}
176+
166177
@CEntryPoint(include = CEntryPoint.NotIncludedAutomatically.class, publishAs = CEntryPoint.Publish.NotPublished)
167-
private static CompilerHandle<byte[]> copyCode0(@SuppressWarnings("unused") @CEntryPoint.IsolateThreadContext CompilerIsolateThread context, CodePointer codeStart, int codeSize) {
178+
private static CompilerHandle<byte[]> copyCodeInCompilerIsolate0(@SuppressWarnings("unused") @CEntryPoint.IsolateThreadContext CompilerIsolateThread context, CodePointer codeStart, int codeSize) {
168179
byte[] code = new byte[codeSize];
169180
CTypeConversion.asByteBuffer(codeStart, codeSize).get(code);
170181
return IsolatedCompileContext.get().hand(code);

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateCodeCacheProvider.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ public InstalledCode installCode(ResolvedJavaMethod method, CompiledCode compile
6363
RuntimeCodeInstaller.install((SharedRuntimeMethod) method, compResult, substrateInstalledCode);
6464
DebugContext debug = DebugContext.forCurrentThread();
6565
if (debug.isDumpEnabled(DebugContext.BASIC_LEVEL)) {
66-
assert debug.contextLookup(CompilationResult.class) != null : "can't dump installed code properly without CompilationResult";
6766
debug.dump(DebugContext.BASIC_LEVEL, substrateInstalledCode, "After code installation");
6867
}
6968
return predefinedInstalledCode;

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateInstalledCodeImpl.java

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import jdk.vm.ci.meta.ResolvedJavaMethod;
4545
import org.graalvm.nativeimage.c.function.CodePointer;
4646
import org.graalvm.nativeimage.c.type.CTypeConversion;
47-
import org.graalvm.word.WordFactory;
4847

4948
/**
5049
* Represents the installed code of a runtime compiled method. Note that Truffle uses its own
@@ -125,38 +124,50 @@ public SubstrateSpeculationLog getSpeculationLog() {
125124
public void setCompilationId(CompilationIdentifier id) {
126125
}
127126

127+
/**
128+
* This method is used by the compiler debugging feature
129+
* {@code jdk.graal.PrintCompilation=true}.
130+
*/
128131
@Override
129132
public long getStart() {
130133
return getAddress();
131134
}
132135

136+
/**
137+
* This method is used by the compiler debugging feature {@code jdk.graal.Dump=CodeInstall} to
138+
* dump a code at the point of code installation.
139+
*/
133140
@Override
134141
public byte[] getCode() {
135-
CodeInfo codeInfo = lookupCodeInfo();
136-
if (codeInfo.isNull()) {
137-
return null;
138-
}
139-
CodePointer codeStart = CodeInfoAccess.getCodeStart(codeInfo);
140-
int codeSize = (int) CodeInfoAccess.getCodeSize(codeInfo).rawValue();
141-
byte[] code = new byte[codeSize];
142-
CTypeConversion.asByteBuffer(codeStart, codeSize).get(code);
143-
return code;
142+
return getCode(Word.pointer(entryPoint));
144143
}
145144

146-
@Uninterruptible(reason = "Looks up code info.")
147-
private CodeInfo lookupCodeInfo() {
148-
UntetheredCodeInfo untetheredCodeInfo = CodeInfoTable.lookupCodeInfo(Word.pointer(entryPoint));
145+
@Uninterruptible(reason = "Accesses code info.")
146+
public static byte[] getCode(CodePointer entryPointAddress) {
147+
UntetheredCodeInfo untetheredCodeInfo = CodeInfoTable.lookupCodeInfo(entryPointAddress);
149148
if (untetheredCodeInfo.isNull()) {
150-
return WordFactory.nullPointer();
149+
return null;
151150
}
152151
Object tether = CodeInfoAccess.acquireTether(untetheredCodeInfo);
153152
try {
154-
return CodeInfoAccess.convert(untetheredCodeInfo, tether);
153+
CodeInfo codeInfo = CodeInfoAccess.convert(untetheredCodeInfo, tether);
154+
return copyCode0(codeInfo);
155155
} finally {
156156
CodeInfoAccess.releaseTether(untetheredCodeInfo, tether);
157157
}
158158
}
159159

160+
@Uninterruptible(reason = "Wrap the now safe call to code info.", calleeMustBe = false)
161+
private static byte[] copyCode0(CodeInfo codeInfo) {
162+
return copyCode(CodeInfoAccess.getCodeStart(codeInfo), (int) CodeInfoAccess.getCodeSize(codeInfo).rawValue());
163+
}
164+
165+
private static byte[] copyCode(CodePointer codeStart, int codeSize) {
166+
byte[] code = new byte[codeSize];
167+
CTypeConversion.asByteBuffer(codeStart, codeSize).get(code);
168+
return code;
169+
}
170+
160171
@Override
161172
public Object executeVarargs(Object... args) {
162173
throw shouldNotReachHere("No implementation in Substrate VM");

‎substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateOptimizedCallTargetInstalledCode.java

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import java.lang.ref.WeakReference;
2828

29+
import com.oracle.svm.graal.meta.SubstrateInstalledCodeImpl;
2930
import jdk.graal.compiler.word.Word;
3031

3132
import com.oracle.svm.core.Uninterruptible;
@@ -46,9 +47,6 @@
4647
import jdk.graal.compiler.truffle.TruffleCompilerImpl;
4748
import jdk.vm.ci.code.InstalledCode;
4849
import jdk.vm.ci.meta.ResolvedJavaMethod;
49-
import org.graalvm.nativeimage.c.function.CodePointer;
50-
import org.graalvm.nativeimage.c.type.CTypeConversion;
51-
import org.graalvm.word.WordFactory;
5250

5351
/**
5452
* Represents the compiled code of a {@link SubstrateOptimizedCallTarget}.
@@ -253,36 +251,22 @@ boolean isValidLastTier() {
253251

254252
private static final String NOT_CALLED_IN_SUBSTRATE_VM = "No implementation in Substrate VM";
255253

254+
/**
255+
* This method is used by the compiler debugging feature
256+
* {@code jdk.graal.PrintCompilation=true}.
257+
*/
256258
@Override
257259
public long getStart() {
258260
return getAddress();
259261
}
260262

263+
/**
264+
* This method is used by the compiler debugging feature {@code jdk.graal.Dump=CodeInstall} to
265+
* dump a code at the point of code installation.
266+
*/
261267
@Override
262268
public byte[] getCode() {
263-
CodeInfo codeInfo = lookupCodeInfo();
264-
if (codeInfo.isNull()) {
265-
return null;
266-
}
267-
CodePointer codeStart = CodeInfoAccess.getCodeStart(codeInfo);
268-
int codeSize = (int) CodeInfoAccess.getCodeSize(codeInfo).rawValue();
269-
byte[] code = new byte[codeSize];
270-
CTypeConversion.asByteBuffer(codeStart, codeSize).get(code);
271-
return code;
272-
}
273-
274-
@Uninterruptible(reason = "Looks up code info.")
275-
private CodeInfo lookupCodeInfo() {
276-
UntetheredCodeInfo untetheredCodeInfo = CodeInfoTable.lookupCodeInfo(Word.pointer(entryPoint));
277-
if (untetheredCodeInfo.isNull()) {
278-
return WordFactory.nullPointer();
279-
}
280-
Object tether = CodeInfoAccess.acquireTether(untetheredCodeInfo);
281-
try {
282-
return CodeInfoAccess.convert(untetheredCodeInfo, tether);
283-
} finally {
284-
CodeInfoAccess.releaseTether(untetheredCodeInfo, tether);
285-
}
269+
return SubstrateInstalledCodeImpl.getCode(Word.pointer(entryPoint));
286270
}
287271

288272
@Override

0 commit comments

Comments
 (0)