This is an archive of the discontinued LLVM Phabricator instance.

[LV] Induction Variable does not remain scalar under tail-folding.
ClosedPublic

Authored by SjoerdMeijer on Apr 27 2020, 3:00 AM.

Details

Summary

If tail-folding of the scalar remainder loop is applied, the primary induction variable is splat to a vector and used by the masked load/store vector instructions, thus the IV does not remain scalar.

This is a minimal change extracted from D78353, and is a useful change/fix on its own. This triggers in function @example2 in test case test/Transforms/LoopVectorize/X86/small-size.ll. Please note that there are quite some changes in this file, but that file was partly auto-generated and partly hand-edited, and I've regenerated all expected output.

Diff Detail

Event Timeline

SjoerdMeijer created this revision.Apr 27 2020, 3:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2020, 3:00 AM

Friendly ping, okay as a minimal change and a separate patch?

Ayal added a comment.May 5 2020, 7:28 AM

Thanks for following-up on this!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
1914–1915

Is this call to CreateSplatIV() now fully redundant and can be removed? Along with fixing the above comment. All users should be scalar, vectorized IV should not be required.

4592

Regarding the comment: the primary iv will be widened rather that remaining scalar and being splatted, to be used by a vector compare rather than (which it turn will be) feeding masked vector load/stores directly.

Suffice to say here that // If tail-folding is applied, the primary induction variable will be used to feed a vector compare.
(This complements the "all users are scalar after vectorization" below.)

Hi Ayal, thanks for taking a look! I think this addresses your comments:

  • removed the call to CreateSplatIV, which was indeed redundant.
  • updated comments,
  • updated tests.

Previous diff was not the full diff, it contained only the latest changes; this is the full diff.

Ayal accepted this revision.May 6 2020, 12:50 PM

Thanks for cleaning this up!
Just adding another follow-up.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
1869

This is now called only for VF=1, so doesn't really create vectors by splatting a scalar IV, only takes care of bump it across UF parts. Can be further cleaned-up, as a follow-up patch.

This revision is now accepted and ready to land.May 6 2020, 12:50 PM

Many thanks for your time and reviews. And I will follow up on what you suggested.

This revision was automatically updated to reflect the committed changes.