This is an archive of the discontinued LLVM Phabricator instance.

[LV][NFC] Precommit test for a follow-up patch that introduces uniformity for a specific VF.
ClosedPublic

Authored by vporpo on Apr 6 2023, 12:49 PM.

Diff Detail

Event Timeline

vporpo created this revision.Apr 6 2023, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2023, 12:49 PM
vporpo requested review of this revision.Apr 6 2023, 12:49 PM
fhahn added a subscriber: fhahn.Apr 20 2023, 1:38 PM

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 ↗(On Diff #511504)

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 ↗(On Diff #511504)

Yeah I can move it one directory up.

vporpo updated this revision to Diff 515521.Apr 20 2023, 4:15 PM

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.

*I meant right shifts (i >> 1)

fhahn added a comment.Apr 22 2023, 1:38 PM

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
55

for those tests, it would probably be good to use %iv2 as well, possibly some cases where the index is %iv /C + iv2 / C.

vporpo updated this revision to Diff 516441.Apr 24 2023, 9:28 AM

Use iv2 in the index calculation: iv/C + iv2/C.

fhahn accepted this revision.Apr 25 2023, 1:34 PM

LGTM, thanks for adding the extensive test coverage. It would be good to extend wit other patterns in the future, like shifts.

Use iv2 in the index calculation: iv/C + iv2/C.

Excellent! This should also be handled by D148841 automatically.

llvm/test/Transforms/LoopVectorize/uniform_across_vf_induction2.ll
1495

nit: stray newline.

This revision is now accepted and ready to land.Apr 25 2023, 1:34 PM
This revision was landed with ongoing or failed builds.Apr 25 2023, 2:14 PM
This revision was automatically updated to reflect the committed changes.