<div dir="ltr">This breaks with -Werror, because there are a couple of variables that aren't properly constrained by debugging.<div><br></div><div>It's not clear to me what the correct fix is, but I've just sent https://reviews.llvm.org/D26421 with what looks to me to be the right fix.</div><div><br><div><div><br></div><div><div>FAILED: lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o </div><div>/usr/local/google/home/dlj/src/llvm/toolchain/bin/clang++   -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/CodeGen/GlobalISel -I/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel -Iinclude -I/usr/local/google/home/dlj/src/llvm/llvm/include -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Werror -Werror=date-time -std=c++11 -fcolor-diagnostics -ffunction-sections -fdata-sections -O3 -DNDEBUG    -fno-exceptions -fno-rtti -MD -MT lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o -MF lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o.d -o lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o -c /usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:105:20: error: unused variable 'AfterIt' [-Werror,-Wunused-variable]</div><div>        const auto AfterIt = std::next(MII);</div><div>                   ^</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:141:27: error: use of undeclared identifier 'MRI'</div><div>  for (auto &VRegToType : MRI.getVRegToType()) {</div><div>                          ^</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:143:16: error: use of undeclared identifier 'MRI'</div><div>    auto *RC = MRI.getRegClassOrNull(VReg);</div><div>               ^</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:144:16: error: use of undeclared identifier 'MRI'</div><div>    auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()</div><div>               ^</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:144:45: error: use of undeclared identifier 'MRI'</div><div>    auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()</div><div>                                            ^</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:146:24: error: use of undeclared identifier 'MRI'</div><div>                   : &*MRI.def_instr_begin(VReg);</div><div>                       ^</div><div>/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:164:3: error: use of undeclared identifier 'MRI'</div><div>  MRI.getVRegToType().clear();</div><div>  ^</div><div>7 errors generated.</div><div><br></div><br>On Tuesday, November 8, 2016 at 12:48:53 PM UTC-8, Tim Northover via llvm-commits wrote:<blockquote class="gmail_quote" style="margin: 0;margin-left: 0.8ex;border-left: 1px #ccc solid;padding-left: 1ex;">Author: tnorthover<br>Date: Tue Nov  8 14:39:03 2016<br>New Revision: 286289<p>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=286289&view=rev" target="_blank" rel="nofollow" onmousedown="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D286289%26view%3Drev\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGoSQ7-As98CY8eEizkooqxzeTwXw';return true;" onclick="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D286289%26view%3Drev\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGoSQ7-As98CY8eEizkooqxzeTwXw';return true;" class="cremed">http://llvm.org/viewvc/llvm-<wbr>project?rev=286289&view=rev</a><br>Log:<br>GlobalISel: allow CodeGen to fallback on VReg type/class issues.</p><p>After instruction selection we perform some checks on each VReg just before<br>discarding the type information. These checks were assertions before, but that<br>breaks the fallback path so this patch moves the logic into the main flow and<br>reports a better error on failure.</p><p>Modified:<br>    llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h<br>    llvm/trunk/lib/CodeGen/<wbr>GlobalISel/InstructionSelect.<wbr>cpp<br>    llvm/trunk/lib/CodeGen/<wbr>MachineRegisterInfo.cpp</p><p>Modified: llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h?rev=286289&r1=286288&r2=286289&view=diff" target="_blank" rel="nofollow" onmousedown="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Finclude%2Fllvm%2FCodeGen%2FMachineRegisterInfo.h%3Frev%3D286289%26r1%3D286288%26r2%3D286289%26view%3Ddiff\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG3p_tRCOMFT3uC8a6hqY-GpqMJ7g';return true;" onclick="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Finclude%2Fllvm%2FCodeGen%2FMachineRegisterInfo.h%3Frev%3D286289%26r1%3D286288%26r2%3D286289%26view%3Ddiff\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG3p_tRCOMFT3uC8a6hqY-GpqMJ7g';return true;" class="cremed">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/CodeGen/<wbr>MachineRegisterInfo.h?rev=<wbr>286289&r1=286288&r2=286289&<wbr>view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h (original)<br>+++ llvm/trunk/include/llvm/<wbr>CodeGen/MachineRegisterInfo.h Tue Nov  8 14:39:03 2016<br>@@ -109,14 +109,6 @@ private:<br>   /// Map generic virtual registers to their actual size.<br>   mutable std::unique_ptr<VRegToTypeMap> VRegToType;<br> <br>-  /// Accessor for VRegToType. This accessor should only be used<br>-  /// by global-isel related work.<br>-  VRegToTypeMap &getVRegToType() const {<br>-    if (!VRegToType)<br>-      VRegToType.reset(new VRegToTypeMap);<br>-    return *VRegToType.get();<br>-  }<br>-<br>   /// Keep track of the physical registers that are live in to the function.<br>   /// Live in values are typically arguments in registers.  LiveIn values are<br>   /// allowed to have virtual registers associated with them, stored in the<br>@@ -642,6 +634,14 @@ public:<br>   ///<br>   unsigned createVirtualRegister(const TargetRegisterClass *RegClass);<br> <br>+  /// Accessor for VRegToType. This accessor should only be used<br>+  /// by global-isel related work.<br>+  VRegToTypeMap &getVRegToType() const {<br>+    if (!VRegToType)<br>+      VRegToType.reset(new VRegToTypeMap);<br>+    return *VRegToType.get();<br>+  }<br>+<br>   /// Get the low-level type of \p VReg or LLT{} if VReg is not a generic<br>   /// (target independent) virtual register.<br>   LLT getType(unsigned VReg) const;<br>@@ -654,7 +654,8 @@ public:<br>   unsigned createGenericVirtualRegister(<wbr>LLT Ty);<br> <br>   /// Remove all types associated to virtual registers (after instruction<br>-  /// selection and constraining of all generic virtual registers).<br>+  /// selection and constraining of all generic virtual registers). Returns true<br>+  /// if the VReg mapping was consistent.<br>   void clearVirtRegTypes();<br> <br>   /// Creates a new virtual register that has no register class, register bank</p><p>Modified: llvm/trunk/lib/CodeGen/<wbr>GlobalISel/InstructionSelect.<wbr>cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp?rev=286289&r1=286288&r2=286289&view=diff" target="_blank" rel="nofollow" onmousedown="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FGlobalISel%2FInstructionSelect.cpp%3Frev%3D286289%26r1%3D286288%26r2%3D286289%26view%3Ddiff\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNHwpl0D76aaFocjO8RERZ6dXlmZsw';return true;" onclick="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FGlobalISel%2FInstructionSelect.cpp%3Frev%3D286289%26r1%3D286288%26r2%3D286289%26view%3Ddiff\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNHwpl0D76aaFocjO8RERZ6dXlmZsw';return true;" class="cremed">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/GlobalISel/<wbr>InstructionSelect.cpp?rev=<wbr>286289&r1=286288&r2=286289&<wbr>view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/lib/CodeGen/<wbr>GlobalISel/InstructionSelect.<wbr>cpp (original)<br>+++ llvm/trunk/lib/CodeGen/<wbr>GlobalISel/InstructionSelect.<wbr>cpp Tue Nov  8 14:39:03 2016<br>@@ -44,11 +44,13 @@ void InstructionSelect::<wbr>getAnalysisUsage<br>   MachineFunctionPass::<wbr>getAnalysisUsage(AU);<br> }<br> <br>-static void reportSelectionError(const MachineInstr &MI, const Twine &Message) {<br>-  const MachineFunction &MF = *MI.getParent()->getParent();<br>+static void reportSelectionError(const MachineInstr *MI, const Twine &Message) {<br>+  const MachineFunction &MF = *MI->getParent()->getParent();<br>   std::string ErrStorage;<br>   raw_string_ostream Err(ErrStorage);<br>-  Err << Message << ":\nIn function: " << MF.getName() << '\n' << MI << '\n';<br>+  Err << Message << ":\nIn function: " << MF.getName() << '\n';<br>+  if (MI)<br>+    Err << *MI << '\n';<br>   report_fatal_error(Err.str())<wbr>;<br> }<br> <br>@@ -80,7 +82,7 @@ bool InstructionSelect::<wbr>runOnMachineFunc<br>     for (const MachineBasicBlock &MBB : MF)<br>       for (const MachineInstr &MI : MBB)<br>         if (isPreISelGenericOpcode(MI.<wbr>getOpcode()) && !MLI->isLegal(MI, MRI))<br>-          reportSelectionError(MI, "Instruction is not legal");<br>+          reportSelectionError(&MI, "Instruction is not legal");<br> <br> #endif<br>   // FIXME: We could introduce new blocks and will need to fix the outer loop.<br>@@ -113,7 +115,10 @@ bool InstructionSelect::<wbr>runOnMachineFunc<br> <br>       if (!ISel->select(MI)) {<br>         if (TPC.isGlobalISelAbortEnabled(<wbr>))<br>-          reportSelectionError(MI, "Cannot select");<br>+          // FIXME: It would be nice to dump all inserted instructions.  It's<br>+          // not<br>+          // obvious how, esp. considering select() can insert after MI.<br>+          reportSelectionError(&MI, "Cannot select");<br>         Failed = true;<br>         break;<br>       }<br>@@ -129,21 +134,40 @@ bool InstructionSelect::<wbr>runOnMachineFunc<br>     }<br>   }<br> <br>+  // Now that selection is complete, there are no more generic vregs.  Verify<br>+  // that the size of the now-constrained vreg is unchanged and that it has a<br>+  // register class.<br>+  for (auto &VRegToType : MRI.getVRegToType()) {<br>+    unsigned VReg = VRegToType.first;<br>+    auto *RC = MRI.getRegClassOrNull(VReg);<br>+    auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()<br>+                   ? nullptr<br>+                   : &*MRI.def_instr_begin(VReg);<br>+    if (!RC) {<br>+      if (TPC.isGlobalISelAbortEnabled(<wbr>))<br>+        reportSelectionError(MI, "VReg as no regclass after selection");<br>+      Failed = true;<br>+      break;<br>+    }<br>+<br>+    if (VRegToType.second.isValid() &&<br>+        VRegToType.second.<wbr>getSizeInBits() > (RC->getSize() * 8)) {<br>+      if (TPC.isGlobalISelAbortEnabled(<wbr>))<br>+        reportSelectionError(<br>+            MI, "VReg has explicit size different from class size");<br>+      Failed = true;<br>+      break;<br>+    }<br>+  }<br>+<br>+  MRI.getVRegToType().clear();<br>+<br>   if (!TPC.<wbr>isGlobalISelAbortEnabled() && (Failed || MF.size() == NumBlocks)) {<br>     MF.getProperties().set(<wbr>MachineFunctionProperties::<wbr>Property::FailedISel);<br>     return false;<br>   }<br>   assert(MF.size() == NumBlocks && "Inserting blocks is not supported yet");<br> <br>-  // Now that selection is complete, there are no more generic vregs.<br>-  // FIXME: We're still discussing what to do with the vreg->size map:<br>-  // it's somewhat redundant (with the def MIs type size), but having to<br>-  // examine MIs is also awkward.  Another alternative is to track the type on<br>-  // the vreg instead, but that's not ideal either, because it's saying that<br>-  // vregs have types, which they really don't. But then again, LLT is just<br>-  // a size and a "shape": it's probably the same information as regbank info.<br>-  MF.getRegInfo().<wbr>clearVirtRegTypes();<br>-<br>   // FIXME: Should we accurately track changes?<br>   return true;<br> }</p><p>Modified: llvm/trunk/lib/CodeGen/<wbr>MachineRegisterInfo.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp?rev=286289&r1=286288&r2=286289&view=diff" target="_blank" rel="nofollow" onmousedown="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FMachineRegisterInfo.cpp%3Frev%3D286289%26r1%3D286288%26r2%3D286289%26view%3Ddiff\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNElmaTaJeUSnb6CW8djzeK1xaimUg';return true;" onclick="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FMachineRegisterInfo.cpp%3Frev%3D286289%26r1%3D286288%26r2%3D286289%26view%3Ddiff\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNElmaTaJeUSnb6CW8djzeK1xaimUg';return true;" class="cremed">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/MachineRegisterInfo.<wbr>cpp?rev=286289&r1=286288&r2=<wbr>286289&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/lib/CodeGen/<wbr>MachineRegisterInfo.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/<wbr>MachineRegisterInfo.cpp Tue Nov  8 14:39:03 2016<br>@@ -143,17 +143,6 @@ MachineRegisterInfo::<wbr>createGenericVirtua<br> }<br> <br> void MachineRegisterInfo::<wbr>clearVirtRegTypes() {<br>-#ifndef NDEBUG<br>-  // Verify that the size of the now-constrained vreg is unchanged.<br>-  for (auto &VRegToType : getVRegToType()) {<br>-    auto *RC = getRegClass(VRegToType.first);<br>-    if (VRegToType.second.isValid() &&<br>-        VRegToType.second.<wbr>getSizeInBits() > (RC->getSize() * 8))<br>-      llvm_unreachable(<br>-          "Virtual register has explicit size different from its class size");<br>-  }<br>-#endif<br>-<br>   getVRegToType().clear();<br> }<br> </p><p><br>______________________________<wbr>_________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank" rel="nofollow" onmousedown="this.href='mailto:llvm-commits@lists.llvm.org';return true;" onclick="this.href='mailto:llvm-commits@lists.llvm.org';return true;" class="cremed">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank" rel="nofollow" onmousedown="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFQx5it08j_9kXyPppBMYrkOwlE2A';return true;" onclick="this.href='http://www.google.com/url?q\x3dhttp%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFQx5it08j_9kXyPppBMYrkOwlE2A';return true;" class="cremed">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br></p><p></p><p></p><p></p><p></p><p></p><p></p></blockquote></div></div></div></div>