[Mlir-commits] [mlir] [mlir] Make fold result type check more verbose (PR #76867)

[Mehdi Amini]

================
@@ -611,11 +612,19 @@ void Operation::setSuccessor(Block *block, unsigned index) {
 /// the results of the given op.
 static void checkFoldResultTypes(Operation *op,
                                  SmallVectorImpl<OpFoldResult> &results) {
-  if (!results.empty())
-    for (auto [ofr, opResult] : llvm::zip_equal(results, op->getResults()))
-      if (auto value = ofr.dyn_cast<Value>())
-        assert(value.getType() == opResult.getType() &&
-               "folder produced value of incorrect type");
+  if (results.empty())
+    return;
+
+  for (auto [ofr, opResult] : llvm::zip_equal(results, op->getResults())) {
+    if (auto value = dyn_cast<Value>(ofr)) {
+      if (value.getType() != opResult.getType()) {
+        llvm::errs() << "Folder produced a value of incorrect type for: " << *op
+                     << "\nOriginal type: '" << value.getType()
+                     << "'\nNew type: '" << opResult.getType() << "'\n";
+        assert(false && "incorrect fold result type");
----------------
joker-eph wrote:

>  I remember there was a long discussion on this at some point that made me hesitant to use unreachable in cases like this one: https://discourse.llvm.org/t/llvm-unreachable-is-widely-misused/60587

In LLVM, I don't see any case for `assert(false`.
I believe we should always use `llvm_unreachable` instead. We already have means to control the behavior:
https://github.com/llvm/llvm-project/commit/6316129e066e
(this patch is at the end of the thread you link to).


https://github.com/llvm/llvm-project/pull/76867