[mlir][DialectConversion] Abort early if a subregion has a disconnected CFG.

[matthiaskramm]

Summary

Make computeConversionSet bubble up errors from nested regions. Note
that this doesn't change top-level behavior - since the nested region
calls emitError, the error was visible before, just not surfaced as
quickly.

[rriddle]

Can you please make sure to add a [mlir] tag to the beginning of the commit title?

mlir/lib/Transforms/DialectConversion.cpp

55

nit: Just inline this variable.

[matthiaskramm]

mlir/lib/Transforms/DialectConversion.cpp

55

Good idea, but I can't- it's also used in the return.

[matthiaskramm]

mlir/lib/Transforms/DialectConversion.cpp

55

Nm, I see what you mean now. Changing.

[matthiaskramm]

Wrapped the failed() directly around the recursive call.

[rriddle]

Do you need me to land this for you?

[matthiaskramm]

Yes please. Thank you!

mlir/lib/Transforms/DialectConversion.cpp

for (Operation &op : *block) {
       toConvert.emplace_back(&op);
 
       // Don't check this operation's children for conversion if the operation
       // is recursively legal.
       auto legalityInfo = target ? target->isLegal(&op)
                                  : Optional<ConversionTarget::LegalOpDetails>();
       if (legalityInfo && legalityInfo->isRecursivelyLegal)
         continue;
-      for (auto &region : op.getRegions())
-        computeConversionSet(region.getBlocks(), region.getLoc(), toConvert,
-                             target);
+      for (auto &region : op.getRegions()) {
+        if (failed(computeConversionSet(region.getBlocks(), region.getLoc(),
+                                        toConvert, target)))
+          return failure();
+      }
     }
 
     // Recurse to children that haven't been visited.
     for (Block *succ : block->getSuccessors())
       if (visitedBlocks.insert(succ).second)
         worklist.push_back(succ);
   }