GlobalISel: Fix unconditional fallback with global isel abort is disabled

[t.p.northover]

Should that condition even be there? It seems like a selector may well want to add extra blocks and that's probably one of the benefits of GlobalISel.

[• tstellarAMD]

In D26765#597858, @t.p.northover wrote:

Should that condition even be there? It seems like a selector may well want to add extra blocks and that's probably one of the benefits of GlobalISel.

There assertion right afterwards says that inserting blocks is not supported, so if that's true then I think we need to keep it.

[ab]

I'm fine with removing the restriction, but we need to decide whether to select the new block. If so, the MF iteration will need to be fixed. If not, let's make it explicit and verify it.

For now, the patch LGTM.

[qcolombet]

I'm fine with removing the restriction, but we need to decide whether to select the new block. If so, the MF iteration will need to be fixed. If not, let's make it explicit and verify it.

It could be a work list and the target decide to push the new block into it or not.

I would rather remove this limitation but given we don't have a use case/test case, for now let us keep it.

llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp

if (VRegToType.second.isValid() &&
             MI, "VReg has explicit size different from class size");
       Failed = true;
       break;
     }
   }
 
   MRI.getVRegToType().clear();
 
-  if (!TPC.isGlobalISelAbortEnabled() && (Failed || MF.size() == NumBlocks)) {
+  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");
 
   // FIXME: Should we accurately track changes?
   return true;
 }

llvm/trunk/test/CodeGen/AArch64/GlobalISel/gisel-abort.ll

+; RUN: llc -march aarch64 -global-isel -global-isel-abort=2 -verify-machineinstrs %s -o - 2>&1 | FileCheck %s
+
+; CHECK-NOT: fallback
+; CHECK: empty
+define void @empty() {
+  ret void
+}
+