We should check if loop size allows us to peel at least one iteration before we do so.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
LGTM with a style nit.
lib/Transforms/Utils/LoopUnrollPeel.cpp | ||
---|---|---|
78 ↗ | (On Diff #90641) | I would combine this into a single if with a &&, rather than two nested ones. |
lib/Transforms/Utils/LoopUnrollPeel.cpp | ||
---|---|---|
78 ↗ | (On Diff #90641) | We can't combine it with variable's declaration, and declaring the latch externally looks quite ugly. |
lib/Transforms/Utils/LoopUnrollPeel.cpp | ||
---|---|---|
78 ↗ | (On Diff #90641) | I think having another level of nesting here is worse than declaring the latch externally. |
lib/Transforms/Utils/LoopUnrollPeel.cpp | ||
---|---|---|
78 ↗ | (On Diff #90641) | I just realized that getLoopPatch never returns null, and this is ensured within isLoopSimplifyForm in canPeel method. So I will replace the condition here and make an assert for latch. |
Comment Actions
LGTM.
Also, forgot it before, sorry - please add a test. (No need for a huge test, you can force a small threshold.)