[mlir] Don't assert when simplifying certain AffineExprs

[ubfx]

Currently, simplifyMul() asserts that either lhs or rhs is symbolic or constant. This method is called by the overloaded * operator for AffineExprs which leads to a crash when building a multiplication expression where neither operand is symbolic or constant.
This patch returns a nullptr from simplifyMul() to signal that the expression could not be simplified instead.

Fix #75770

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Felix Schneider (ubfx)

Changes

Currently, simplifyMul() asserts that either lhs or rhs is symbolic or constant. This method is called by the overloaded * operator for AffineExprs which leads to a crash when building a multiplication expression where neither operand is symbolic or constant.
This patch returns a nullptr from simplifyMul() to signal that the expression could not be simplified instead.

Fix #75770

---

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

1 Files Affected:

• (modified) mlir/lib/IR/AffineExpr.cpp (+2-1)

diff --git a/mlir/lib/IR/AffineExpr.cpp b/mlir/lib/IR/AffineExpr.cpp
index a90b264a8edd26..77c8fcc96d0ea0 100644
--- a/mlir/lib/IR/AffineExpr.cpp
+++ b/mlir/lib/IR/AffineExpr.cpp
@@ -774,7 +774,8 @@ static AffineExpr simplifyMul(AffineExpr lhs, AffineExpr rhs) {
     return getAffineConstantExpr(lhsConst.getValue() * rhsConst.getValue(),
                                  lhs.getContext());
 
-  assert(lhs.isSymbolicOrConstant() || rhs.isSymbolicOrConstant());
+  if (!lhs.isSymbolicOrConstant() && !rhs.isSymbolicOrConstant())
+    return nullptr;
 
   // Canonicalize the mul expression so that the constant/symbolic term is the
   // RHS. If both the lhs and rhs are symbolic, swap them if the lhs is a

[ftynse]

Is there a way to test this? Perhaps in a unit test?

[ubfx]

@ftynse Like this? Or were you thinking a more thorough test with walking the resulting AffineExpr. Although this might get more difficult because we would have to anticipate all possible rearrangements and simplifications.
This test would have caught the linked issue because simplifyMul() would have crashed on d0*d1

[ftynse]

Good enough. I usually add the minimized test case that triggered the assertion in the first place, this lets us catch an eventual regression.