[GR-47800] Mask registers shouldn't appear in callee save oop maps

PullRequest: graal/15190
oracle · Aug 11, 2023 · 7416cea · 7416cea
2 parents f4f5f2e + 9a486f7
commit 7416cea
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 71 deletions.
diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotBackend.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotBackend.java
@@ -457,7 +457,7 @@ protected void updateStub(Stub stub, HotSpotLIRGenerationResult gen, FrameMap fr
             SaveRegistersOp save = cursor.getValue();
             save.remove(destroyedRegisters);
             if (cursor.getKey() != LIRFrameState.NO_CALLEE_SAVE_INFO) {
-                cursor.getKey().debugInfo().setCalleeSaveInfo(save.getMap(frameMap));
+                cursor.getKey().debugInfo().setCalleeSaveInfo(save.getRegisterSaveLayout(frameMap));
             }
         }
     }

diff --git a/...jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotFrameMap.java b/...jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotFrameMap.java
@@ -24,6 +24,7 @@
  */
 package org.graalvm.compiler.hotspot.amd64;
 
+import static jdk.vm.ci.amd64.AMD64.MASK;
 import static jdk.vm.ci.code.ValueUtil.asStackSlot;
 
 import org.graalvm.compiler.core.common.LIRKind;
@@ -32,6 +33,7 @@
 
 import jdk.vm.ci.amd64.AMD64Kind;
 import jdk.vm.ci.code.CodeCacheProvider;
+import jdk.vm.ci.code.Register;
 import jdk.vm.ci.code.RegisterConfig;
 import jdk.vm.ci.code.StackSlot;
 
@@ -123,4 +125,23 @@ public StackSlot getDeoptimizationRescueSlot() {
         assert deoptimizationRescueSlot != null;
         return deoptimizationRescueSlot;
     }
+
+    @Override
+    protected Register[] filterSavedRegisters(Register[] savedRegisters) {
+        Register[] filtered = null;
+        for (int i = 0; i < savedRegisters.length; i++) {
+            Register reg = savedRegisters[i];
+            if (reg == null) {
+                continue;
+            }
+            if (reg.getRegisterCategory().equals(MASK)) {
+                // These can't appear in HotSpot debug info
+                if (filtered == null) {
+                    filtered = savedRegisters.clone();
+                }
+                filtered[i] = null;
+            }
+        }
+        return filtered != null ? filtered : savedRegisters;
+    }
 }
diff --git a/...al.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotHostForeignCallsProvider.java b/...al.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotHostForeignCallsProvider.java
@@ -80,7 +80,6 @@
 import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Transition.LEAF;
 import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Transition.LEAF_NO_VZERO;
 import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Transition.SAFEPOINT;
-import static org.graalvm.compiler.hotspot.replacements.AssertionSnippets.ASSERTION_VM_MESSAGE_C;
 import static org.graalvm.compiler.hotspot.replacements.HotSpotAllocationSnippets.DYNAMIC_NEW_INSTANCE;
 import static org.graalvm.compiler.hotspot.replacements.HotSpotAllocationSnippets.DYNAMIC_NEW_INSTANCE_OR_NULL;
 import static org.graalvm.compiler.hotspot.replacements.HotSpotG1WriteBarrierSnippets.G1WBPOSTCALL;
@@ -463,12 +462,7 @@ public void initialize(HotSpotProviders providers, OptionValues options) {
 
         CreateExceptionStub.registerForeignCalls(c, this);
 
-        /*
-         * This message call is registered twice, where the second one must only be used for calls
-         * that do not return, i.e., that exit the VM.
-         */
         registerForeignCall(VM_MESSAGE_C, c.vmMessageAddress, NativeCall);
-        registerForeignCall(ASSERTION_VM_MESSAGE_C, c.vmMessageAddress, NativeCall);
 
         linkForeignCall(options, providers, NEW_INSTANCE, c.newInstanceAddress, PREPEND_THREAD);
         linkForeignCall(options, providers, NEW_ARRAY, c.newArrayAddress, PREPEND_THREAD);

diff --git a/...internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/AssertionSnippets.java b/...internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/AssertionSnippets.java
@@ -26,9 +26,7 @@
 
 import static org.graalvm.compiler.api.directives.GraalDirectives.SLOWPATH_PROBABILITY;
 import static org.graalvm.compiler.api.directives.GraalDirectives.injectBranchProbability;
-import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Reexecutability.REEXECUTABLE;
-import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Transition.LEAF;
-import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallsProviderImpl.NO_LOCATIONS;
+import static org.graalvm.compiler.hotspot.stubs.StubUtil.VM_MESSAGE_C;
 import static org.graalvm.compiler.replacements.SnippetTemplate.DEFAULT_REPLACER;
 
 import org.graalvm.compiler.api.replacements.Snippet;
@@ -37,7 +35,6 @@
 import org.graalvm.compiler.core.common.type.StampFactory;
 import org.graalvm.compiler.graph.Node.ConstantNodeParameter;
 import org.graalvm.compiler.graph.Node.NodeIntrinsic;
-import org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor;
 import org.graalvm.compiler.hotspot.meta.HotSpotProviders;
 import org.graalvm.compiler.hotspot.nodes.StubForeignCallNode;
 import org.graalvm.compiler.hotspot.nodes.StubStartNode;
@@ -56,24 +53,17 @@
 
 public class AssertionSnippets implements Snippets {
 
-    /**
-     * This call can only be used with true for the "vmError" parameter, so that it can be
-     * configured to be a leaf method.
-     */
-    public static final HotSpotForeignCallDescriptor ASSERTION_VM_MESSAGE_C = new HotSpotForeignCallDescriptor(LEAF, REEXECUTABLE, NO_LOCATIONS, "assertionVmMessageC", void.class, boolean.class,
-                    Word.class, long.class, long.class, long.class);
-
     @Snippet
     public static void assertion(boolean condition, @ConstantParameter Word message, long l1, long l2) {
         if (injectBranchProbability(SLOWPATH_PROBABILITY, !condition)) {
-            vmMessageC(ASSERTION_VM_MESSAGE_C, true, message, l1, l2, 0L);
+            vmMessageC(VM_MESSAGE_C, true, message, l1, l2, 0L);
         }
     }
 
     @Snippet
     public static void stubAssertion(boolean condition, @ConstantParameter Word message, long l1, long l2) {
         if (injectBranchProbability(SLOWPATH_PROBABILITY, !condition)) {
-            vmMessageStub(ASSERTION_VM_MESSAGE_C, true, message, l1, l2, 0L);
+            vmMessageStub(VM_MESSAGE_C, true, message, l1, l2, 0L);
         }
     }
 

diff --git a/....vm.compiler/src/org/graalvm/compiler/hotspot/replacements/HotSpotAllocationSnippets.java b/....vm.compiler/src/org/graalvm/compiler/hotspot/replacements/HotSpotAllocationSnippets.java
@@ -62,6 +62,7 @@
 import static org.graalvm.compiler.hotspot.replacements.HotSpotReplacementsUtil.registerAsWord;
 import static org.graalvm.compiler.hotspot.replacements.HotspotSnippetsOptions.ProfileAllocations;
 import static org.graalvm.compiler.hotspot.replacements.HotspotSnippetsOptions.ProfileAllocationsContext;
+import static org.graalvm.compiler.hotspot.stubs.StubUtil.VM_MESSAGE_C;
 import static org.graalvm.compiler.nodes.PiArrayNode.piArrayCastToSnippetReplaceeStamp;
 import static org.graalvm.compiler.nodes.PiNode.piCastToSnippetReplaceeStamp;
 import static org.graalvm.compiler.nodes.extended.BranchProbabilityNode.DEOPT_PROBABILITY;
@@ -290,7 +291,7 @@ private void verifyHeap() {
         if (!topValue.equal(WordFactory.zero())) {
             Word topValueContents = topValue.readWord(0, MARK_WORD_LOCATION);
             if (topValueContents.equal(WordFactory.zero())) {
-                AssertionSnippets.vmMessageC(AssertionSnippets.ASSERTION_VM_MESSAGE_C, true, cstring("overzeroing of TLAB detected"), 0L, 0L, 0L);
+                AssertionSnippets.vmMessageC(VM_MESSAGE_C, true, cstring("overzeroing of TLAB detected"), 0L, 0L, 0L);
             }
         }
     }

diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/stubs/StubUtil.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/stubs/StubUtil.java
@@ -25,7 +25,7 @@
 package org.graalvm.compiler.hotspot.stubs;
 
 import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Reexecutability.REEXECUTABLE;
-import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Transition.SAFEPOINT;
+import static org.graalvm.compiler.hotspot.meta.HotSpotForeignCallDescriptor.Transition.LEAF;
 import static org.graalvm.compiler.replacements.nodes.CStringConstant.cstring;
 
 import java.lang.reflect.Method;
@@ -54,7 +54,7 @@
  */
 public class StubUtil {
 
-    public static final HotSpotForeignCallDescriptor VM_MESSAGE_C = newDescriptor(SAFEPOINT, REEXECUTABLE, null, StubUtil.class, "vmMessageC", void.class, boolean.class, Word.class, long.class,
+    public static final HotSpotForeignCallDescriptor VM_MESSAGE_C = newDescriptor(LEAF, REEXECUTABLE, null, StubUtil.class, "vmMessageC", void.class, boolean.class, Word.class, long.class,
                     long.class, long.class);
 
     public static HotSpotForeignCallDescriptor newDescriptor(HotSpotForeignCallDescriptor.Transition safepoint, HotSpotForeignCallDescriptor.Reexecutability reexecutable,

diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/StandardOp.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/StandardOp.java
@@ -24,8 +24,6 @@
  */
 package org.graalvm.compiler.lir;
 
-import static jdk.vm.ci.code.ValueUtil.asStackSlot;
-import static jdk.vm.ci.code.ValueUtil.isStackSlot;
 import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.CONST;
 import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.OUTGOING;
 import static org.graalvm.compiler.lir.LIRInstruction.OperandFlag.REG;
@@ -44,7 +42,6 @@
 
 import jdk.vm.ci.code.Register;
 import jdk.vm.ci.code.RegisterSaveLayout;
-import jdk.vm.ci.code.StackSlot;
 import jdk.vm.ci.meta.AllocatableValue;
 import jdk.vm.ci.meta.Constant;
 import jdk.vm.ci.meta.Value;
@@ -300,7 +297,7 @@ static boolean isLoadConstantOp(LIRInstruction op) {
     /**
      * An operation that saves registers to the stack. The set of saved registers can be
      * {@linkplain #remove(EconomicSet) pruned} and a mapping from registers to the frame slots in
-     * which they are saved can be {@linkplain #getMap(FrameMap) retrieved}.
+     * which they are saved can be {@linkplain #getRegisterSaveLayout(FrameMap) retrieved}.
      */
     public abstract static class SaveRegistersOp extends LIRInstruction {
         public static final LIRInstructionClass<SaveRegistersOp> TYPE = LIRInstructionClass.create(SaveRegistersOp.class);
@@ -338,37 +335,8 @@ public int remove(EconomicSet<Register> doNotSave) {
             return prune(doNotSave, savedRegisters);
         }
 
-        /**
-         * Gets a map from the saved registers saved by this operation to the frame slots in which
-         * they are saved.
-         *
-         * @param frameMap used to {@linkplain FrameMap#offsetForStackSlot(StackSlot) convert} a
-         *            virtual slot to a frame slot index
-         */
-
-        public RegisterSaveLayout getMap(FrameMap frameMap) {
-            int total = 0;
-            for (int i = 0; i < savedRegisters.length; i++) {
-                if (savedRegisters[i] != null) {
-                    total++;
-                }
-            }
-            Register[] keys = new Register[total];
-            int[] values = new int[total];
-            if (total != 0) {
-                int mapIndex = 0;
-                for (int i = 0; i < savedRegisters.length; i++) {
-                    if (savedRegisters[i] != null) {
-                        keys[mapIndex] = savedRegisters[i];
-                        assert isStackSlot(slots[i]) : "not a StackSlot: " + slots[i];
-                        StackSlot slot = asStackSlot(slots[i]);
-                        values[mapIndex] = indexForStackSlot(frameMap, slot);
-                        mapIndex++;
-                    }
-                }
-                assert mapIndex == total;
-            }
-            return new RegisterSaveLayout(keys, values);
+        public RegisterSaveLayout getRegisterSaveLayout(FrameMap frameMap) {
+            return frameMap.getRegisterSaveLayout(savedRegisters, slots);
         }
 
         public Register[] getSavedRegisters() {
@@ -402,19 +370,6 @@ static int prune(EconomicSet<Register> toRemove, Register[] registers) {
             }
             return pruned;
         }
-
-        /**
-         * Computes the index of a stack slot relative to slot 0. This is also the bit index of
-         * stack slots in the reference map.
-         *
-         * @param slot a stack slot
-         * @return the index of the stack slot
-         */
-        private static int indexForStackSlot(FrameMap frameMap, StackSlot slot) {
-            assert frameMap.offsetForStackSlot(slot) % frameMap.getTarget().wordSize == 0;
-            int value = frameMap.offsetForStackSlot(slot) / frameMap.getTarget().wordSize;
-            return value;
-        }
     }
 
     /**

diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/framemap/FrameMap.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/lir/framemap/FrameMap.java
@@ -24,6 +24,9 @@
  */
 package org.graalvm.compiler.lir.framemap;
 
+import static jdk.vm.ci.code.ValueUtil.asStackSlot;
+import static jdk.vm.ci.code.ValueUtil.isStackSlot;
+
 import org.graalvm.compiler.core.common.LIRKind;
 import org.graalvm.compiler.core.common.NumUtil;
 import org.graalvm.compiler.core.common.PermanentBailoutException;
@@ -33,9 +36,12 @@
 import jdk.vm.ci.code.CallingConvention;
 import jdk.vm.ci.code.CodeCacheProvider;
 import jdk.vm.ci.code.CodeUtil;
+import jdk.vm.ci.code.Register;
 import jdk.vm.ci.code.RegisterConfig;
+import jdk.vm.ci.code.RegisterSaveLayout;
 import jdk.vm.ci.code.StackSlot;
 import jdk.vm.ci.code.TargetDescription;
+import jdk.vm.ci.meta.AllocatableValue;
 import jdk.vm.ci.meta.ValueKind;
 
 /**
@@ -269,4 +275,50 @@ public StackSlot allocateStackMemory(int sizeInBytes, int alignmentInBytes) {
     public ReferenceMapBuilder newReferenceMapBuilder() {
         return referenceMapFactory.newReferenceMapBuilder(totalFrameSize());
     }
+
+    public RegisterSaveLayout getRegisterSaveLayout(Register[] savedRegisters, AllocatableValue[] slots) {
+        Register[] filteredSavedRegisters = filterSavedRegisters(savedRegisters);
+        int total = 0;
+        for (int i = 0; i < filteredSavedRegisters.length; i++) {
+            if (filteredSavedRegisters[i] != null) {
+                total++;
+            }
+        }
+        Register[] keys = new Register[total];
+        int[] values = new int[total];
+        if (total != 0) {
+            int mapIndex = 0;
+            for (int i = 0; i < filteredSavedRegisters.length; i++) {
+                if (filteredSavedRegisters[i] != null) {
+                    keys[mapIndex] = filteredSavedRegisters[i];
+                    assert isStackSlot(slots[i]) : "not a StackSlot: " + slots[i];
+                    StackSlot slot = asStackSlot(slots[i]);
+                    values[mapIndex] = indexForStackSlot(slot);
+                    mapIndex++;
+                }
+            }
+            assert mapIndex == total;
+        }
+        return new RegisterSaveLayout(keys, values);
+    }
+
+    /**
+     * {@link org.graalvm.compiler.lir.StandardOp.SaveRegistersOp} might save more registers than
+     * {@link RegisterSaveLayout} should know about so permit subclasses to filter the contents.
+     */
+    protected Register[] filterSavedRegisters(Register[] savedRegisters) {
+        return savedRegisters;
+    }
+
+    /**
+     * Computes the index of a stack slot relative to slot 0. This is also the bit index of stack
+     * slots in the reference map.
+     *
+     * @param slot a stack slot
+     * @return the index of the stack slot
+     */
+    private int indexForStackSlot(StackSlot slot) {
+        assert offsetForStackSlot(slot) % getTarget().wordSize == 0;
+        return offsetForStackSlot(slot) / getTarget().wordSize;
+    }
 }