[Mlir-commits] [mlir] [mlir][memref] Fix an invalid dim loop motion crash (PR #74204)

Rik Huijzer llvmlistbot at llvm.org
Sun Dec 3 00:56:53 PST 2023


================
@@ -893,8 +893,9 @@ Speculation::Speculatability DimOp::getSpeculatability() {
   if (!rankedSourceType)
     return Speculation::NotSpeculatable;
 
-  // The verifier rejects operations that violate this assertion.
-  assert(constantIndex < rankedSourceType.getRank());
+  if (rankedSourceType.getRank() < constantIndex)
----------------
rikhuijzer wrote:

> Is the comment factually incorrect: `The verifier rejects operations that violate this assertion.`?

The comment is correct. I think what happened is that the idea about this has changed over time, the assertion was added in Oct 2022, but an argument to not crash was made Jan 2023, see https://github.com/llvm/llvm-project/issues/60295. From my interpretation of https://github.com/llvm/llvm-project/issues/60295, the comment is correct but crashing is not. 

> Also, I think the condition should be `rankedSourceType.getRank() <= constantIndex`. Same for the other check.

Well spotted! I've added tests in [53e2bcd](https://github.com/llvm/llvm-project/pull/74204/commits/53e2bcd0cc42b518d45f7d8c71373b3da633ec4a) to ensure that `-loop-invariant-code-motion` optimizes the code when it should.



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


More information about the Mlir-commits mailing list