| Issue |
147096
|
| Summary |
[mlir] Bugs in `loop-invariant-subset-hoisting` implementation
|
| Labels |
mlir
|
| Assignees |
|
| Reporter |
christopherbate
|
I was reviewing existing implementation of `loop-invariant-subset-hoisting` and founds a couple issues. They are related to the [last change](https://github.com/llvm/llvm-project/pull/70623) from ~2years ago dealing with nested loops:
- In [this function](https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L242), the `collectHoistableOps` parameter is not used. This means that the if you enumerate LoopLikeOps in pre-order and apply the transformation `mlir::hoistLoopInvariantSubsets`, you can produce invalid IR since it may try to hoist args not belonging to the current loop at [this point](https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L332).
- When checking use-def chain in the body of the loop and there is a nested loop user in the use-def chain, at [this point](https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp#L268) there is no `if(nextValue) return failure()` like in the insertion user branch, which looks very suspicious. I wasn't able to create a failing test case. However since user order is not deterministic, it doesn't make sense for the check to be omitted.
![]()
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
