[MLIR][Affine] Fix crash with invalid unroll factor (Issue #64321) #7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please use an appropriate commit title: instead of "Solved ...", use "[MLIR][Affine] Fix crash with an invalid unroll factor", etc. |
SwapnilGhanshyala
force-pushed
the
issue_64321
branch
from
fc21dd7 to
7011834
Compare
SwapnilGhanshyala
changed the title
Author
|
Updated the commit message and the commit summary. |
SwapnilGhanshyala
pushed a commit
that referenced
this pull request
Oct 4, 2023
…fine.parallel verifier
This patch updates AffineParallelOp::verify() to check each result type matches
its corresponding reduction op (i.e, the result type must be a `FloatType` if
the reduction attribute is `addf`)
affine.parallel will crash on --lower-affine if the corresponding result type
cannot match the reduction attribute.
```
%128 = affine.parallel (%arg2, %arg3) = (0, 0) to (8, 7) reduce ("maxf") -> (memref<8x7xf32>) {
%alloc_33 = memref.alloc() : memref<8x7xf32>
affine.yield %alloc_33 : memref<8x7xf32>
}
```
This will crash and report a type conversion issue when we run `mlir-opt --lower-affine`
```
Assertion failed: (isa<To>(Val) && "cast<Ty>() argument of incompatible type!"), function cast, file Casting.h, line 572.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: mlir-opt --lower-affine temp.mlir
#0 0x0000000102a18f18 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/workspacebin/mlir-opt+0x1002f8f18)
#1 0x0000000102a171b4 llvm::sys::RunSignalHandlers() (/workspacebin/mlir-opt+0x1002f71b4)
#2 0x0000000102a195c4 SignalHandler(int) (/workspacebin/mlir-opt+0x1002f95c4)
#3 0x00000001be7894c4 (/usr/lib/system/libsystem_platform.dylib+0x1803414c4)
#4 0x00000001be771ee0 (/usr/lib/system/libsystem_pthread.dylib+0x180329ee0)
#5 0x00000001be6ac340 (/usr/lib/system/libsystem_c.dylib+0x180264340)
#6 0x00000001be6ab754 (/usr/lib/system/libsystem_c.dylib+0x180263754)
#7 0x0000000106864790 mlir::arith::getIdentityValueAttr(mlir::arith::AtomicRMWKind, mlir::Type, mlir::OpBuilder&, mlir::Location) (.cold.4) (/workspacebin/mlir-opt+0x104144790)
#8 0x0000000102ba66ac mlir::arith::getIdentityValueAttr(mlir::arith::AtomicRMWKind, mlir::Type, mlir::OpBuilder&, mlir::Location) (/workspacebin/mlir-opt+0x1004866ac)
#9 0x0000000102ba6910 mlir::arith::getIdentityValue(mlir::arith::AtomicRMWKind, mlir::Type, mlir::OpBuilder&, mlir::Location) (/workspacebin/mlir-opt+0x100486910)
...
```
Fixes llvm#64068
Reviewed By: mehdi_amini
Differential Revision: https://reviews.llvm.org/D157985
bondhugula
requested changes
Oct 6, 2023
bondhugula
left a comment
bondhugula
left a comment
There was a problem hiding this comment.
Commit titles shouldn't have full stops - please drop it.
There was a problem hiding this comment.
You don't need this include I think - it's transitively covered.
Comment on lines
1019
to
1025
There was a problem hiding this comment.
The assert is fine. The utility shouldn't typically emit a message in this situation. The 0 unroll factor should be checked by the user of this utility, which would be the pass, given the crash issue you are addressing. Please put the check and emit message in the pass.
SwapnilGhanshyala
force-pushed
the
issue_64321
branch
from
7011834 to
9b6d0aa
Compare
Updated LoopUnroll pass to check if unrollFactor passed by the user is invalid, i.e., less than or equal to zero. If so, emit appropriate message and signal pass failure. Reporting Issue is llvm#64321
SwapnilGhanshyala
force-pushed
the
issue_64321
branch
from
9b6d0aa to
09397d5
Compare
Author
|
Changes Addressed:
Updated LoopUnroll pass to check if unrollFactor passed by the user is invalid, i.e., less than or equal to Reporting Issue is llvm#64321 |
SwapnilGhanshyala
changed the title
SwapnilGhanshyala
pushed a commit
that referenced
this pull request
Jan 22, 2024
…8055) This fixes a crash where `path::parent_path` causes an invalid access on a string upon receiving a path that consists of a single colon. On Windows machine, with runtime checks enabled build, upon `clang -I: test.cc` produces: ``` Assertion failed: Index < Length && "Invalid index!", file llvm\include\llvm/ADT/StringRef.h, line 232 ... #6 0x00007ff7816201eb `anonymous namespace'::parent_path_end llvm\lib\Support\Path.cpp:144:0 #7 0x00007ff781620135 llvm::sys::path::parent_path(class llvm::StringRef, enum llvm::sys::path::Style) llvm\lib\Support\Path.cpp:470:0 ``` Ideally, we can look for the last colon starting from the last character, but we can instead start from second to last, and handle empty paths by abusing `0 - 1 == npos`.
bondhugula
reviewed
Mar 9, 2025
| if (func.isExternal()) | ||
| return; | ||
|
|
||
| if (unrollFactor <= 0) { |
There was a problem hiding this comment.
This is an unsigned. == 0 would be the right check.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.