[Mlir-commits] [mlir] d8804ec - [mlir][Interfaces] Better error handling for `addBound`

[Matthias Springer]

Author: Matthias Springer
Date: 2023-04-11T10:09:52+09:00
New Revision: d8804ecd76db331b622d011f5b72838b25bbf910

URL: https://github.com/llvm/llvm-project/commit/d8804ecd76db331b622d011f5b72838b25bbf910
DIFF: https://github.com/llvm/llvm-project/commit/d8804ecd76db331b622d011f5b72838b25bbf910.diff

LOG: [mlir][Interfaces] Better error handling for `addBound`

Do not assert if a bound could not be added. This may or may not cause an error. We have the appropriate error handling in `computeBound` (which returns a `LogicalResult`).

Differential Revision: https://reviews.llvm.org/D147769

Added: 
    

Modified: 
    mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
    mlir/test/Dialect/Arith/value-bounds-op-interface-impl.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
index aae9f39afc182..639f2136ff5da 100644
--- a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
+++ b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
@@ -11,6 +11,9 @@
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/Matchers.h"
 #include "llvm/ADT/APSInt.h"
+#include "llvm/Support/Debug.h"
+
+#define DEBUG_TYPE "value-bounds-op-interface"
 
 using namespace mlir;
 using presburger::BoundType;
@@ -61,8 +64,14 @@ void ValueBoundsConstraintSet::addBound(BoundType type, int64_t pos,
   LogicalResult status = cstr.addBound(
       type, pos,
       AffineMap::get(cstr.getNumDimVars(), cstr.getNumSymbolVars(), expr));
-  (void)status;
-  assert(succeeded(status) && "failed to add bound to constraint system");
+  if (failed(status)) {
+    // Non-pure (e.g., semi-affine) expressions are not yet supported by
+    // FlatLinearConstraints. However, we can just ignore such failures here.
+    // Even without this bound, there may be enough information in the
+    // constraint system to compute the requested bound. In case this bound is
+    // actually needed, `computeBound` will return `failure`.
+    LLVM_DEBUG(llvm::dbgs() << "Failed to add bound: " << expr << "\n");
+  }
 }
 
 AffineExpr ValueBoundsConstraintSet::getExpr(Value value,

diff  --git a/mlir/test/Dialect/Arith/value-bounds-op-interface-impl.mlir b/mlir/test/Dialect/Arith/value-bounds-op-interface-impl.mlir
index ea44966af271d..18d3bac9da14c 100644
--- a/mlir/test/Dialect/Arith/value-bounds-op-interface-impl.mlir
+++ b/mlir/test/Dialect/Arith/value-bounds-op-interface-impl.mlir
@@ -1,5 +1,5 @@
 // RUN: mlir-opt %s -test-affine-reify-value-bounds -verify-diagnostics \
-// RUN:     -split-input-file | FileCheck %s
+// RUN:     -verify-diagnostics -split-input-file | FileCheck %s
 
 // CHECK: #[[$map:.*]] = affine_map<()[s0] -> (s0 + 5)>
 // CHECK-LABEL: func @arith_addi(
@@ -43,6 +43,16 @@ func.func @arith_muli(%a: index) -> index {
 
 // -----
 
+func.func @arith_muli_non_pure(%a: index, %b: index) -> index {
+  %0 = arith.muli %a, %b : index
+  // Semi-affine expressions (such as "symbol * symbol") are not supported.
+  // expected-error @below{{could not reify bound}}
+  %1 = "test.reify_bound"(%0) : (index) -> (index)
+  return %1 : index
+}
+
+// -----
+
 // CHECK-LABEL: func @arith_const()
 //       CHECK:   %[[c5:.*]] = arith.constant 5 : index
 //       CHECK:   %[[c5:.*]] = arith.constant 5 : index