[PATCH 2/2] Centralize info about spill areas in data structures

[thopre]

Summary

Context

This patch is part of a patch series to make code for spilling/restoring
callee-saved register more data driven and with less code duplication.
Its purpose is to centralize properties about spill areas away from
various location in code and into data structures.

Patch description

A number of properties of spill areas is currently written in code, for
instance whether spilling non contiguous registers can be done in one
instruction or not. This means adding a new spill area or changing
properties of a spill area (for instance if a new instruction is
introduced that can improve spilling) code needs to be updated rather
than a well identified data structure. Worse off, that info is
duplicated in many places.

This patch builds on the spill area enumeration introduced in previous
patch to associate a SpillAreaProperty structure to each spill area
enumeration via an array whose index match the enumerators values. This
is done using an X-macro to avoid hardcoding the order of enumerator
declaration into the array initialization and because designated array
and structure initializer are not available in C++.

The rest of the patch updates the code to make use of these structures,
thereby making the code a bit more spill area independent and closer to
iterate over spill areas using loops.

Testing: Ran llvm_regr which shows no regression.

[thopre]

Ping? Would be nice to reduce complexity in this area.

Best regards.

[thopre]

No longer working for Arm. This would need to be rebased first if anybody picks it up.

lib/Target/ARM/ARMBaseRegisterInfo.h

namespace ARMRI {
 
   enum {
     RegPairOdd  = 1,
     RegPairEven = 2
   };
 
 } // end namespace ARMRI
 
+struct SpillAreaProperty {
+  bool AllowDiscreteRegs;
+  unsigned PushMultiThumb;
+  unsigned PushMultiARM;
+  unsigned PushOneThumb;
+  unsigned PushOneARM;
+  unsigned PopMultiThumb;
+  unsigned PopMultiARM;
+  unsigned PopOneThumb;
+  unsigned PopOneARM;
+};
+
+#define DEF_PROPERTIES                                                        \
+  /* General purpose registers spilled/restored alongside LR/PC */            \
+  DEF_PROPERTY(                                                               \
+      /*enumerator*/ GPRCS1Area, /*AllowDiscreteRegs=*/true,                  \
+      /*PushMultiThumb=*/ARM::t2STMDB_UPD, /*PushMultiARM=*/ARM::STMDB_UPD,   \
+      /*PushOneThumb*/ ARM::t2STR_PRE, /*PushOneARM=*/ARM::STR_PRE_IMM,       \
+      /*PopMultiThumb=*/ARM::t2LDMIA_UPD, /*PopMultiARM=*/ARM::LDMIA_UPD,     \
+      /*PopOneThumb=*/ARM::t2LDR_POST, /*PopOneARM=*/ARM::LDR_POST_IMM),      \
+                                                                              \
+  /* General purpose registers spilled/restored separately from LR/PC */      \
+  DEF_PROPERTY(                                                               \
+      /*enumerator*/ GPRCS2Area, /*AllowDiscreteRegs=*/true,                  \
+      /*PushMultiThumb=*/ARM::t2STMDB_UPD, /*PushMultiARM=*/ARM::STMDB_UPD,   \
+      /*PushOneThumb*/ ARM::t2STR_PRE, /*PushOneARM=*/ARM::STR_PRE_IMM,       \
+      /*PopMultiThumb=*/ARM::t2LDMIA_UPD, /*PopMultiARM=*/ARM::LDMIA_UPD,     \
+      /*PopOneThumb=*/ARM::t2LDR_POST, /*PopOneARM=*/ARM::LDR_POST_IMM),      \
+                                                                              \
+  /* VFP registers without extra SP alignment requirement */                  \
+  DEF_PROPERTY(                                                               \
+      /*enumerator*/ DPRCS1Area, /*AllowDiscreteRegs=*/false,                 \
+      /*PushMultiThumb=*/ARM::VSTMDDB_UPD, /*PushMultiARM=*/ARM::VSTMDDB_UPD, \
+      /*PushOneThumb*/ 0, /*PushOneARM=*/0,                                   \
+      /*PopMultiThumb=*/ARM::VLDMDIA_UPD, /*PopMultiARM=*/ARM::VLDMDIA_UPD,   \
+      /*PopOneThumb=*/0, /*PopOneARM=*/0),                                    \
+                                                                              \
+  /* VFP registers requiring 16-byte SP alignment (see -align-neon-spills) */ \
+  DEF_PROPERTY(                                                               \
+      /*enumerator*/ DPRCS2Area, /*AllowDiscreteRegs=*/false,                 \
+      /*PushMultiThumb=*/0, /*PushMultiARM=*/0,                               \
+      /*PushOneThumb*/ 0, /*PushOneARM=*/0,                                   \
+      /*PopMultiThumb=*/0, /*PopMultiARM=*/0,                                 \
+      /*PopOneThumb=*/0, /*PopOneARM=*/0)
+
 /// Sets of register to be pushed/poped together when spilling/restoring
 /// callee-saved registers.
+#define DEF_PROPERTY(enumerator, AllowDiscreteRegs, PushMultiThumb,            \
+                     PushMultiARM, PushOneThumb, PushOneARM, PopMultiThumb,    \
+                     PopMultiARM, PopOneThumb, PopOneARM)                      \
+  enumerator
 enum SpillArea {
-  GPRCS1Area,    // GP registers spilled/restored alongside LR/PC
-  GPRCS2Area,    // GP registers spilled/restored separately from LR/PC
-  DPRCS1Area,    // VFP registers without extra SP alignment requirement
-  DPRCS2Area,    // VFP registers requiring 16-byte SP alignment (see
-                 // -align-neon-spills)
+  DEF_PROPERTIES,
   UnknownCSArea, // Unexpected callee-saved registers
   CSMaxArea = UnknownCSArea
 };
+#undef DEF_PROPERTY
+
+#define DEF_PROPERTY(enumerator, AllowDiscreteRegs, PushMultiThumb,            \
+                     PushMultiARM, PushOneThumb, PushOneARM, PopMultiThumb,    \
+                     PopMultiARM, PopOneThumb, PopOneARM)                      \
+  {AllowDiscreteRegs, PushMultiThumb, PushMultiARM, PushOneThumb, PushOneARM,  \
+   PopMultiThumb,     PopMultiARM,    PopOneThumb,  PopOneARM}
+const std::array<SpillAreaProperty, CSMaxArea> SpillAreaProperties = {
+    {DEF_PROPERTIES}};
 
 /// getRegSpillArea - Returns which sets of registers Reg should be
 /// pushed/poped with, as a SpillArea enumerator. SplitGPRSpillArea
 /// controls whether high registers can be pushed together with LR and
 /// NumAlignedDPRCS2Regs the number of registers, starting from D8, in the
 /// DPRCS2Area.
 inline enum SpillArea getRegSpillArea(unsigned Reg, bool SplitGPRSpillArea,
                                       unsigned NumAlignedDPRCS2Regs) {

lib/Target/ARM/ARMFrameLowering.h

public:
 
   /// Returns true if the target will correctly handle shrink wrapping.
   bool enableShrinkWrapping(const MachineFunction &MF) const override {
     return true;
   }
 
 private:
   void emitPushInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
-                    const std::vector<CalleeSavedInfo> &CSI, unsigned StmOpc,
-                    unsigned StrOpc, bool NoGap, enum SpillArea SpillArea,
-                    unsigned MIFlags = 0) const;
+                    const std::vector<CalleeSavedInfo> &CSI,
+                    enum SpillArea SpillArea, unsigned MIFlags = 0) const;
   void emitPopInst(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
-                   std::vector<CalleeSavedInfo> &CSI, unsigned LdmOpc,
-                   unsigned LdrOpc, bool isVarArg, bool NoGap,
+                   std::vector<CalleeSavedInfo> &CSI, bool isVarArg,
                    enum SpillArea SpillArea) const;
 
   MachineBasicBlock::iterator
   eliminateCallFramePseudoInstr(MachineFunction &MF,
                                 MachineBasicBlock &MBB,
                                 MachineBasicBlock::iterator MI) const override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_ARM_ARMFRAMELOWERING_H

lib/Target/ARM/ARMFrameLowering.cpp

ARMFrameLowering::ResolveFrameIndexReference(const MachineFunction &MF,
   if (RegInfo->hasBasePointer(MF))
     FrameReg = RegInfo->getBaseRegister();
   return Offset;
 }
 
 void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB,
                                     MachineBasicBlock::iterator MI,
                                     const std::vector<CalleeSavedInfo> &CSI,
-                                    unsigned StmOpc, unsigned StrOpc,
-                                    bool NoGap, enum SpillArea SpillArea,
+                                    enum SpillArea SpillArea,
                                     unsigned MIFlags) const {
   MachineFunction &MF = *MBB.getParent();
   ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
   const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
   const TargetRegisterInfo &TRI = *STI.getRegisterInfo();
   bool SplitFramePushPop = STI.splitFramePushPop(MF);
   unsigned NumAlignedDPRCS2Regs = AFI->getNumAlignedDPRCS2Regs();
+  const struct SpillAreaProperty SpillAreaProperty =
+      SpillAreaProperties.at(SpillArea);
+  unsigned StmOpc = AFI->isThumbFunction() ? SpillAreaProperty.PushMultiThumb
+                                           : SpillAreaProperty.PushMultiARM;
+  unsigned StrOpc = AFI->isThumbFunction() ? SpillAreaProperty.PushOneThumb
+                                           : SpillAreaProperty.PushOneARM;
 
   DebugLoc DL;
 
   using RegAndKill = std::pair<unsigned, bool>;
 
   SmallVector<RegAndKill, 4> Regs;
   unsigned i = CSI.size();
   while (i != 0) {
     unsigned LastReg = 0;
     for (; i != 0; --i) {
       unsigned Reg = CSI[i-1].getReg();
       if (getRegSpillArea(Reg, SplitFramePushPop, NumAlignedDPRCS2Regs) !=
           SpillArea)
         continue;
 
       const MachineRegisterInfo &MRI = MF.getRegInfo();
       bool isLiveIn = MRI.isLiveIn(Reg);
       if (!isLiveIn && !MRI.isReserved(Reg))
         MBB.addLiveIn(Reg);
-      // If NoGap is true, push consecutive registers and then leave the rest
-      // for other instructions. e.g.
+      // If spill area only support consecutive registers, push consecutive
+      // registers and then leave the rest for other instructions. e.g.
       // vpush {d8, d10, d11} -> vpush {d8}, vpush {d10, d11}
-      if (NoGap && LastReg && LastReg != Reg-1)
+      if (!SpillAreaProperty.AllowDiscreteRegs && LastReg && LastReg != Reg - 1)
         break;
       LastReg = Reg;
       // Do not set a kill flag on values that are also marked as live-in. This
       // happens with the @llvm-returnaddress intrinsic and with arguments
       // passed in callee saved registers.
       // Omitting the kill flags is conservatively correct even if the live-in
       // is not used after all.
       Regs.push_back(std::make_pair(Reg, /*isKill=*/!isLiveIn));
while (i != 0) {
     if (MI != MBB.begin())
       --MI;
   }
 }
 
 void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
                                    MachineBasicBlock::iterator MI,
                                    std::vector<CalleeSavedInfo> &CSI,
-                                   unsigned LdmOpc, unsigned LdrOpc,
-                                   bool isVarArg, bool NoGap,
+                                   bool isVarArg,
                                    enum SpillArea SpillArea) const {
   MachineFunction &MF = *MBB.getParent();
   const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
   const TargetRegisterInfo &TRI = *STI.getRegisterInfo();
   ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
   bool SplitFramePushPop = STI.splitFramePushPop(MF);
   unsigned NumAlignedDPRCS2Regs = AFI->getNumAlignedDPRCS2Regs();
+  const struct SpillAreaProperty SpillAreaProperty =
+      SpillAreaProperties.at(SpillArea);
+  unsigned LdmOpc = AFI->isThumbFunction() ? SpillAreaProperty.PopMultiThumb
+                                           : SpillAreaProperty.PopMultiARM;
+  unsigned LdrOpc = AFI->isThumbFunction() ? SpillAreaProperty.PopOneThumb
+                                           : SpillAreaProperty.PopOneARM;
   DebugLoc DL;
   bool isTailCall = false;
   bool isInterrupt = false;
   bool isTrap = false;
   if (MBB.end() != MI) {
     DL = MI->getDebugLoc();
     unsigned RetOpcode = MI->getOpcode();
     isTailCall = (RetOpcode == ARM::TCRETURNdi || RetOpcode == ARM::TCRETURNri);
for (; i != 0; --i) {
           // We 'restore' LR into PC so it is not live out of the return block:
           // Clear Restored bit.
           Info.setRestored(false);
         } else
           LdmOpc = AFI->isThumbFunction() ? ARM::t2LDMIA_UPD : ARM::LDMIA_UPD;
         // Fold the return instruction into the LDM.
       }
 
-      // If NoGap is true, pop consecutive registers and then leave the rest
-      // for other instructions. e.g.
+      // If spill area only support consecutive registers, pop consecutive
+      // registers and then leave the rest for other instructions. e.g.
       // vpop {d8, d10, d11} -> vpop {d8}, vpop {d10, d11}
-      if (NoGap && LastReg && LastReg != Reg-1)
+      if (!SpillAreaProperty.AllowDiscreteRegs && LastReg && LastReg != Reg - 1)
         break;
 
       LastReg = Reg;
       Regs.push_back(Reg);
     }
 
     if (Regs.empty())
       continue;
bool ARMFrameLowering::spillCalleeSavedRegisters(MachineBasicBlock &MBB,
                                         const std::vector<CalleeSavedInfo> &CSI,
                                         const TargetRegisterInfo *TRI) const {
   if (CSI.empty())
     return false;
 
   MachineFunction &MF = *MBB.getParent();
   ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
 
-  unsigned PushOpc = AFI->isThumbFunction() ? ARM::t2STMDB_UPD : ARM::STMDB_UPD;
-  unsigned PushOneOpc = AFI->isThumbFunction() ?
-    ARM::t2STR_PRE : ARM::STR_PRE_IMM;
-  unsigned FltOpc = ARM::VSTMDDB_UPD;
   unsigned NumAlignedDPRCS2Regs = AFI->getNumAlignedDPRCS2Regs();
-  emitPushInst(MBB, MI, CSI, PushOpc, PushOneOpc, false, GPRCS1Area,
-               MachineInstr::FrameSetup);
-  emitPushInst(MBB, MI, CSI, PushOpc, PushOneOpc, false, GPRCS2Area,
-               MachineInstr::FrameSetup);
-  emitPushInst(MBB, MI, CSI, FltOpc, 0, true, DPRCS1Area,
-               MachineInstr::FrameSetup);
+  emitPushInst(MBB, MI, CSI, GPRCS1Area, MachineInstr::FrameSetup);
+  emitPushInst(MBB, MI, CSI, GPRCS2Area, MachineInstr::FrameSetup);
+  emitPushInst(MBB, MI, CSI, DPRCS1Area, MachineInstr::FrameSetup);
 
   // The code above does not insert spill code for the aligned DPRCS2 registers.
   // The stack realignment code will be inserted between the push instructions
   // and these spills.
   if (NumAlignedDPRCS2Regs)
     emitAlignedDPRCS2Spills(MBB, MI, NumAlignedDPRCS2Regs, CSI, TRI);
 
   return true;
bool ARMFrameLowering::restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
   bool isVarArg = AFI->getArgRegsSaveSize() > 0;
   unsigned NumAlignedDPRCS2Regs = AFI->getNumAlignedDPRCS2Regs();
 
   // The emitPopInst calls below do not insert reloads for the aligned DPRCS2
   // registers. Do that here instead.
   if (NumAlignedDPRCS2Regs)
     emitAlignedDPRCS2Restores(MBB, MI, NumAlignedDPRCS2Regs, CSI, TRI);
 
-  unsigned PopOpc = AFI->isThumbFunction() ? ARM::t2LDMIA_UPD : ARM::LDMIA_UPD;
-  unsigned LdrOpc = AFI->isThumbFunction() ? ARM::t2LDR_POST :ARM::LDR_POST_IMM;
-  unsigned FltOpc = ARM::VLDMDIA_UPD;
-  emitPopInst(MBB, MI, CSI, FltOpc, 0, isVarArg, true, DPRCS1Area);
-  emitPopInst(MBB, MI, CSI, PopOpc, LdrOpc, isVarArg, false, GPRCS2Area);
-  emitPopInst(MBB, MI, CSI, PopOpc, LdrOpc, isVarArg, false, GPRCS1Area);
+  emitPopInst(MBB, MI, CSI, isVarArg, DPRCS1Area);
+  emitPopInst(MBB, MI, CSI, isVarArg, GPRCS2Area);
+  emitPopInst(MBB, MI, CSI, isVarArg, GPRCS1Area);
 
   return true;
 }
 
 // FIXME: Make generic?
 static unsigned GetFunctionSizeInBytes(const MachineFunction &MF,
                                        const ARMBaseInstrInfo &TII) {
   unsigned FnSize = 0;