[llvm] r286289 - GlobalISel: allow CodeGen to fallback on VReg type/class issues.

Tim Northover via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 8 12:39:04 PST 2016


Author: tnorthover
Date: Tue Nov  8 14:39:03 2016
New Revision: 286289

URL: http://llvm.org/viewvc/llvm-project?rev=286289&view=rev
Log:
GlobalISel: allow CodeGen to fallback on VReg type/class issues.

After instruction selection we perform some checks on each VReg just before
discarding the type information. These checks were assertions before, but that
breaks the fallback path so this patch moves the logic into the main flow and
reports a better error on failure.

Modified:
    llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
    llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
    llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp

Modified: llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h?rev=286289&r1=286288&r2=286289&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h Tue Nov  8 14:39:03 2016
@@ -109,14 +109,6 @@ private:
   /// Map generic virtual registers to their actual size.
   mutable std::unique_ptr<VRegToTypeMap> VRegToType;
 
-  /// Accessor for VRegToType. This accessor should only be used
-  /// by global-isel related work.
-  VRegToTypeMap &getVRegToType() const {
-    if (!VRegToType)
-      VRegToType.reset(new VRegToTypeMap);
-    return *VRegToType.get();
-  }
-
   /// Keep track of the physical registers that are live in to the function.
   /// Live in values are typically arguments in registers.  LiveIn values are
   /// allowed to have virtual registers associated with them, stored in the
@@ -642,6 +634,14 @@ public:
   ///
   unsigned createVirtualRegister(const TargetRegisterClass *RegClass);
 
+  /// Accessor for VRegToType. This accessor should only be used
+  /// by global-isel related work.
+  VRegToTypeMap &getVRegToType() const {
+    if (!VRegToType)
+      VRegToType.reset(new VRegToTypeMap);
+    return *VRegToType.get();
+  }
+
   /// Get the low-level type of \p VReg or LLT{} if VReg is not a generic
   /// (target independent) virtual register.
   LLT getType(unsigned VReg) const;
@@ -654,7 +654,8 @@ public:
   unsigned createGenericVirtualRegister(LLT Ty);
 
   /// Remove all types associated to virtual registers (after instruction
-  /// selection and constraining of all generic virtual registers).
+  /// selection and constraining of all generic virtual registers). Returns true
+  /// if the VReg mapping was consistent.
   void clearVirtRegTypes();
 
   /// Creates a new virtual register that has no register class, register bank

Modified: llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp?rev=286289&r1=286288&r2=286289&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp Tue Nov  8 14:39:03 2016
@@ -44,11 +44,13 @@ void InstructionSelect::getAnalysisUsage
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-static void reportSelectionError(const MachineInstr &MI, const Twine &Message) {
-  const MachineFunction &MF = *MI.getParent()->getParent();
+static void reportSelectionError(const MachineInstr *MI, const Twine &Message) {
+  const MachineFunction &MF = *MI->getParent()->getParent();
   std::string ErrStorage;
   raw_string_ostream Err(ErrStorage);
-  Err << Message << ":\nIn function: " << MF.getName() << '\n' << MI << '\n';
+  Err << Message << ":\nIn function: " << MF.getName() << '\n';
+  if (MI)
+    Err << *MI << '\n';
   report_fatal_error(Err.str());
 }
 
@@ -80,7 +82,7 @@ bool InstructionSelect::runOnMachineFunc
     for (const MachineBasicBlock &MBB : MF)
       for (const MachineInstr &MI : MBB)
         if (isPreISelGenericOpcode(MI.getOpcode()) && !MLI->isLegal(MI, MRI))
-          reportSelectionError(MI, "Instruction is not legal");
+          reportSelectionError(&MI, "Instruction is not legal");
 
 #endif
   // FIXME: We could introduce new blocks and will need to fix the outer loop.
@@ -113,7 +115,10 @@ bool InstructionSelect::runOnMachineFunc
 
       if (!ISel->select(MI)) {
         if (TPC.isGlobalISelAbortEnabled())
-          reportSelectionError(MI, "Cannot select");
+          // FIXME: It would be nice to dump all inserted instructions.  It's
+          // not
+          // obvious how, esp. considering select() can insert after MI.
+          reportSelectionError(&MI, "Cannot select");
         Failed = true;
         break;
       }
@@ -129,21 +134,40 @@ bool InstructionSelect::runOnMachineFunc
     }
   }
 
+  // Now that selection is complete, there are no more generic vregs.  Verify
+  // that the size of the now-constrained vreg is unchanged and that it has a
+  // register class.
+  for (auto &VRegToType : MRI.getVRegToType()) {
+    unsigned VReg = VRegToType.first;
+    auto *RC = MRI.getRegClassOrNull(VReg);
+    auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()
+                   ? nullptr
+                   : &*MRI.def_instr_begin(VReg);
+    if (!RC) {
+      if (TPC.isGlobalISelAbortEnabled())
+        reportSelectionError(MI, "VReg as no regclass after selection");
+      Failed = true;
+      break;
+    }
+
+    if (VRegToType.second.isValid() &&
+        VRegToType.second.getSizeInBits() > (RC->getSize() * 8)) {
+      if (TPC.isGlobalISelAbortEnabled())
+        reportSelectionError(
+            MI, "VReg has explicit size different from class size");
+      Failed = true;
+      break;
+    }
+  }
+
+  MRI.getVRegToType().clear();
+
   if (!TPC.isGlobalISelAbortEnabled() && (Failed || MF.size() == NumBlocks)) {
     MF.getProperties().set(MachineFunctionProperties::Property::FailedISel);
     return false;
   }
   assert(MF.size() == NumBlocks && "Inserting blocks is not supported yet");
 
-  // Now that selection is complete, there are no more generic vregs.
-  // FIXME: We're still discussing what to do with the vreg->size map:
-  // it's somewhat redundant (with the def MIs type size), but having to
-  // examine MIs is also awkward.  Another alternative is to track the type on
-  // the vreg instead, but that's not ideal either, because it's saying that
-  // vregs have types, which they really don't. But then again, LLT is just
-  // a size and a "shape": it's probably the same information as regbank info.
-  MF.getRegInfo().clearVirtRegTypes();
-
   // FIXME: Should we accurately track changes?
   return true;
 }

Modified: llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp?rev=286289&r1=286288&r2=286289&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp Tue Nov  8 14:39:03 2016
@@ -143,17 +143,6 @@ MachineRegisterInfo::createGenericVirtua
 }
 
 void MachineRegisterInfo::clearVirtRegTypes() {
-#ifndef NDEBUG
-  // Verify that the size of the now-constrained vreg is unchanged.
-  for (auto &VRegToType : getVRegToType()) {
-    auto *RC = getRegClass(VRegToType.first);
-    if (VRegToType.second.isValid() &&
-        VRegToType.second.getSizeInBits() > (RC->getSize() * 8))
-      llvm_unreachable(
-          "Virtual register has explicit size different from its class size");
-  }
-#endif
-
   getVRegToType().clear();
 }
 




More information about the llvm-commits mailing list