Skip to content

Conversation

raikonenfnu
Copy link
Member

Currently in torch-mlir, indexOp folder is segfaulting when we call createOrFold in a genericOp builder. (*this)->getParentOp ended up with null which is causing the issue.

This is seen in poolSizeCalculator.getPoolSize
being called from createAvgPoolValueCountIncludePadFalseCase. link:
https://github.com/llvm/torch-mlir/blob/80a3dfddd341c72ab9bd6c6688b872bf3a5e4ddb/lib/Conversion/TorchToLinalg/Pooling.cpp#L918-L921

…ateOrFold

Currently in torch-mlir, indexOp folder is segfaulting when we call
createOrFold in a genericOp builder. (*this)->getParentOp ended up with
null which is causing the issue.

This is seen in poolSizeCalculator.getPoolSize
being called from createAvgPoolValueCountIncludePadFalseCase.
link:
https://github.com/llvm/torch-mlir/blob/80a3dfddd341c72ab9bd6c6688b872bf3a5e4ddb/lib/Conversion/TorchToLinalg/Pooling.cpp#L918-L921

Signed-off-by: Stanley Winata <[email protected]>
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2025

@llvm/pr-subscribers-mlir-linalg

Author: Stanley Winata (raikonenfnu)

Changes

Currently in torch-mlir, indexOp folder is segfaulting when we call createOrFold in a genericOp builder. (*this)->getParentOp ended up with null which is causing the issue.

This is seen in poolSizeCalculator.getPoolSize
being called from createAvgPoolValueCountIncludePadFalseCase. link:
https://github.com/llvm/torch-mlir/blob/80a3dfddd341c72ab9bd6c6688b872bf3a5e4ddb/lib/Conversion/TorchToLinalg/Pooling.cpp#L918-L921


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp (+4-1)
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
index 72fb3308a2549..bd2430927fca2 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
@@ -2284,7 +2284,10 @@ LogicalResult IndexOp::verify() {
 }
 
 OpFoldResult IndexOp::fold(FoldAdaptor adaptor) {
-  auto linalgOp = cast<LinalgOp>((*this)->getParentOp());
+  auto linalgOp = dyn_cast_or_null<LinalgOp>((*this)->getParentOp());
+  // Early exit if parent op is not linalgOp.
+  if (!linalgOp)
+    return OpFoldResult{};
 
   // Index of unit dims is always 0.
   SmallVector<int64_t, 4> loopBounds = linalgOp.getStaticLoopRanges();

@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2025

@llvm/pr-subscribers-mlir

Author: Stanley Winata (raikonenfnu)

Changes

Currently in torch-mlir, indexOp folder is segfaulting when we call createOrFold in a genericOp builder. (*this)->getParentOp ended up with null which is causing the issue.

This is seen in poolSizeCalculator.getPoolSize
being called from createAvgPoolValueCountIncludePadFalseCase. link:
https://github.com/llvm/torch-mlir/blob/80a3dfddd341c72ab9bd6c6688b872bf3a5e4ddb/lib/Conversion/TorchToLinalg/Pooling.cpp#L918-L921


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp (+4-1)
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
index 72fb3308a2549..bd2430927fca2 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
@@ -2284,7 +2284,10 @@ LogicalResult IndexOp::verify() {
 }
 
 OpFoldResult IndexOp::fold(FoldAdaptor adaptor) {
-  auto linalgOp = cast<LinalgOp>((*this)->getParentOp());
+  auto linalgOp = dyn_cast_or_null<LinalgOp>((*this)->getParentOp());
+  // Early exit if parent op is not linalgOp.
+  if (!linalgOp)
+    return OpFoldResult{};
 
   // Index of unit dims is always 0.
   SmallVector<int64_t, 4> loopBounds = linalgOp.getStaticLoopRanges();

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! Can we add a test?

@raikonenfnu
Copy link
Member Author

Thanks for fixing this! Can we add a test?

Was thinking about this but can't think of a good test for it, do you have some suggestions? :)

@kuhar
Copy link
Member

kuhar commented Apr 26, 2025

Thanks for fixing this! Can we add a test?

Was thinking about this but can't think of a good test for it, do you have some suggestions? :)

Can we take the IR from troch-mlir that caused the original crash?

@raikonenfnu
Copy link
Member Author

Thanks for fixing this! Can we add a test?

Was thinking about this but can't think of a good test for it, do you have some suggestions? :)

Can we take the IR from troch-mlir that caused the original crash?

Fantastic question! I have a repro IR here https://gist.github.com/raikonenfnu/5f9df8748712fada5b167abd696a13fa. However, it is failing not from an IR level, but rather it fails during a lowering, because it calls into createOrFold here https://github.com/llvm/torch-mlir/blob/80a3dfddd341c72ab9bd6c6688b872bf3a5e4ddb/lib/Conversion/TorchToLinalg/Pooling.cpp#L918-L921

so not so sure if I can recreate that in IR 😅

@kuhar
Copy link
Member

kuhar commented Apr 26, 2025

Oh, I understand, so it's more that linalg.index doesn't have a proper parent yet at that point. In this case, could you clarify this in the code comment above the null check?

@raikonenfnu
Copy link
Member Author

Oh, I understand, so it's more that linalg.index doesn't have a proper parent yet at that point. In this case, could you clarify this in the code comment above the null check?

Exactly! :)

@raikonenfnu
Copy link
Member Author

Oh, I understand, so it's more that linalg.index doesn't have a proper parent yet at that point. In this case, could you clarify this in the code comment above the null check?

done!

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % one suggestion

Copy link

github-actions bot commented Apr 26, 2025

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

Signed-off-by: Stanley Winata <[email protected]>
@raikonenfnu raikonenfnu merged commit 9bf7023 into llvm:main Apr 26, 2025
11 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ateOrFold (llvm#137427)

Currently in torch-mlir, indexOp folder is segfaulting when we call
createOrFold in a genericOp builder. (*this)->getParentOp ended up with
null which is causing the issue.

This is seen in poolSizeCalculator.getPoolSize
being called from createAvgPoolValueCountIncludePadFalseCase. link:

https://github.com/llvm/torch-mlir/blob/80a3dfddd341c72ab9bd6c6688b872bf3a5e4ddb/lib/Conversion/TorchToLinalg/Pooling.cpp#L918-L921

---------

Signed-off-by: Stanley Winata <[email protected]>
Co-authored-by: Jakub Kuderski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants