This patch extends the optimization of truncations whose operand is an induction variable with a constant integer step. Previously we were only applying this optimization to the primary induction variable. However, the cost model assumes the optimization is applied to the truncation of all integer induction variables (even regardless of step type). The transform is now applied to the other induction variables, and I've updated the cost model to ensure it is better in sync with the transformation we actually perform.
Details
Details
- Reviewers
mkuper - Commits
- rGf09d13e5cc47: Reapply "[LV] Extend trunc optimization to all IVs with constant integer steps"
rG7b7f40297f23: [LV] Extend trunc optimization to all IVs with constant integer steps
rL295063: Reapply "[LV] Extend trunc optimization to all IVs with constant integer steps"
rL294967: [LV] Extend trunc optimization to all IVs with constant integer steps
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
I'm reopening this review since I reverted the original patch. It caused execution time slowdowns in a few test-suite programs that were caught by the clang-cmake-aarch64-quick bot. It looks like the cause is related to the fact that we're not cost-modeling this optimization very well. Some truncations are free - if we replace those truncations with an induction variable, we'll end up with an extra instruction (the IV update) on every iteration of the loop. I'll post an updated patch shortly.
Comment Actions
- I updated patch so that we extend the truncate optimization to non-primary induction variables only if the truncate isn't already free.
- I factored out the logic into a helper so we can keep things in sync easier.
- I added a test case for AArch64 showing that we keep a free truncate.
Comment Actions
Heh, interesting.
Anyway, LGTM.
lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
2037 ↗ | (On Diff #88256) | Unrelated, but after this lands, I think I'll change the name to getPrimaryInduction() or getPrimaryIV(). This is a really bad name at this point. |
lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
2037 ↗ | (On Diff #88256) | I agree. Thanks Michael. |