LivePhysRegs: Add support to add pristine registers when populating with live-in/live-out registers.

[MatzeB]

Summary

This feature is necessary for the ARMLoadStoreOpt cleanup that I will submit next.

[qcolombet]

Hi Matthias,

I miss something.
Why do we need to account for the pristine registers?

Thanks,
-Quentin

[MatzeB]

In D10139#181314, @qcolombet wrote:

Hi Matthias,

I miss something.
Why do we need to account for the pristine registers?

Before prologue/epilogue insertion we don't need to account for them. All current users for LivePhysRegs are before prologue/epilogue insertion so they won't need this (by default AddPristineRegs is false).

After prologue/epilogue insertion those registers are live, you cannot use them for temporary values or place an instruction that would clobber them. Now you can either include them in the set of live registers compute by this class, the alternative is that each time you ask LivePhysReg whether something is alive and you get a negative answer that it may be pristine registers and alive anyway. I decided that it less error prone to simply include the pristine registers in the live registers list.

[MatzeB]

In D10139#181352, @MatzeB wrote:

In D10139#181314, @qcolombet wrote:

Hi Matthias,

I miss something.
Why do we need to account for the pristine registers?

Before prologue/epilogue insertion we don't need to account for them. All current users for LivePhysRegs are before prologue/epilogue insertion so they won't need this (by default AddPristineRegs is false).

Actually I just realized that the StackMapLiveness pass uses LivePhysRegs after prologue/epilogue insertion; At a first glance I'd say this is a bug and will miss the pristine registers (though I'm not sure what the repercussions are if pristine regs are missed in patchpoint instruction (or maybe someone adjust for that somewhere else in the code).

[qcolombet]

I agree, the error proneness is a good argument!

Small comments inlined.

Cheers,
-Quentin

lib/CodeGen/LivePhysRegs.cpp

140 ↗

(On Diff #26821)

Is there a reason we are not using MachineFrameInfo::getPristineRegs?

I may be wrong, but at first glance it seems as if we copied/pasted the code from there.

[MatzeB]

lib/CodeGen/LivePhysRegs.cpp

140 ↗

(On Diff #26821)

This is mostly because the existing API is clunky: getPristineRegs() returns a BitVector by value(!) and has to compute an additional intermediate bitvector which we can avoid here by clearing and setting the registers directly in the LivePhysRegs.

[qcolombet]

LGTM.

Thanks Matthias!

Q.

llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h

public:
   /// recommended way, because it depends on accurate kill flags. If possible
   /// use stepBackwards() instead of this function.
   /// The clobbers set will be the list of registers either defined or clobbered
   /// by a regmask.  The operand will identify whether this is a regmask or
   /// register operand.
   void stepForward(const MachineInstr &MI,
         SmallVectorImpl<std::pair<unsigned, const MachineOperand*>> &Clobbers);
 
-  /// \brief Adds all live-in registers of basic block @p MBB.
-  void addLiveIns(const MachineBasicBlock *MBB) {
-    for (MachineBasicBlock::livein_iterator LI = MBB->livein_begin(),
-         LE = MBB->livein_end(); LI != LE; ++LI)
-      addReg(*LI);
-  }
-
-  /// \brief Adds all live-out registers of basic block @p MBB.
-  void addLiveOuts(const MachineBasicBlock *MBB) {
-    for (MachineBasicBlock::const_succ_iterator SI = MBB->succ_begin(),
-         SE = MBB->succ_end(); SI != SE; ++SI)
-      addLiveIns(*SI);
-  }
+  /// \brief Adds all live-in registers of basic block @p MBB; After prologue/
+  /// epilogue insertion \p AddPristines should be set to true to insert the
+  /// pristine registers.
+  void addLiveIns(const MachineBasicBlock *MBB, bool AddPristines = false);
+
+  /// \brief Adds all live-out registers of basic block @p MBB; After prologue/
+  /// epilogue insertion \p AddPristines should be set to true to insert the
+  /// pristine registers.
+  void addLiveOuts(const MachineBasicBlock *MBB, bool AddPristines = false);
 
   typedef SparseSet<unsigned>::const_iterator const_iterator;
   const_iterator begin() const { return LiveRegs.begin(); }
   const_iterator end() const { return LiveRegs.end(); }
 
   /// \brief Prints the currently live registers to @p OS.
   void print(raw_ostream &OS) const;
 

llvm/trunk/lib/CodeGen/LivePhysRegs.cpp

 //===--- LivePhysRegs.cpp - Live Physical Register Set --------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the LivePhysRegs utility for tracking liveness of
 // physical registers across machine instructions in forward or backward order.
 // A more detailed description can be found in the corresponding header file.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/LivePhysRegs.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
+#include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBundle.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 
 /// \brief Remove all registers from the set that get clobbered by the register
 /// mask.

 }
 
 /// Dumps the currently live registers to the debug output.
 void LivePhysRegs::dump() const {
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   dbgs() << "  " << *this;
 #endif
 }
+
+/// Add live-in registers of basic block \p MBB to \p LiveRegs.
+static void addLiveIns(LivePhysRegs &LiveRegs, const MachineBasicBlock &MBB) {
+  for (unsigned Reg : make_range(MBB.livein_begin(), MBB.livein_end()))
+    LiveRegs.addReg(Reg);
+}
+
+/// Add pristine registers to the given \p LiveRegs. This function removes
+/// actually saved callee save registers when \p InPrologueEpilogue is false.
+static void addPristines(LivePhysRegs &LiveRegs, const MachineFunction &MF,
+                         const TargetRegisterInfo &TRI) {
+  const MachineFrameInfo &MFI = *MF.getFrameInfo();
+  if (!MFI.isCalleeSavedInfoValid())
+    return;
+
+  for (const MCPhysReg *CSR = TRI.getCalleeSavedRegs(&MF); CSR && *CSR; ++CSR)
+    LiveRegs.addReg(*CSR);
+  for (const CalleeSavedInfo &Info : MFI.getCalleeSavedInfo())
+    LiveRegs.removeReg(Info.getReg());
+}
+
+void LivePhysRegs::addLiveOuts(const MachineBasicBlock *MBB,
+                               bool AddPristines) {
+  if (AddPristines) {
+    const MachineFunction &MF = *MBB->getParent();
+    addPristines(*this, MF, *TRI);
+  }
+  for (const MachineBasicBlock *Succ : MBB->successors())
+    ::addLiveIns(*this, *Succ);
+}
+
+void LivePhysRegs::addLiveIns(const MachineBasicBlock *MBB,
+                              bool AddPristines) {
+  if (AddPristines) {
+    const MachineFunction &MF = *MBB->getParent();
+    addPristines(*this, MF, *TRI);
+  }
+  ::addLiveIns(*this, *MBB);
+}