[llvm] a27b9ab - llvm-reduce: Preserve frame index values when cloning function

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2022-04-26T13:17:13-04:00
New Revision: a27b9ab391d1e0749e37b7db93b0fae18e23ecf6

URL: https://github.com/llvm/llvm-project/commit/a27b9ab391d1e0749e37b7db93b0fae18e23ecf6
DIFF: https://github.com/llvm/llvm-project/commit/a27b9ab391d1e0749e37b7db93b0fae18e23ecf6.diff

LOG: llvm-reduce: Preserve frame index values when cloning function

Previously the specific values used for fixed frame indexes was in
reverse order in the cloned function from the original, and a map was
used to adjust all frame indexes to the potentially new values. Insert
the fixed objects in reverse to avoid this. This simplifies other
code, since now we don't need to track down all frame indexes. This
will allow targets that store frame indexes in MachineFunctionInfo to
simply copy the values.

Note this isn't directly observable in the test since the resulting
MIR print/parse can shuffle the IDs around (in particular the final
serialization implicitly strips out dead objects).

Added: 
    

Modified: 
    llvm/tools/llvm-reduce/ReducerWorkItem.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/tools/llvm-reduce/ReducerWorkItem.cpp b/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
index 0f953f5787972..1e186221bbb51 100644
--- a/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
+++ b/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
@@ -21,14 +21,9 @@
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 
-// FIXME: Preserve frame index numbers. The numbering is off for fixed objects
-// since they are inserted at the beginning. This would avoid the need for the
-// Src2DstFrameIndex map and in the future target MFI code wouldn't need to
-// worry about it either.
 static void cloneFrameInfo(
     MachineFrameInfo &DstMFI, const MachineFrameInfo &SrcMFI,
-    const DenseMap<MachineBasicBlock *, MachineBasicBlock *> Src2DstMBB,
-    DenseMap<int, int> &Src2DstFrameIndex) {
+    const DenseMap<MachineBasicBlock *, MachineBasicBlock *> &Src2DstMBB) {
   DstMFI.setFrameAddressIsTaken(SrcMFI.isFrameAddressTaken());
   DstMFI.setReturnAddressIsTaken(SrcMFI.isReturnAddressTaken());
   DstMFI.setHasStackMap(SrcMFI.hasStackMap());
@@ -61,15 +56,22 @@ static void cloneFrameInfo(
   if (MachineBasicBlock *RestorePt = SrcMFI.getRestorePoint())
     DstMFI.setRestorePoint(Src2DstMBB.find(RestorePt)->second);
 
-  for (int i = SrcMFI.getObjectIndexBegin(), e = SrcMFI.getObjectIndexEnd();
+
+  auto CopyObjectProperties = [](MachineFrameInfo &DstMFI,
+                                 const MachineFrameInfo &SrcMFI, int FI) {
+    if (SrcMFI.isStatepointSpillSlotObjectIndex(FI))
+      DstMFI.markAsStatepointSpillSlotObjectIndex(FI);
+    DstMFI.setObjectSSPLayout(FI, SrcMFI.getObjectSSPLayout(FI));
+    DstMFI.setObjectZExt(FI, SrcMFI.isObjectZExt(FI));
+    DstMFI.setObjectSExt(FI, SrcMFI.isObjectSExt(FI));
+  };
+
+  for (int i = 0, e = SrcMFI.getNumObjects() - SrcMFI.getNumFixedObjects();
        i != e; ++i) {
     int NewFI;
 
-    if (SrcMFI.isFixedObjectIndex(i)) {
-      NewFI = DstMFI.CreateFixedObject(
-          SrcMFI.getObjectSize(i), SrcMFI.getObjectOffset(i),
-          SrcMFI.isImmutableObjectIndex(i), SrcMFI.isAliasedObjectIndex(i));
-    } else if (SrcMFI.isVariableSizedObjectIndex(i)) {
+    assert(!SrcMFI.isFixedObjectIndex(i));
+    if (SrcMFI.isVariableSizedObjectIndex(i)) {
       NewFI = DstMFI.CreateVariableSizedObject(SrcMFI.getObjectAlign(i),
                                                SrcMFI.getObjectAllocation(i));
     } else {
@@ -80,13 +82,23 @@ static void cloneFrameInfo(
       DstMFI.setObjectOffset(NewFI, SrcMFI.getObjectOffset(i));
     }
 
-    if (SrcMFI.isStatepointSpillSlotObjectIndex(i))
-      DstMFI.markAsStatepointSpillSlotObjectIndex(NewFI);
-    DstMFI.setObjectSSPLayout(NewFI, SrcMFI.getObjectSSPLayout(i));
-    DstMFI.setObjectZExt(NewFI, SrcMFI.isObjectZExt(i));
-    DstMFI.setObjectSExt(NewFI, SrcMFI.isObjectSExt(i));
+    CopyObjectProperties(DstMFI, SrcMFI, i);
+
+    (void)NewFI;
+    assert(i == NewFI && "expected to keep stable frame index numbering");
+  }
 
-    Src2DstFrameIndex[i] = NewFI;
+  // Copy the fixed frame objects backwards to preserve frame index numbers,
+  // since CreateFixedObject uses front insertion.
+  for (int i = -1; i >= (int)-SrcMFI.getNumFixedObjects(); --i) {
+    assert(SrcMFI.isFixedObjectIndex(i));
+    int NewFI = DstMFI.CreateFixedObject(
+      SrcMFI.getObjectSize(i), SrcMFI.getObjectOffset(i),
+      SrcMFI.isImmutableObjectIndex(i), SrcMFI.isAliasedObjectIndex(i));
+    CopyObjectProperties(DstMFI, SrcMFI, i);
+
+    (void)NewFI;
+    assert(i == NewFI && "expected to keep stable frame index numbering");
   }
 
   for (unsigned I = 0, E = SrcMFI.getLocalFrameObjectCount(); I < E; ++I) {
@@ -94,24 +106,15 @@ static void cloneFrameInfo(
     DstMFI.mapLocalFrameObject(LocalObject.first, LocalObject.second);
   }
 
-  // Remap the frame indexes in the CalleeSavedInfo
-  std::vector<CalleeSavedInfo> CalleeSavedInfos = SrcMFI.getCalleeSavedInfo();
-  for (CalleeSavedInfo &CSInfo : CalleeSavedInfos) {
-    if (!CSInfo.isSpilledToReg())
-      CSInfo.setFrameIdx(Src2DstFrameIndex[CSInfo.getFrameIdx()]);
-  }
-
-  DstMFI.setCalleeSavedInfo(std::move(CalleeSavedInfos));
+  DstMFI.setCalleeSavedInfo(SrcMFI.getCalleeSavedInfo());
 
   if (SrcMFI.hasStackProtectorIndex()) {
-    DstMFI.setStackProtectorIndex(
-        Src2DstFrameIndex[SrcMFI.getStackProtectorIndex()]);
+    DstMFI.setStackProtectorIndex(SrcMFI.getStackProtectorIndex());
   }
 
   // FIXME: Needs test, missing MIR serialization.
   if (SrcMFI.hasFunctionContextIndex()) {
-    DstMFI.setFunctionContextIndex(
-        Src2DstFrameIndex[SrcMFI.getFunctionContextIndex()]);
+    DstMFI.setFunctionContextIndex(SrcMFI.getFunctionContextIndex());
   }
 }
 
@@ -121,7 +124,6 @@ static std::unique_ptr<MachineFunction> cloneMF(MachineFunction *SrcMF) {
       SrcMF->getFunctionNumber(), SrcMF->getMMI());
   DenseMap<MachineBasicBlock *, MachineBasicBlock *> Src2DstMBB;
   DenseMap<Register, Register> Src2DstReg;
-  DenseMap<int, int> Src2DstFrameIndex;
 
   auto *SrcMRI = &SrcMF->getRegInfo();
   auto *DstMRI = &DstMF->getRegInfo();
@@ -167,12 +169,10 @@ static std::unique_ptr<MachineFunction> cloneMF(MachineFunction *SrcMF) {
   MachineFrameInfo &DstMFI = DstMF->getFrameInfo();
 
   // Copy stack objects and other info
-  cloneFrameInfo(DstMFI, SrcMFI, Src2DstMBB, Src2DstFrameIndex);
+  cloneFrameInfo(DstMFI, SrcMFI, Src2DstMBB);
 
   // Remap the debug info frame index references.
   DstMF->VariableDbgInfos = SrcMF->VariableDbgInfos;
-  for (MachineFunction::VariableDbgInfo &DbgInfo : DstMF->VariableDbgInfos)
-    DbgInfo.Slot = Src2DstFrameIndex[DbgInfo.Slot];
 
   // FIXME: Need to clone MachineFunctionInfo, which may also depend on frame
   // index and block mapping.
@@ -265,9 +265,6 @@ static std::unique_ptr<MachineFunction> cloneMF(MachineFunction *SrcMF) {
         // Update MBB.
         if (DstMO.isMBB()) {
           DstMO.setMBB(Src2DstMBB[DstMO.getMBB()]);
-        } else if (DstMO.isFI()) {
-          // Update frame indexes
-          DstMO.setIndex(Src2DstFrameIndex[DstMO.getIndex()]);
         }
 
         DstMI->addOperand(DstMO);