Add callee-saved registers as implicit uses in return instructions

[kparzysz]

Summary

Update the block live-ins to have the saved registers on the paths from the entry blocks to the save blocks, and have the restored registers on the paths from the restore blocks to the returns.

[kparzysz]

This patch doesn't update any other code that may be simplified as a result.

[MatzeB]

I really like the direction of this as it would get rid of the "pristine registers" and the "how to calculate live-outs of a return block" special cases.

Some notes:

• This needs some compiletime checking to ensure the pristine registers were really just a premature optimization rather than something that fundamentally improves compiletime. Though in the interest of simplicity/maintainability I'd even accept smallish compiletime changes.
• I still have to dive into the patches details, at a first glance I am surprised this needs the new getSavedRegisters()/getRestoredRegisters() callbacks. Seems like there is some exception handling magic going on, why do we need to special case it?

[kparzysz]

In D33003#749994, @MatzeB wrote:

• I still have to dive into the patches details, at a first glance I am surprised this needs the new getSavedRegisters()/getRestoredRegisters() callbacks. Seems like there is some exception handling magic going on, why do we need to special case it?

Magic #1: ARM saves LR, but restores the value directly into PC, and in general there is no way of telling that LR does not need to be restored. The general assumption is that the registers that are saved in the save blocks are the same that are later restored, and also the same registers that are live-out from the function (i.e. consumed by the return instructions). Without this, LR would have ended up as imp-use on return instructions, and caused "use of undefined register" errors.

Magic #2: X86 on Windows has some concept of funclets, whose entry points are treated as save blocks, but which do not save all of the registers in CSI. The exception handling runtime saves a few of them, and those ones are excluded from saving/restoring. I think it actually happens to save/restore all of them (based on my reading of the code). In this case, again we have to keep track which saves actually reached a given return instruction if we are to add the implicit uses correctly.

I don't like these hooks that much either.

[kparzysz]

In D33003#749994, @MatzeB wrote:

I really like the direction of this as it would get rid of the "pristine registers"

This patch wouldn't allow us to get rid of pristine registers, unless it added them as live-ins to all blocks (and implicit uses of them to all returns). Is that what you have in mind?

[kparzysz]

Add pristine registers as live-in to all blocks that are on a path from entry to a return block.

This causes one X86 testcase to fail: CodeGen/X86/break-false-dep.ll

test/CodeGen/X86/break-false-dep.ll:317:7: error: expected string not found in input
;AVX: vcvtsi2sdq {{.*}}, %xmm6, {{%xmm[0-9]+}}
      ^
<stdin>:496:2: note: scanning from here
 #APP
 ^
<stdin>:585:2: note: possible intended match here
 vcvtsi2sdq %r11, %xmm2, %xmm4
 ^

[kparzysz]

The difference is that XMM6 is a pristine register, and is started showing up as live-in in basic blocks. This lead the ExecutionDepsFix to pick a different register to use in an undef use (and it happened to be XMM2).

[kparzysz]

Any comments since the patch has been updated? It now adds pristine registers as live-in to all blocks between entry and exit. It caused one testcase failure, but the failure seems harmless.

[MatzeB]

As far as I understand it the main trick here is introducing the getSavedRegisters()/getRestoredRegisters() callbacks, because as it turns out the CalleeSavedInfo is not flexible enough to express what actually happens in practice (as you explained above exceptions and EH landing pads).

I found the code in updateLiveness() perfectly esp. compared to the new one that visited the blocks to fill a bitvector and then does something with every set bit, no idea why the step through the bitvector is necessary. Should be easy enough to do this while working like the original code. There are some good updates there like using a bitvector for the visited set or block numbers for the worklist. But factoring this out into the getAllPaths() function feels cumbersome: It requires you to first transform the list of save blocks/restore blocks which is already a nice list into a bitset just so the bitset is iterated again and put into the worklist. Similar with the results: You already visit the relevant blocks but instead of updating the liveins lists you set a bit in the bitset just so that caller has to iterate the bitset again to do something for each visited block.

If this patch works then we should be able to simplify the code in LivePhysRegs::addLiveOutsNoPristines() by removing all special handling for return blocks and LivePhysRegs::addLiveOuts() to simply add the pristine registers in the return block case.

lib/CodeGen/PrologEpilogInserter.cpp

975–981

• Those two loops should be merged.
• I guess it's customary to call block variables MBB in CodeGen.
• On a general note I wish that we could make better assumptions about EH landing pads, but I understand why you did it this way and it's fine for this patch.

982

Maybe increasing this to 16 or 32; I'd expect more than 4 CSRs to be common.

983

Could use MCPhysReg instead of unsigned.

984–985

Francis landed a patch recently that finally allows for(unsigned PRegs : Pristine.set_bits()). Similar in a few other places here (though as noted in the general comments, it feels like too many unnecessary intermediate bitsets to me).

990

Sorry for the confusion about the pristine registers.
Handling them like this is wrong: You need to push them into every single livein list in the function, not just to the ones betweeen entry/save and restore/return.

I think we better leave that to another patch then (after measuring the impact on compiletime/codesize).

1008

A normal list should do here, as you already habe visited sets around. You only add stuff to the worklist that is not in the visited set yet. That allows you to use a simple Vector again.

lib/Target/ARM/ARMLoadStoreOptimizer.cpp

1930–1936

Maybe factor out the duplicated code.

[kparzysz]

In D33003#767147, @MatzeB wrote:

But factoring this out into the getAllPaths() function feels cumbersome: It requires you to first transform the list of save blocks/restore blocks which is already a nice list into a bitset just so the bitset is iterated again and put into the worklist. Similar with the results: You already visit the relevant blocks but instead of updating the liveins lists you set a bit in the bitset just so that caller has to iterate the bitset again to do something for each visited block.

One of the factors that contributed to this choice was that getting the set of all blocks on paths between two other sets of blocks (e.g. restore blocks and returning blocks) seems like a useful utility to have. The need for calculating this could reasonably arise outside of PEI (I remember needing/wanting this for some work op Hexagon optimizations). It could be placed in another file, but there is no "precedent" for that (we have BasicBlockUtils.h for IR-level blocks, but not for CodeGen).

Regarding the use of BitVector---sure, I can change it to something more universal (any model of a set that can be iterated over would do). BitVector has both: cheap test for membership and iteration, and as a bonus a predictable traversal ordering (based on block numbers). Copying the argument into another structure (work list) may seem cumbersome, but it would need to happen regardless of what structure is used.

I'll leave the bit vector in for now and address your other comments.

[kparzysz]

Addressed the comments. Removed the handling of pristine registers from this version.

lib/CodeGen/PrologEpilogInserter.cpp

984–985

Removed this particular statement, but used the set_bits range in other places.

include/llvm/Target/TargetFrameLowering.h

public:
   /// Returns false otherwise.
   virtual bool restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
                                            MachineBasicBlock::iterator MI,
                                         const std::vector<CalleeSavedInfo> &CSI,
                                         const TargetRegisterInfo *TRI) const {
     return false;
   }
 
+  /// Get the set of callee-saved registers that were saved in this basic
+  /// block. SaveBB must be a save block.
+  virtual void getSavedRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                 MachineBasicBlock &SaveBB,
+                                 const std::vector<CalleeSavedInfo> &CSI,
+                                 const TargetRegisterInfo *TRI) const;
+
+  /// Get the set of callee-saved registers that were restored in this basic
+  /// block. RestoreBB must be a restore block.
+  virtual void getRestoredRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                    MachineBasicBlock &RestoreBB,
+                                    const std::vector<CalleeSavedInfo> &CSI,
+                                    const TargetRegisterInfo *TRI) const;
+
   /// Return true if the target needs to disable frame pointer elimination.
   virtual bool noFramePointerElim(const MachineFunction &MF) const;
 
   /// hasFP - Return true if the specified function should have a dedicated
   /// frame pointer register. For most targets this is true only if the function
   /// has variable sized allocas or if frame pointer elimination is disabled.
   virtual bool hasFP(const MachineFunction &MF) const = 0;
 

lib/CodeGen/PrologEpilogInserter.cpp

private:
   void calculateCallFrameInfo(MachineFunction &Fn);
   void calculateSaveRestoreBlocks(MachineFunction &Fn);
 
   void calculateFrameObjectOffsets(MachineFunction &Fn);
   void replaceFrameIndices(MachineFunction &Fn);
   void replaceFrameIndices(MachineBasicBlock *BB, MachineFunction &Fn,
                            int &SPAdj);
   void insertPrologEpilogCode(MachineFunction &Fn);
+
+  void updateCalleeSavedLiveness(MachineFunction &Fn);
+  void getAllPaths(BitVector &Paths, const MachineFunction &Fn,
+                   const BitVector &Starts, const BitVector &Targets) const;
+  void addLiveInsToBlocks(MachineFunction &Fn, const BitVector &Blocks,
+                          const SmallVectorImpl<MCPhysReg> &Regs) const;
+  void updateEntryPaths(MachineFunction &Fn, const BitVector &Entries,
+                        MachineBasicBlock &SaveBB,
+                        const SmallVectorImpl<MCPhysReg> &Regs) const;
+  void updateExitPaths(MachineFunction &Fn, const BitVector &Returns,
+                       MachineBasicBlock &RestBB,
+                       const SmallVectorImpl<MCPhysReg> &Regs) const;
+  void updatePathLiveIns(MachineFunction &Fn, BitVector &Starts,
+                         BitVector &Targets,
+                         SmallVectorImpl<MCPhysReg> &Regs) const;
 };
 } // namespace
 
 char PEI::ID = 0;
 char &llvm::PrologEpilogCodeInserterID = PEI::ID;
 
 static cl::opt<unsigned>
 WarnStackSize("warn-stack-size", cl::Hidden, cl::init((unsigned)-1),
for (auto &CS : CSI) {
 
       CS.setFrameIdx(FrameIdx);
     }
   }
 
   MFI.setCalleeSavedInfo(CSI);
 }
 
-/// Helper function to update the liveness information for the callee-saved
-/// registers.
-static void updateLiveness(MachineFunction &MF) {
-  MachineFrameInfo &MFI = MF.getFrameInfo();
-  // Visited will contain all the basic blocks that are in the region
-  // where the callee saved registers are alive:
-  // - Anything that is not Save or Restore -> LiveThrough.
-  // - Save -> LiveIn.
-  // - Restore -> LiveOut.
-  // The live-out is not attached to the block, so no need to keep
-  // Restore in this set.
-  SmallPtrSet<MachineBasicBlock *, 8> Visited;
-  SmallVector<MachineBasicBlock *, 8> WorkList;
-  MachineBasicBlock *Entry = &MF.front();
-  MachineBasicBlock *Save = MFI.getSavePoint();
-
-  if (!Save)
-    Save = Entry;
-
-  if (Entry != Save) {
-    WorkList.push_back(Entry);
-    Visited.insert(Entry);
-  }
-  Visited.insert(Save);
-
-  MachineBasicBlock *Restore = MFI.getRestorePoint();
-  if (Restore)
-    // By construction Restore cannot be visited, otherwise it
-    // means there exists a path to Restore that does not go
-    // through Save.
-    WorkList.push_back(Restore);
-
-  while (!WorkList.empty()) {
-    const MachineBasicBlock *CurBB = WorkList.pop_back_val();
-    // By construction, the region that is after the save point is
-    // dominated by the Save and post-dominated by the Restore.
-    if (CurBB == Save && Save != Restore)
-      continue;
-    // Enqueue all the successors not already visited.
-    // Those are by construction either before Save or after Restore.
-    for (MachineBasicBlock *SuccBB : CurBB->successors())
-      if (Visited.insert(SuccBB).second)
-        WorkList.push_back(SuccBB);
-  }
-
-  const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
-
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  for (unsigned i = 0, e = CSI.size(); i != e; ++i) {
-    for (MachineBasicBlock *MBB : Visited) {
-      MCPhysReg Reg = CSI[i].getReg();
-      // Add the callee-saved register as live-in.
-      // It's killed at the spill.
-      if (!MRI.isReserved(Reg) && !MBB->isLiveIn(Reg))
-        MBB->addLiveIn(Reg);
-    }
-  }
-}
-
 /// insertCSRSpillsAndRestores - Insert spill and restore code for
 /// callee saved registers used in the function.
 ///
 static void insertCSRSpillsAndRestores(MachineFunction &Fn,
                                        const MBBVector &SaveBlocks,
                                        const MBBVector &RestoreBlocks) {
   // Get callee saved register information.
   MachineFrameInfo &MFI = Fn.getFrameInfo();
if (!TFI->spillCalleeSavedRegisters(*SaveBlock, I, CSI, TRI)) {
       for (unsigned i = 0, e = CSI.size(); i != e; ++i) {
         // Insert the spill to the stack frame.
         unsigned Reg = CSI[i].getReg();
         const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
         TII.storeRegToStackSlot(*SaveBlock, I, Reg, true, CSI[i].getFrameIdx(),
                                 RC, TRI);
       }
     }
-    // Update the live-in information of all the blocks up to the save point.
-    updateLiveness(Fn);
   }
 
   // Restore using target interface.
   for (MachineBasicBlock *MBB : RestoreBlocks) {
     I = MBB->end();
 
     // Skip over all terminator instructions, which are part of the return
     // sequence.
void PEI::insertPrologEpilogCode(MachineFunction &Fn) {
   // Add prologue to the function...
   for (MachineBasicBlock *SaveBlock : SaveBlocks)
     TFI.emitPrologue(Fn, *SaveBlock);
 
   // Add epilogue to restore the callee-save registers in each exiting block.
   for (MachineBasicBlock *RestoreBlock : RestoreBlocks)
     TFI.emitEpilogue(Fn, *RestoreBlock);
 
+  updateCalleeSavedLiveness(Fn);
+
   for (MachineBasicBlock *SaveBlock : SaveBlocks)
     TFI.inlineStackProbe(Fn, *SaveBlock);
 
   // Emit additional code that is required to support segmented stacks, if
   // we've been asked for it.  This, when linked with a runtime with support
   // for segmented stacks (libgcc is one), will result in allocating stack
   // space in small chunks instead of one large contiguous block.
   if (Fn.shouldSplitStack()) {
     for (MachineBasicBlock *SaveBlock : SaveBlocks)
       TFI.adjustForSegmentedStacks(Fn, *SaveBlock);
   }
 
   // Emit additional code that is required to explicitly handle the stack in
   // HiPE native code (if needed) when loaded in the Erlang/OTP runtime. The
   // approach is rather similar to that of Segmented Stacks, but it uses a
   // different conditional check and another BIF for allocating more stack
   // space.
   if (Fn.getFunction()->getCallingConv() == CallingConv::HiPE)
     for (MachineBasicBlock *SaveBlock : SaveBlocks)
       TFI.adjustForHiPEPrologue(Fn, *SaveBlock);
 }
 
+void PEI::updateCalleeSavedLiveness(MachineFunction &Fn) {
+  const TargetFrameLowering &TFI = *Fn.getSubtarget().getFrameLowering();
+  MachineFrameInfo &MFI = Fn.getFrameInfo();
+  const TargetRegisterInfo *TRI = Fn.getSubtarget().getRegisterInfo();
+  const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
+
+  unsigned NumBN = Fn.getNumBlockIDs();
+  BitVector Entries(NumBN), Returns(NumBN);
+  Entries[Fn.front().getNumber()] = true;
+  for (MachineBasicBlock &MBB : Fn) {
+    unsigned N = MBB.getNumber();
+    if (MBB.isReturnBlock())
+      Returns[N] = true;
+    if (MBB.isEHFuncletEntry())
+      Entries[N] = true;
+  }
+
+  SmallVector<MCPhysReg,32> Regs;
+
+  for (MachineBasicBlock *B : SaveBlocks) {
+    Regs.clear();
+    TFI.getSavedRegisters(Regs, *B, CSI, TRI);
+    if (!Regs.empty())
+      updateEntryPaths(Fn, Entries, *B, Regs);
+  }
+
+  for (MachineBasicBlock *B : RestoreBlocks) {
+    Regs.clear();
+    TFI.getRestoredRegisters(Regs, *B, CSI, TRI);
+    if (Regs.empty())
+      updateExitPaths(Fn, Returns, *B, Regs);
+  }
+}
+
+void PEI::getAllPaths(BitVector &Paths, const MachineFunction &Fn,
+                      const BitVector &Starts, const BitVector &Targets) const {
+  BitVector Up(Starts.size()); // Blocks reachable from Targets going upwards.
+  BitVector &Down = Paths;     // Blocks reachable from Starts going downwards.
+  SmallVector<unsigned,16> Worklist;
+
+  for (unsigned N : Starts.set_bits())
+    Worklist.push_back(N);
+  for (unsigned i = 0; i < Worklist.size(); ++i) {
+    unsigned N = Worklist[i];
+    if (Down[N])
+      continue;
+    Down[N] = true;
+    if (Targets[N])
+      continue;
+    MachineBasicBlock &B = *Fn.getBlockNumbered(N);
+    for (MachineBasicBlock *SB : B.successors())
+      Worklist.push_back(SB->getNumber());
+  }
+
+  Worklist.clear();
+  for (unsigned N : Targets.set_bits())
+    Worklist.push_back(N);
+
+  for (unsigned i = 0; i < Worklist.size(); ++i) {
+    unsigned N = Worklist[i];
+    if (Up[N])
+      continue;
+    Up[N] = true;
+    if (Starts[N])
+      continue;
+    MachineBasicBlock &B = *Fn.getBlockNumbered(N);
+    for (MachineBasicBlock *PB : B.predecessors())
+      Worklist.push_back(PB->getNumber());
+  }
+
+  Down &= Up;
+}
+
+void PEI::addLiveInsToBlocks(MachineFunction &Fn, const BitVector &Blocks,
+                             const SmallVectorImpl<MCPhysReg> &Regs) const {
+  for (unsigned N : Blocks.set_bits()) {
+    MachineBasicBlock &B = *Fn.getBlockNumbered(N);
+    for (unsigned R : Regs)
+      B.addLiveIn(R);
+    B.sortUniqueLiveIns();
+  }
+}
+
+void PEI::updateEntryPaths(MachineFunction &Fn, const BitVector &Entries,
+      MachineBasicBlock &SaveBB, const SmallVectorImpl<MCPhysReg> &Regs) const {
+  unsigned NumBN = Fn.getNumBlockIDs();
+  BitVector Paths(NumBN), Saves(NumBN);
+
+  Saves[SaveBB.getNumber()] = true;
+  getAllPaths(Paths, Fn, Entries, Saves);
+  addLiveInsToBlocks(Fn, Paths, Regs);
+}
+
+void PEI::updateExitPaths(MachineFunction &Fn, const BitVector &Returns,
+      MachineBasicBlock &RestBB, const SmallVectorImpl<MCPhysReg> &Regs) const {
+  unsigned NumBN = Fn.getNumBlockIDs();
+  BitVector Paths(NumBN), Restores(NumBN);
+
+  if (!Returns[RestBB.getNumber()]) {
+    // Start with the successors of the restore block because we don't want
+    // to add live-ins to the restore block itself.
+    for (MachineBasicBlock *SB : RestBB.successors())
+      Restores[SB->getNumber()] = true;
+    getAllPaths(Paths, Fn, Restores, Returns);
+    addLiveInsToBlocks(Fn, Paths, Regs);
+  } else
+    Paths[RestBB.getNumber()] = true;
+
+  // Add implicit uses to all reached return instructions.
+  Paths &= Returns;
+
+  const TargetRegisterInfo *TRI = Fn.getSubtarget().getRegisterInfo();
+  for (unsigned N : Paths.set_bits()) {
+    MachineBasicBlock &RetB = *Fn.getBlockNumbered(N);
+    for (MachineInstr &T : RetB.terminators()) {
+      if (!T.isReturn())  // XXX or a tail call
+        continue;
+      for (unsigned R : Regs) {
+        // The returning instruction may actually be the one that does the
+        // restoring of the CS registers: a target may tail-call a stub
+        // that restores the registers and returns to the original caller.
+        // These instructions should not have the CS registers as uses.
+        if (!T.modifiesRegister(R, TRI))
+          T.addOperand(MachineOperand::CreateReg(R, false, true));
+      }
+    }
+  }
+}
+
 /// replaceFrameIndices - Replace all MO_FrameIndex operands with physical
 /// register references and actual offsets.
 ///
 void PEI::replaceFrameIndices(MachineFunction &Fn) {
   const TargetFrameLowering &TFI = *Fn.getSubtarget().getFrameLowering();
   if (!TFI.needsFrameIndexResolution(Fn)) return;
 
   // Store SPAdj at exit of a basic block.

lib/CodeGen/TargetFrameLoweringImpl.cpp

 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cstdlib>
 using namespace llvm;
 
 TargetFrameLowering::~TargetFrameLowering() {
 }
 
+/// By default, all callee-saved registers are considered to be saved in
+/// each save block.
+void TargetFrameLowering::getSavedRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                       MachineBasicBlock &SaveBB,
+                                       const std::vector<CalleeSavedInfo> &CSI,
+                                       const TargetRegisterInfo *TRI) const {
+  for (const CalleeSavedInfo &I : CSI)
+    Regs.push_back(I.getReg());
+}
+
+/// By default, all callee-saved registers are considered to be restored in
+/// each restore block.
+void TargetFrameLowering::getRestoredRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                      MachineBasicBlock &RestoreBB,
+                                      const std::vector<CalleeSavedInfo> &CSI,
+                                      const TargetRegisterInfo *TRI) const {
+  for (const CalleeSavedInfo &I : CSI)
+    Regs.push_back(I.getReg());
+}
+
 /// The default implementation just looks at attribute "no-frame-pointer-elim".
 bool TargetFrameLowering::noFramePointerElim(const MachineFunction &MF) const {
   auto Attr = MF.getFunction()->getFnAttribute("no-frame-pointer-elim");
   return Attr.getValueAsString() == "true";
 }
 
 /// Returns the displacement from the frame register to the stack
 /// frame of the specified index, along with the frame register used

lib/Target/ARM/ARMFrameLowering.h

bool spillCalleeSavedRegisters(MachineBasicBlock &MBB,
                                  const std::vector<CalleeSavedInfo> &CSI,
                                  const TargetRegisterInfo *TRI) const override;
 
   bool restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
                                   MachineBasicBlock::iterator MI,
                                   const std::vector<CalleeSavedInfo> &CSI,
                                   const TargetRegisterInfo *TRI) const override;
 
+  void getRestoredRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                            MachineBasicBlock &RestoreBB,
+                            const std::vector<CalleeSavedInfo> &CSI,
+                            const TargetRegisterInfo *TRI) const override;
+
   bool noFramePointerElim(const MachineFunction &MF) const override;
 
   bool hasFP(const MachineFunction &MF) const override;
   bool hasReservedCallFrame(const MachineFunction &MF) const override;
   bool canSimplifyCallFramePseudos(const MachineFunction &MF) const override;
   int getFrameIndexReference(const MachineFunction &MF, int FI,
                              unsigned &FrameReg) const override;
   int ResolveFrameIndexReference(const MachineFunction &MF, int FI,

lib/Target/ARM/ARMFrameLowering.cpp

bool ARMFrameLowering::restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
   emitPopInst(MBB, MI, CSI, PopOpc, LdrOpc, isVarArg, false,
               &isARMArea2Register, 0);
   emitPopInst(MBB, MI, CSI, PopOpc, LdrOpc, isVarArg, false,
               &isARMArea1Register, 0);
 
   return true;
 }
 
+void ARMFrameLowering::getRestoredRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                        MachineBasicBlock &RestoreBB,
+                                        const std::vector<CalleeSavedInfo> &CSI,
+                                        const TargetRegisterInfo *TRI) const {
+  for (const CalleeSavedInfo &I : CSI)
+    if (I.getReg() != ARM::LR)
+      Regs.push_back(I.getReg());
+}
+
+
 // FIXME: Make generic?
 static unsigned GetFunctionSizeInBytes(const MachineFunction &MF,
                                        const ARMBaseInstrInfo &TII) {
   unsigned FnSize = 0;
   for (auto &MBB : MF) {
     for (auto &MI : MBB)
       FnSize += TII.getInstSizeInBytes(MI);
   }

lib/Target/ARM/ARMLoadStoreOptimizer.cpp

private:
     bool FixInvalidRegPairOp(MachineBasicBlock &MBB,
                              MachineBasicBlock::iterator &MBBI);
     bool MergeBaseUpdateLoadStore(MachineInstr *MI);
     bool MergeBaseUpdateLSMultiple(MachineInstr *MI);
     bool MergeBaseUpdateLSDouble(MachineInstr &MI) const;
     bool LoadStoreMultipleOpti(MachineBasicBlock &MBB);
     bool MergeReturnIntoLDM(MachineBasicBlock &MBB);
     bool CombineMovBx(MachineBasicBlock &MBB);
+
+    void copyUnmodifiedImplicitOps(MachineInstr &Dst, const MachineInstr &Src);
   };
   char ARMLoadStoreOpt::ID = 0;
 }
 
 INITIALIZE_PASS(ARMLoadStoreOpt, "arm-ldst-opt", ARM_LOAD_STORE_OPT_NAME, false,
                 false)
 
 static bool definesCPSR(const MachineInstr &MI) {
bool ARMLoadStoreOpt::LoadStoreMultipleOpti(MachineBasicBlock &MBB) {
   // Try to fold add/sub into the LDRD/STRD formed by ARMPreAllocLoadStoreOpt.
   for (MachineInstr *MI : MergeBaseCandidates)
     MergeBaseUpdateLSDouble(*MI);
   MergeBaseCandidates.clear();
 
   return Changed;
 }
 
+void ARMLoadStoreOpt::copyUnmodifiedImplicitOps(MachineInstr &Dst,
+                                                const MachineInstr &Src) {
+  // Do not copy implicit uses from the source instruction that are
+  // modified by the target instruction.
+  for (const MachineOperand &Op : Src.operands()) {
+    if (!Op.isReg() || !Op.isImplicit())
+      continue;
+    if (Op.isUse() && Dst.modifiesRegister(Op.getReg(), TRI))
+      continue;
+    Dst.addOperand(Op);
+  }
+}
+
 /// If this is a exit BB, try merging the return ops ("bx lr" and "mov pc, lr")
 /// into the preceding stack restore so it directly restore the value of LR
 /// into pc.
 ///   ldmfd sp!, {..., lr}
 ///   bx lr
 /// or
 ///   ldmfd sp!, {..., lr}
 ///   mov pc, lr
if (Opcode == ARM::LDMIA_UPD || Opcode == ARM::LDMDA_UPD ||
       MachineOperand &MO = PrevMI.getOperand(PrevMI.getNumOperands() - 1);
       if (MO.getReg() != ARM::LR)
         return false;
       unsigned NewOpc = (isThumb2 ? ARM::t2LDMIA_RET : ARM::LDMIA_RET);
       assert(((isThumb2 && Opcode == ARM::t2LDMIA_UPD) ||
               Opcode == ARM::LDMIA_UPD) && "Unsupported multiple load-return!");
       PrevMI.setDesc(TII->get(NewOpc));
       MO.setReg(ARM::PC);
-      PrevMI.copyImplicitOps(*MBB.getParent(), *MBBI);
+      copyUnmodifiedImplicitOps(PrevMI, *MBBI);
       MBB.erase(MBBI);
       return true;
     }
   }
   return false;
 }
 
 bool ARMLoadStoreOpt::CombineMovBx(MachineBasicBlock &MBB) {
   MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator();
   if (MBBI == MBB.begin() || MBBI == MBB.end() ||
       MBBI->getOpcode() != ARM::tBX_RET)
     return false;
 
   MachineBasicBlock::iterator Prev = MBBI;
   --Prev;
   if (Prev->getOpcode() != ARM::tMOVr || !Prev->definesRegister(ARM::LR))
     return false;
 
   for (auto Use : Prev->uses())
     if (Use.isKill()) {
-      BuildMI(MBB, MBBI, MBBI->getDebugLoc(), TII->get(ARM::tBX))
+      auto NewMI = BuildMI(MBB, MBBI, MBBI->getDebugLoc(), TII->get(ARM::tBX))
           .addReg(Use.getReg(), RegState::Kill)
-          .add(predOps(ARMCC::AL))
-          .copyImplicitOps(*MBBI);
+          .add(predOps(ARMCC::AL));
+      copyUnmodifiedImplicitOps(*NewMI, *MBBI);
       MBB.erase(MBBI);
       MBB.erase(Prev);
       return true;
     }
 
   llvm_unreachable("tMOVr doesn't kill a reg before tBX_RET?");
 }
 
 bool ARMLoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
   if (skipFunction(*Fn.getFunction()))
     return false;
 
   MF = &Fn;
   STI = &static_cast<const ARMSubtarget &>(Fn.getSubtarget());

lib/Target/X86/X86FrameLowering.h

bool spillCalleeSavedRegisters(MachineBasicBlock &MBB,
                                  const std::vector<CalleeSavedInfo> &CSI,
                                  const TargetRegisterInfo *TRI) const override;
 
   bool restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
                                   MachineBasicBlock::iterator MI,
                                   const std::vector<CalleeSavedInfo> &CSI,
                                   const TargetRegisterInfo *TRI) const override;
 
+  void getSavedRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                         MachineBasicBlock &SaveBB,
+                         const std::vector<CalleeSavedInfo> &CSI,
+                         const TargetRegisterInfo *TRI) const override;
+
+  void getRestoredRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                            MachineBasicBlock &RestoreBB,
+                            const std::vector<CalleeSavedInfo> &CSI,
+                            const TargetRegisterInfo *TRI) const override;
+
   bool hasFP(const MachineFunction &MF) const override;
   bool hasReservedCallFrame(const MachineFunction &MF) const override;
   bool canSimplifyCallFramePseudos(const MachineFunction &MF) const override;
   bool needsFrameIndexResolution(const MachineFunction &MF) const override;
 
   int getFrameIndexReference(const MachineFunction &MF, int FI,
                              unsigned &FrameReg) const override;
 

lib/Target/X86/X86FrameLowering.cpp

if (!X86::GR64RegClass.contains(Reg) &&
       continue;
 
     BuildMI(MBB, MI, DL, TII.get(Opc), Reg)
         .setMIFlag(MachineInstr::FrameDestroy);
   }
   return true;
 }
 
+void X86FrameLowering::getSavedRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                        MachineBasicBlock &SaveBB,
+                                        const std::vector<CalleeSavedInfo> &CSI,
+                                        const TargetRegisterInfo *TRI) const {
+  if (!SaveBB.isEHFuncletEntry() || !STI.is32Bit() || !STI.isOSWindows())
+    return TargetFrameLowering::getSavedRegisters(Regs, SaveBB, CSI, TRI);
+}
+
+void X86FrameLowering::getRestoredRegisters(SmallVectorImpl<MCPhysReg> &Regs,
+                                        MachineBasicBlock &RestoreBB,
+                                        const std::vector<CalleeSavedInfo> &CSI,
+                                        const TargetRegisterInfo *TRI) const {
+  assert(!RestoreBB.empty());
+  MachineInstr &LastI = RestoreBB.back();
+  if (isFuncletReturnInstr(LastI) && STI.isOSWindows()) {
+    // Don't restore CSRs in 32-bit EH funclets. Matches
+    // spillCalleeSavedRegisters.
+    if (STI.is32Bit())
+      return;
+    // Don't restore CSRs before an SEH catchret. SEH except blocks do not form
+    // funclets. emitEpilogue transforms these to normal jumps.
+    if (LastI.getOpcode() == X86::CATCHRET) {
+      const Function *Func = RestoreBB.getParent()->getFunction();
+      bool IsSEH = isAsynchronousEHPersonality(
+          classifyEHPersonality(Func->getPersonalityFn()));
+      if (IsSEH)
+        return;
+    }
+  }
+  return TargetFrameLowering::getRestoredRegisters(Regs, RestoreBB, CSI, TRI);
+}
+
 void X86FrameLowering::determineCalleeSaves(MachineFunction &MF,
                                             BitVector &SavedRegs,
                                             RegScavenger *RS) const {
   TargetFrameLowering::determineCalleeSaves(MF, SavedRegs, RS);
 
   MachineFrameInfo &MFI = MF.getFrameInfo();
 
   X86MachineFunctionInfo *X86FI = MF.getInfo<X86MachineFunctionInfo>();

test/CodeGen/PowerPC/ppc-shrink-wrapping.ll

 ; CHECK: mtctr [[IV]]
 ;
 ; CHECK: .[[LOOP_LABEL:LBB[0-9_]+]]: # %for.body
 ; Inline asm statement.
 ; CHECK: addi 14, 14, 1
 ; CHECK: bdnz .[[LOOP_LABEL]]
 ;
 ; Epilogue code.
-; CHECK: li 3, 0
+; CHECK-DAG: li 3, 0
 ; CHECK-DAG: ld 14, -[[STACK_OFFSET]](1) # 8-byte Folded Reload
 ; CHECK: nop
 ; CHECK: blr
 ;
 ; CHECK: [[ELSE_LABEL]]
-; CHECK-NEXT: slwi 3, 4, 1
-; DISABLE: ld 14, -[[STACK_OFFSET]](1) # 8-byte Folded Reload
+; CHECK-DAG: slwi 3, 4, 1
+; DISABLE-DAG: ld 14, -[[STACK_OFFSET]](1) # 8-byte Folded Reload
 ; CHECK-NEXT: blr
 ; 
 define i32 @inlineAsm(i32 %cond, i32 %N) {
 entry:
   %tobool = icmp eq i32 %cond, 0
   br i1 %tobool, label %if.else, label %for.preheader
 
 for.preheader: