[GR-70581] Duplicate CodeMark upon assembly replay.

PullRequest: graal/22313

Author: mur47x111

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/ThreadedInterpreterTest.java

@@ -409,7 +409,7 @@ private int countPotentialJumpTableJumps(byte[] code) {
                 } else {
                     if (opcodeJmpReg) {
                         // preceding byte is 0xFF
-                        if ((b & 0xC2) == 0xC2) {
+                        if ((b & 0xF8) == 0xE0) {
                             count++;
                         }
                     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/Assembler.java

@@ -370,7 +370,7 @@ public void stopRecordingCodeSnippet(CompilationResultBuilder crb) {
             if (recordedCodeSnippets == null) {
                 recordedCodeSnippets = EconomicMap.create();
             }
-            recordedCodeSnippets.put(currentCodeSnippet.codeStart, currentCodeSnippet);
+            recordedCodeSnippets.put(currentCodeSnippet.getCodeStart(), currentCodeSnippet);
             currentCodeSnippet = null;
         }
     }
@@ -393,11 +393,11 @@ public boolean replayCodeSnippetAt(CompilationResultBuilder crb, int pos) {
         return false;
     }
 
-    protected boolean isRecordingCodeSnippet() {
+    public boolean isRecordingCodeSnippet() {
         return currentCodeSnippet != null && currentCodeSnippet.isRecording();
     }
 
-    protected void abortRecordingCodeSnippet() {
+    public void abortRecordingCodeSnippet() {
         currentCodeSnippet = null;
     }
 

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/CodeSnippetRecord.java

@@ -24,31 +24,42 @@
  */
 package jdk.graal.compiler.asm;
 
-import jdk.vm.ci.code.site.ImplicitExceptionDispatch;
-import jdk.vm.ci.code.site.Site;
 import org.graalvm.collections.EconomicMap;
 
 import jdk.graal.compiler.code.CompilationResult.CompilationResultWatermark;
+import jdk.graal.compiler.lir.LIR;
 import jdk.graal.compiler.lir.asm.CompilationResultBuilder;
+import jdk.vm.ci.code.site.ImplicitExceptionDispatch;
+import jdk.vm.ci.code.site.Site;
 
 /**
  * Records a snippet of emitted assembly. The recorded snippet can be replayed, i.e., inserted into
  * the code buffer at different positions, multiple times. On each replay, the snippet is patched
  * based on its insertion position.
+ *
+ * If a slow path is added via {@link LIR#addSlowPath} during recording, it will not be duplicated
+ * during replay, as the replay only copies the exact bytes between {@link #codeStart} and
+ * {@link #codeEnd}. As a result, all replayed snippets will jump to the same slow path. The slow
+ * path may jump back to the originally recorded snippet (e.g., in the G1 write barrier, or ZGC load
+ * and write barriers). This behavior is semantically correct because each replayed snippet is a
+ * complete replica. However, it may reduce the effectiveness of branch prediction optimizations
+ * introduced by the assembly replay mechanism. To preserve potential branch predictor benefits,
+ * {@link LIR#addSlowPath} should be reserved exclusively for genuine slow paths that are
+ * infrequently taken and do not usually jump back to the original snippet.
  */
-final class CodeSnippetRecord {
-    int codeStart;
-    int codeEnd;
-    EconomicMap<Integer, Label> codePatchLabels;
+public final class CodeSnippetRecord {
+    private int codeStart;
+    private int codeEnd;
+    private EconomicMap<Integer, Label> codePatchLabels;
 
     /**
      * Watermarks represent the sizes of recorded or pending {@link Site}s essential for execution,
      * e.g., {@link ImplicitExceptionDispatch}. By capturing watermarks at the start and end of code
      * execution, we can identify the {@link Site}s appended during recording and replicate them
      * during replay.
      */
-    CompilationResultWatermark watermarkAtCodeStart;
-    CompilationResultWatermark watermarkAtCodeEnd;
+    private CompilationResultWatermark watermarkAtCodeStart;
+    private CompilationResultWatermark watermarkAtCodeEnd;
 
     CodeSnippetRecord(int codeStart, CompilationResultWatermark watermark) {
         this.codeStart = codeStart;
@@ -58,6 +69,10 @@ final class CodeSnippetRecord {
         this.codePatchLabels = EconomicMap.create();
     }
 
+    int getCodeStart() {
+        return codeStart;
+    }
+
     boolean isRecording() {
         return codeStart != -1 && codeEnd == -1;
     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/code/CompilationResult.java

@@ -726,6 +726,13 @@ public interface MarkId {
         default Object getId() {
             return this;
         }
+
+        /**
+         * @return {@code true} if only one instance of this {@code MarkID} is allowed
+         */
+        default boolean isUnique() {
+            return true;
+        }
     }
 
     /**
@@ -883,6 +890,7 @@ public static String getSignature(byte[] data) {
     public record CompilationResultWatermark(int infopointsSize,
                     int dataPatchesSize,
                     int exceptionHandlersSize,
+                    int marksSize,
                     int pendingExceptionInfoListSize,
                     int pendingImplicitExceptionListSize) {
     }
@@ -893,7 +901,7 @@ public record CompilationResultWatermark(int infopointsSize,
      *         {@link ExceptionHandler} or {@link ImplicitExceptionDispatch}.
      */
     public CompilationResultWatermark getSitesWatermark(int pendingExceptionInfoListSize, int pendingImplicitExceptionListSize) {
-        return new CompilationResultWatermark(infopoints.size(), dataPatches.size(), exceptionHandlers.size(), pendingExceptionInfoListSize,
+        return new CompilationResultWatermark(infopoints.size(), dataPatches.size(), exceptionHandlers.size(), marks.size(), pendingExceptionInfoListSize,
                         pendingImplicitExceptionListSize);
     }
 
@@ -925,6 +933,14 @@ private boolean verifySites(int codeStart, CompilationResultWatermark watermarkA
                 assert codeOffset < codeStart || codeOffset >= codeEnd : Assertions.errorMessage(codeOffset, codeStart, codeEnd, exceptionHandlers);
             }
         }
+        for (int i = 0; i < marks.size(); i++) {
+            int codeOffset = marks.get(i).pcOffset;
+            if (watermarkAtCodeStart.marksSize() <= i && i < watermarkAtCodeEnd.marksSize()) {
+                assert codeStart <= codeOffset && codeOffset < codeEnd : Assertions.errorMessage(codeOffset, codeStart, codeEnd, marks);
+            } else {
+                assert codeOffset < codeStart || codeOffset >= codeEnd : Assertions.errorMessage(codeOffset, codeStart, codeEnd, marks);
+            }
+        }
         return true;
     }
 
@@ -952,5 +968,9 @@ public void duplicateSites(int codeStart, CompilationResultWatermark watermarkAt
             ExceptionHandler handler = exceptionHandlers.get(i);
             exceptionHandlers.add(new ExceptionHandler(handler.pcOffset + offset, handler.handlerPos));
         }
+        for (int i = watermarkAtCodeStart.marksSize; i < watermarkAtCodeEnd.marksSize; i++) {
+            CodeMark mark = marks.get(i);
+            marks.add(new CodeMark(mark.pcOffset + offset, mark.id));
+        }
     }
 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/HotSpotMarkId.java

@@ -33,14 +33,14 @@
  * code.
  */
 public enum HotSpotMarkId implements CompilationResult.MarkId {
-    VERIFIED_ENTRY,
-    UNVERIFIED_ENTRY,
-    OSR_ENTRY,
-    EXCEPTION_HANDLER_ENTRY,
-    DEOPT_HANDLER_ENTRY,
-    DEOPT_MH_HANDLER_ENTRY,
-    FRAME_COMPLETE,
-    ENTRY_BARRIER_PATCH,
+    VERIFIED_ENTRY(true),
+    UNVERIFIED_ENTRY(true),
+    OSR_ENTRY(true),
+    EXCEPTION_HANDLER_ENTRY(true),
+    DEOPT_HANDLER_ENTRY(true),
+    DEOPT_MH_HANDLER_ENTRY(true),
+    FRAME_COMPLETE(true),
+    ENTRY_BARRIER_PATCH(true),
     INVOKEINTERFACE,
     INVOKEVIRTUAL,
     INVOKESTATIC,
@@ -73,14 +73,24 @@ public enum HotSpotMarkId implements CompilationResult.MarkId {
 
     private Integer value;
     private final String arch;
+    private final boolean isUnique;
 
     HotSpotMarkId() {
-        this(null);
+        this(null, false);
+    }
+
+    HotSpotMarkId(boolean isUnique) {
+        this(null, isUnique);
     }
 
     HotSpotMarkId(String arch) {
+        this(arch, false);
+    }
+
+    HotSpotMarkId(String arch, boolean isUnique) {
         this.value = null;
         this.arch = arch;
+        this.isUnique = isUnique;
     }
 
     void setValue(Integer value) {
@@ -110,6 +120,11 @@ public boolean isAvailable() {
         return value != null;
     }
 
+    @Override
+    public boolean isUnique() {
+        return isUnique;
+    }
+
     @Override
     public String toString() {
         return "HotSpotCodeMark{" + name() +

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/LIR.java

@@ -27,6 +27,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import jdk.graal.compiler.asm.CodeSnippetRecord;
 import jdk.graal.compiler.asm.Label;
 import jdk.graal.compiler.core.common.cfg.AbstractControlFlowGraph;
 import jdk.graal.compiler.core.common.cfg.BasicBlock;
@@ -206,6 +207,10 @@ public ArrayList<LIRInstruction.LIRInstructionSlowPath> getSlowPaths() {
 
     /**
      * Add a chunk of assembly that will be emitted out of line after all LIR has been emitted.
+     *
+     * If called during assembler code snippet recording, the slow path will not be duplicated.
+     * Instead, it will be shared among replayed code snippets. See {@link CodeSnippetRecord} for
+     * more details.
      */
     public void addSlowPath(LIRInstruction op, Runnable slowPath) {
         if (slowPaths == null) {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/asm/CompilationResultBuilder.java

@@ -215,6 +215,10 @@ public void setMinDataSectionItemAlignment(int alignment) {
      * @return the recorded entry for the mark
      */
     public CompilationResult.CodeMark recordMark(int codePos, CompilationResult.MarkId markId) {
+        if (markId.isUnique() && asm.isRecordingCodeSnippet()) {
+            // We cannot duplicate unique MarkId
+            asm.abortRecordingCodeSnippet();
+        }
         return compilationResult.recordMark(codePos, markId);
     }