Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I think it would be good to extend test coverage by adding tests with 1) multiple inductions, 2) different induction start values (different constants, maybe a function argument), 3) different divisors (including a non-power-of-2 one)
llvm/test/Transforms/LoopVectorize/X86/uniform_across_vf.ll | ||
---|---|---|
2 | Does the test have to be X86 specific or could it be generic? |
OK let me add more tests.
llvm/test/Transforms/LoopVectorize/X86/uniform_across_vf.ll | ||
---|---|---|
2 | Yeah I can move it one directory up. |
Added more tests. Testing all combinations of: start from 0 to 1, step from 1 to 3 and div from 1 to 3.
Added a separate file for the same tests but with 2 inductions.
Do you think I should add a couple of tests with a left-shift too , instead of a div (like i << 1 instead of i / 2 ? Though the SCEV implementation should cover both.
Thanks for adding all the extra tests! For the multi-induction tests, I think it would be good to have the additional iv2 also used in the loop body (besides incrementing it) in some of the tests, as suggested inline.
llvm/test/Transforms/LoopVectorize/uniform_across_vf_induction2.ll | ||
---|---|---|
54 ↗ | (On Diff #515521) | for those tests, it would probably be good to use %iv2 as well, possibly some cases where the index is %iv /C + iv2 / C. |
LGTM, thanks for adding the extensive test coverage. It would be good to extend wit other patterns in the future, like shifts.
Excellent! This should also be handled by D148841 automatically.
llvm/test/Transforms/LoopVectorize/uniform_across_vf_induction2.ll | ||
---|---|---|
1494 ↗ | (On Diff #516441) | nit: stray newline. |
Does the test have to be X86 specific or could it be generic?