FullLoopUnroll was performing runtime unrolling in certain cases when
'#pragma unroll' was specified. Patch to fix this by introducing new parameter
to tryToUnrollLoop() to differentiate between LoopUnrollPass and
FullLoopUnrollPass. Based on the discussion here
(https://discourse.llvm.org/t/loop-unroller-fails-to-unroll-loop/69834)
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LLVM :: Transforms/LoopUnroll/full-unroll-keep-first-exit.ll LLVM :: Transforms/LoopUnroll/revisit.ll
These are the 2 tests that I think should be updated.
llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp | ||
---|---|---|
1127 | isFullUnroll -> OnlyFullUnroll? | |
1309 | This probably isn't quite the right condition. I can think of two problematic cases here:
I believe our criterion for what counts as a full unroll is basically "is the unroll count >= max trip count". | |
1311 | parital -> partial |
llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp | ||
---|---|---|
1312 | No reason to break the newline out of the string unless you are going to print additional information before it |
llvm/test/Transforms/LoopUnroll/full-unroll-keep-first-exit.ll | ||
---|---|---|
3 | Can you please restore this check line, possibly with an additional prefix? | |
llvm/test/Transforms/LoopUnroll/revisit.ll | ||
11 | I think you can just drop the option entirely, it no longer makes sense. We can probably assert(!IsCurrentLoopValid) now: Full unrolling should always invalidate the current loop. |
llvm/test/Transforms/LoopUnroll/revisit.ll | ||
---|---|---|
11 | Removed this run line. I can submit another change to entirely drop this cl option. |
LGTM
llvm/test/Transforms/LoopUnroll/full-unroll-keep-first-exit.ll | ||
---|---|---|
436 | Spurious change. Sounds like you can just leave this test alone. |
isFullUnroll -> OnlyFullUnroll?