Fix verifier crashing on some invalid IR

[mehdi_amini]

Summary

In a region with multiple blocks the verifier will try to look for
dominance and may get successor list for blocks, even though a block
may be empty or does not end with a terminator.

[rriddle]

mlir/lib/IR/Block.cpp

349–350

block->empty is redundant with the llvm::hasSingleElement check.

[rriddle]

mlir/lib/IR/Block.cpp

349–350

Sorry, meant to drop this comment after I actually looked at the commit (with non-tired eyes).

[GMNGeoffrey]

mlir/test/IR/invalid.mlir

181–187

Wait this is all that was needed to repro? How come I wasn't able to extract mhlo.sort from my repro? I thought it required an op with a region that wasn't isolated from above

[mehdi_amini]

mlir/test/IR/invalid.mlir

181–187

You sent me two bugs, this is the "small" repro.

The larger repro can't be test with opt like that, I would need to write a pass that intentionally create invalid IR and calls the verifier on a nested operation to setup exactly what you have. But the fix here will be enough to cover both test cases.

[GMNGeoffrey]

Thanks!

mlir/test/IR/invalid.mlir

181–187

Yeah I wasn't able to drop the mhlo.sort from even the smaller repro, I thought, but probably I was just doing it wrong :-)

[bondhugula]

Interesting!

mlir/lib/IR/Block.cpp

349–350

Nit: *Might* be better more readable with:

 if (block->empty() || llvm::hasSingleElement(*block->getParent()))
  return;
...

[mehdi_amini]

mlir/lib/IR/Block.cpp

349–350

Done in 066b3207234d

mlir/lib/IR/Block.cpp

 
 //===----------------------------------------------------------------------===//
 // SuccessorRange
 //===----------------------------------------------------------------------===//
 
 SuccessorRange::SuccessorRange() : SuccessorRange(nullptr, 0) {}
 
 SuccessorRange::SuccessorRange(Block *block) : SuccessorRange() {
-  if (!llvm::hasSingleElement(*block->getParent())) {
-    Operation *term = block->getTerminator();
+  if (!block->empty() && !llvm::hasSingleElement(*block->getParent())) {
+    Operation *term = &block->back();
     if ((count = term->getNumSuccessors()))
       base = term->getBlockOperands().data();
   }
 }
 
 SuccessorRange::SuccessorRange(Operation *term) : SuccessorRange() {
   if ((count = term->getNumSuccessors()))
     base = term->getBlockOperands().data();

mlir/test/IR/invalid.mlir

 
 func @no_return() {
   %x = constant 0 : i32
   %y = constant 1 : i32  // expected-error {{block with no terminator}}
 }
 
 // -----
 
+func @no_terminator() {
+  br ^bb1
+^bb1:
+  %x = constant 0 : i32
+  %y = constant 1 : i32  // expected-error {{block with no terminator}}
+}
+
+
+// -----
+
 "       // expected-error {{expected}}
 "
 
 // -----
 
 "       // expected-error {{expected}}
 
 // -----