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).

diff --git a/llvm/tools/llvm-reduce/ReducerWorkItem.cpp b/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
index 0f953f5..1e18622 100644 (file)
--- 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);