[PATCH] D26648: Clarify semantic of reserved registers.

[Matthias Braun via llvm-commits]

MatzeB updated this revision to Diff 77915.
MatzeB marked an inline comment as done.
MatzeB added a comment.

Update comment, include the "at least 1 subregister must be reserved rule".

For the next version: I will add the check for the 1 subregister reserved rule; I'll move the checking from the machine verifier into MachineRegisterInfo::freezeReserved() (under #ifndef NDEBUG of course).


Repository:
  rL LLVM

https://reviews.llvm.org/D26648

Files:
  include/llvm/Target/TargetRegisterInfo.h
  lib/CodeGen/MachineVerifier.cpp


Index: lib/CodeGen/MachineVerifier.cpp
===================================================================
--- lib/CodeGen/MachineVerifier.cpp
+++ lib/CodeGen/MachineVerifier.cpp
@@ -221,7 +221,7 @@
     void report_context(SlotIndex Pos) const;
     void report_context_liverange(const LiveRange &LR) const;
     void report_context_lanemask(LaneBitmask LaneMask) const;
-    void report_context_vreg(unsigned VReg) const;
+    void report_context_reg(unsigned Reg) const;
     void report_context_vreg_regunit(unsigned VRegOrRegUnit) const;
 
     void verifyInlineAsm(const MachineInstr *MI);
@@ -497,13 +497,13 @@
   errs() << "- liverange:   " << LR << '\n';
 }
 
-void MachineVerifier::report_context_vreg(unsigned VReg) const {
-  errs() << "- v. register: " << PrintReg(VReg, TRI) << '\n';
+void MachineVerifier::report_context_reg(unsigned Reg) const {
+  errs() << "- register:    " << PrintReg(Reg, TRI) << '\n';
 }
 
 void MachineVerifier::report_context_vreg_regunit(unsigned VRegOrUnit) const {
   if (TargetRegisterInfo::isVirtualRegister(VRegOrUnit)) {
-    report_context_vreg(VRegOrUnit);
+    report_context_reg(VRegOrUnit);
   } else {
     errs() << "- regunit:     " << PrintRegUnit(VRegOrUnit, TRI) << '\n';
   }
@@ -527,13 +527,23 @@
   lastIndex = SlotIndex();
   regsReserved = MRI->getReservedRegs();
 
-  // A sub-register of a reserved register is also reserved
+  // Check that all super registers of reserved regs are reserved as well.
+  BitVector Checked(TRI->getNumRegs());
   for (int Reg = regsReserved.find_first(); Reg>=0;
        Reg = regsReserved.find_next(Reg)) {
-    for (MCSubRegIterator SubRegs(Reg, TRI); SubRegs.isValid(); ++SubRegs) {
-      // FIXME: This should probably be:
-      // assert(regsReserved.test(*SubRegs) && "Non-reserved sub-register");
-      regsReserved.set(*SubRegs);
+    if (Checked[Reg])
+      continue;
+    for (MCSuperRegIterator SR(Reg, TRI); SR.isValid(); ++SR) {
+      if (!regsReserved[*SR]) {
+        report("Super register of reserved register is not reserved.", MF);
+        report_context_reg(Reg);
+        errs() << "Super register " << PrintReg(*SR, TRI) << '\n';
+        regsReserved.set(*SR);
+      }
+
+      // We transitively check superregs. So we can remember this for later
+      // to avoid compiletime explosion in deep register hierarchies.
+      Checked.set(*SR);
     }
   }
 
@@ -1574,7 +1584,7 @@
          I = MInfo.vregsRequired.begin(), E = MInfo.vregsRequired.end(); I != E;
          ++I) {
       report("Virtual register defs don't dominate all uses.", MF);
-      report_context_vreg(*I);
+      report_context_reg(*I);
     }
   }
 
Index: include/llvm/Target/TargetRegisterInfo.h
===================================================================
--- include/llvm/Target/TargetRegisterInfo.h
+++ include/llvm/Target/TargetRegisterInfo.h
@@ -486,8 +486,14 @@
 
   /// Returns a bitset indexed by physical register number indicating if a
   /// register is a special register that has particular uses and should be
-  /// considered unavailable at all times, e.g. SP, RA. This is
-  /// used by register scavenger to determine what registers are free.
+  /// considered unavailable at all times, e.g. stack pointer, return address.
+  /// A reserved register:
+  /// - is not allocatable
+  /// - is considered always live
+  /// - is ignored by liveness tracking
+  /// - super registers must be reserved
+  /// - if there are subregisters at least one of them must be reserved, the
+  ///   others may be non-reserved
   virtual BitVector getReservedRegs(const MachineFunction &MF) const = 0;
 
   /// Returns true if PhysReg is unallocatable and constant throughout the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26648.77915.patch
Type: text/x-patch
Size: 3693 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161115/307c11e5/attachment.bin>