[mlir] Make fold result type check more verbose

[kuhar]

Print the op and its types when the fold type check fails. This is to speed up debuging as it should be trivial to map the offending op to its folder based on the op name.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Jakub Kuderski (kuhar)

Changes

Print the op and its types when the fold type check fails. This is to speed up debuging as it should be trivial to map the offending op to its folder based on the op name.

---

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

1 Files Affected:

• (modified) mlir/lib/IR/Operation.cpp (+14-5)

diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index a726790391a0c5..a7d5904faa5980 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -20,6 +20,7 @@
 #include "mlir/Interfaces/FoldInterfaces.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <numeric>
 #include <optional>
 
@@ -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");
+      }
+    }
+  }
 }
 #endif // NDEBUG
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Groverkss]

LGTM. This change helped a lot in debugging downstream folders.

[matthias-springer]

nit: You can use llvm_unreachable("incorrect ...");

[kuhar]

I do want the error message to be printed though and that code to be reachable. Probably doesn't matter as long as this code if wrapped in NDEBUG, but 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

[joker-eph]

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:
6316129e066e
(this patch is at the end of the thread you link to).

[joker-eph]

Nice! I am a big proponent of making this kind of error as verbose as needed.