[PATCH] D78788: [MLIR] Modify Analysis op conversion mode to emit errors for all unlegalizable operations.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/lib/Transforms/DialectConversion.cpp:1568
+    if ((mode == OpConversionMode::Partial ||
+         mode == OpConversionMode::Analysis) &&
+        opLegalizer.isIllegal(op))
----------------
Isn't the mode necessarily Partial/Analysis here?


================
Comment at: mlir/lib/Transforms/DialectConversion.cpp:1577
+      /// (i.e. performing a partial conversion).
+      unlegalizableOps->insert(op);
+      op->emitWarning() << "failed to legalize operation '" << op->getName()
----------------
Isn't this something we'd want to track even in "Partial" mode? 

It seems like the difference between Partial and Analysis overall is about whether the conversion is performed or not. Anything else should be uniform.
Partial vs Full is that the latter is aborting early and so does not need to accumulate the unlegalizableOps.


================
Comment at: mlir/lib/Transforms/DialectConversion.cpp:1579
+      op->emitWarning() << "failed to legalize operation '" << op->getName()
+                        << "'";
+      return success();
----------------
Do we need to warn here?
Can we let signaling up to the client since we populate unlegalizableOps?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78788/new/

https://reviews.llvm.org/D78788