]> granicus.if.org Git - llvm/commitdiff
GlobalISel: allow CodeGen to fallback on VReg type/class issues.
authorTim Northover <tnorthover@apple.com>
Tue, 8 Nov 2016 20:39:03 +0000 (20:39 +0000)
committerTim Northover <tnorthover@apple.com>
Tue, 8 Nov 2016 20:39:03 +0000 (20:39 +0000)
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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286289 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/MachineRegisterInfo.h
lib/CodeGen/GlobalISel/InstructionSelect.cpp
lib/CodeGen/MachineRegisterInfo.cpp

index b669555093c6a4339affe25c4dc11402a8a74da1..4b572791739d3822f0aaae3fb1dda6e2e0408899 100644 (file)
@@ -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
index 6688142699422adb1fbc8e4c13043a37b7a9c534..bd6d7e09091cf5cb1bded55de5731053b99d6da8 100644 (file)
@@ -44,11 +44,13 @@ void InstructionSelect::getAnalysisUsage(AnalysisUsage &AU) const {
   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::runOnMachineFunction(MachineFunction &MF) {
     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::runOnMachineFunction(MachineFunction &MF) {
 
       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::runOnMachineFunction(MachineFunction &MF) {
     }
   }
 
+  // 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;
 }
index 1d6e10acb15ea5895c3a7f85fb89822895ed1674..242cb0b8095366c8d60f02aca7f42485ba7213f6 100644 (file)
@@ -143,17 +143,6 @@ MachineRegisterInfo::createGenericVirtualRegister(LLT Ty) {
 }
 
 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();
 }