[PATCH] D104487: [LoopUnroll] Push runtime unrolling decision up into tryToUnrollLoop()
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 17 13:50:32 PDT 2021
nikic added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:391
+ assert((!HasConvergent || !ULO.Runtime) &&
+ "Can't runtime unroll if loop contains a convergent operation.");
});
----------------
I'm not particularly familiar with convergent operations, but looking back at D17526 the concern here seems to have been specifically about the prologue introduced by runtime unrolling, not anything else.
================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp:251
// We use the runtime remainder in cases where we don't know trip multiple
- if (TripMultiple == 1 || TripMultiple % Count != 0) {
+ if (TripMultiple % Count != 0) {
if (!UnrollRuntimeLoopRemainder(L, Count, /*AllowExpensiveTripCount*/ false,
----------------
This change deserves a comment: If TripMultiple is 1 and Count is 1 and UnrollRemainder was set, this tried to unroll the remainder loop with a Count of 0, which is not legal. It wasn't caught previously due to the check
```
// Don't enter the unroll code if there is nothing to do.
if (ULO.TripCount == 0 && ULO.Count < 2) {
```
which happened before the `assert(Count > 0)` assertion. I've removed that check and the assertion is triggered now.
The change here adjust things to not try to create a remainder loop for an unroll count of 1, as this doesn't make sense in the first place.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104487/new/
https://reviews.llvm.org/D104487
More information about the llvm-commits
mailing list