[Mlir-commits] [mlir] [MLIR][Tensor] Fix out-of-bounds FoldEmptyTensorWithDimOp crash #111270 (PR #112196)
Mehdi Amini
llvmlistbot at llvm.org
Thu Oct 17 15:57:27 PDT 2024
================
@@ -979,7 +979,9 @@ struct FoldEmptyTensorWithDimOp : public OpRewritePattern<DimOp> {
auto emptyTensorOp = dimOp.getSource().getDefiningOp<EmptyOp>();
if (!emptyTensorOp || !maybeConstantIndex)
return failure();
- if (!emptyTensorOp.getType().isDynamicDim(*maybeConstantIndex))
+ auto emptyTensorType = emptyTensorOp.getType();
----------------
joker-eph wrote:
I don't understand your point actually: this check only applies to places where you would try to **fold** the constant. This shouldn't happen in so many places.
Regardless, this is something common to every op that has UB on specific input values, that happens all the time!
For example you seems like you'd want to make the LLVM verifier reject this IR (which is runtime UB):
```
%cst1 = llvm.constant -1 : i32
%overflow = llvm.add %0, %0 nsw : i32
```
> If you are going to go through larger changes I would first ask on Discourse. As a user of MLIR such changes and "loosening" of verification can have very disruptive effects. I would strongly recommend that we discuss this on discourse first cause such blanket principles without some more nuance on cases-by-case basis is BAD.
Sorry: I won't. If I find verifiers in MLIR that are violating this principle without a very strong exceptional argument (I know of a couple of these out-of-tree that I don't want to get into here), I **will** remove these without any Discourse thread. We have a policy and guideline on this, I am sorry you don't seem to understand the basic principle of the design behind the IR consistency and validity here, I'm happy to explain it to you further, but I won't jump through these hoops you seem to intend to put here in the way of keeping the code consistent from a system design point of view.
https://github.com/llvm/llvm-project/pull/112196
More information about the Mlir-commits
mailing list