Replace MachineRegisterInfo::isSSA() with a MachineFunctionProperty

[dschuff]

Summary

Use the MachineFunctionProperty mechanism to indicate whether a MachineFunction
is in SSA form instead of a custom method on MachineRegisterInfo. NFC.

[dschuff]

The syntax for MachineFunctionProperties is fairly verbose. I'm wondering if we want to do one or more of the following:

1. Shorten the name of MachineFunctionProperties and/or MachineFunctionProperties::Property
2. Rename MachineFunctionProperties::hasProperty (maybe test to match BitVector, or perhaps operator[])
3. Rename MachineFunction::getProperties or perhaps replace it with a hasProperty or testProperty method on MachineFunction directly
4. Make MachineFunctionProperties::Property a C-style enum rather than a scoped enum

I would say maybe 1 and 2, but I don't have a great idea for what names I'd use for 1.

[qcolombet]

include/llvm/CodeGen/MachineRegisterInfo.h

163 ↗

(On Diff #51976)

Can we keep the getter and setter, just update what they do?

[dschuff]

Keep MRI accessor

[dschuff]

Put the MRI accessor back, and kept all of its uses.
(But kept the new MachineFunction::print() behavior)

include/llvm/CodeGen/MachineRegisterInfo.h

163 ↗

(On Diff #51976)

We can; that would obviously make this change smaller, and be easier on out-of-tree targets. Longer term if/when we get more properties we might still want to make them nicer to use. But I'm fine with keeping this accessor; the compiled code would pretty much be identical anyway.

[qcolombet]

include/llvm/CodeGen/MachineRegisterInfo.h

163 ↗

(On Diff #52580)

Agree.

llvm/trunk/include/llvm/CodeGen/MachineFunction.h

class MachineFunctionProperties {
   // Possible TODO: Allow requiring the negative (e.g. VRegsAllocated could be
   // stated as the negative of "has vregs"
 
 public:
   // The properties are stated in "positive" form; i.e. a pass could require
   // that the property hold, but not that it does not hold.
 
   // Property descriptions:
-  // IsSSA (currently unused, intended to eventually replace
-  // MachineRegisterInfo::isSSA())
+  // IsSSA: True when the machine function is in SSA form and virtual registers
+  // have a single def.
   // TracksLiveness: (currently unsued, intended to eventually replace
   // MachineRegisterInfo::tracksLiveness())
   // AllVRegsAllocated: All virtual registers have been allocated; i.e. all
   // register operands are physical registers.
   enum class Property : unsigned {
     IsSSA,
     TracksLiveness,
     AllVRegsAllocated,

llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h

class Delegate {
     virtual void anchor();
   public:
     virtual void MRI_NoteNewVirtualRegister(unsigned Reg) = 0;
 
     virtual ~Delegate() {}
   };
 
 private:
-  const MachineFunction *MF;
+  MachineFunction *MF;
   Delegate *TheDelegate;
 
-  /// IsSSA - True when the machine function is in SSA form and virtual
-  /// registers have a single def.
-  bool IsSSA;
-
   /// TracksLiveness - True while register liveness is being tracked accurately.
   /// Basic block live-in lists, kill flags, and implicit defs may not be
   /// accurate when after this flag is cleared.
   bool TracksLiveness;
 
   /// True if subregister liveness is tracked.
   bool TracksSubRegLiveness;
 
private:
   /// Live in values are typically arguments in registers.  LiveIn values are
   /// allowed to have virtual registers associated with them, stored in the
   /// second element.
   std::vector<std::pair<unsigned, unsigned> > LiveIns;
 
   MachineRegisterInfo(const MachineRegisterInfo&) = delete;
   void operator=(const MachineRegisterInfo&) = delete;
 public:
-  explicit MachineRegisterInfo(const MachineFunction *MF);
+  explicit MachineRegisterInfo(MachineFunction *MF);
 
   const TargetRegisterInfo *getTargetRegisterInfo() const {
     return MF->getSubtarget().getRegisterInfo();
   }
 
   void resetDelegate(Delegate *delegate) {
     // Ensure another delegate does not take over unless the current
     // delegate first unattaches itself. If we ever need to multicast
public:
 
   // isSSA - Returns true when the machine function is in SSA form. Early
   // passes require the machine function to be in SSA form where every virtual
   // register has a single defining instruction.
   //
   // The TwoAddressInstructionPass and PHIElimination passes take the machine
   // function out of SSA form when they introduce multiple defs per virtual
   // register.
-  bool isSSA() const { return IsSSA; }
+  bool isSSA() const {
+    return MF->getProperties().hasProperty(
+        MachineFunctionProperties::Property::IsSSA);
+  }
 
   // leaveSSA - Indicates that the machine function is no longer in SSA form.
-  void leaveSSA() { IsSSA = false; }
+  void leaveSSA() {
+    MF->getProperties().clear(MachineFunctionProperties::Property::IsSSA);
+  }
 
   /// tracksLiveness - Returns true when tracking register liveness accurately.
   ///
   /// While this flag is true, register liveness information in basic block
   /// live-in lists and machine instruction operands is accurate. This means it
   /// can be used to change the code in ways that affect the values in
   /// registers, for example by the register scavenger.
   ///

llvm/trunk/lib/CodeGen/MachineFunction.cpp

AlignAllFunctions("align-all-functions",
                       cl::desc("Force the alignment of all functions."),
                       cl::init(0), cl::Hidden);
 
 void MachineFunctionInitializer::anchor() {}
 
 void MachineFunctionProperties::print(raw_ostream &ROS) const {
   // Leave this function even in NDEBUG as an out-of-line anchor.
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-  if (!Properties.any()) {
-    ROS << "(empty)";
-    return;
-  }
   for (BitVector::size_type i = 0; i < Properties.size(); ++i) {
-    if (Properties[i]) {
-      switch(static_cast<Property>(i)) {
-        case Property::AllVRegsAllocated:
-          ROS << "AllVRegsAllocated ";
-          break;
-        default:
-          // TODO: Implement IsSSA/TracksLiveness when we make them properties.
-          llvm_unreachable("Unexpected value for property enum");
-      }
+    bool HasProperty = Properties[i];
+      case Property::IsSSA:
+        ROS << (HasProperty ? "SSA, " : "Post SSA, ");
+        break;
+        ROS << (HasProperty ? "AllVRegsAllocated" : "HasVRegs");
+        break;
     }
   }
 #endif
 }
 
 //===----------------------------------------------------------------------===//
 // MachineFunction implementation
 //===----------------------------------------------------------------------===//
 
 // Out-of-line virtual method.
 MachineFunctionInfo::~MachineFunctionInfo() {}
 
 void ilist_traits<MachineBasicBlock>::deleteNode(MachineBasicBlock *MBB) {
   MBB->getParent()->DeleteMachineBasicBlock(MBB);
 }
 
 MachineFunction::MachineFunction(const Function *F, const TargetMachine &TM,
                                  unsigned FunctionNum, MachineModuleInfo &mmi)
     : Fn(F), Target(TM), STI(TM.getSubtargetImpl(*F)), Ctx(mmi.getContext()),
       MMI(mmi) {
+  // Assume the function starts in SSA form.
+  Properties.set(MachineFunctionProperties::Property::IsSSA);
   if (STI->getRegisterInfo())
     RegInfo = new (Allocator) MachineRegisterInfo(this);
   else
     RegInfo = nullptr;
 
   MFInfo = nullptr;
   FrameInfo = new (Allocator)
       MachineFrameInfo(STI->getFrameLowering()->getStackAlignment(),

 }
 
 void MachineFunction::print(raw_ostream &OS, SlotIndexes *Indexes) const {
   OS << "# Machine code for function " << getName() << ": ";
   OS << "Properties: <";
   getProperties().print(OS);
   OS << "> : ";
   if (RegInfo) {
-    OS << (RegInfo->isSSA() ? "SSA" : "Post SSA");
     if (!RegInfo->tracksLiveness())
-      OS << ", not tracking liveness";
+      OS << "not tracking liveness";
   }
   OS << '\n';
 
   // Print Frame Information
   FrameInfo->print(*this, OS);
 
   // Print JumpTable Information
   if (JumpTableInfo)

llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp

 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 
 using namespace llvm;
 
 // Pin the vtable to this file.
 void MachineRegisterInfo::Delegate::anchor() {}
 
-MachineRegisterInfo::MachineRegisterInfo(const MachineFunction *MF)
-  : MF(MF), TheDelegate(nullptr), IsSSA(true), TracksLiveness(true),
+MachineRegisterInfo::MachineRegisterInfo(MachineFunction *MF)
+  : MF(MF), TheDelegate(nullptr), TracksLiveness(true),
     TracksSubRegLiveness(false) {
   unsigned NumRegs = getTargetRegisterInfo()->getNumRegs();
   VRegInfo.reserve(256);
   RegAllocHints.reserve(256);
   UsedPhysRegMask.resize(NumRegs);
   PhysRegUseDefLists.reset(new MachineOperand*[NumRegs]());
 }