Page MenuHomePhabricator

[LV] Still vectorise when tail-folding can't find a primary inducation variable
ClosedPublic

Authored by SjoerdMeijer on Jan 7 2020, 5:13 AM.

Details

Summary

This addresses a vectorisation regression for tail-folded loops that are counting down, e.g. loops as simple as this:

void foo(char *A, char *B, char *C, uint32_t N) {
  while (N > 0) {
    *C++ = *A++ + *B++;
     N--;
  }
}

These are loops that can be vectorised, but when tail-folding is requested, it can't find a primary induction variable which we do need for predicating the loop. As a result, the loop isn't vectorised at all, which it is able to do when tail-folding is not attempted. So, this adds a check for the primary induction variable where we decide how to lower the scalar epilogue. I.e., when there isn't a primary induction variable, a scalar epilogue loop is allowed (i.e. don't request tail-folding) so that vectorisation could still be triggered.

Having this check for the primary induction variable make sense anyway, and in addition, in a follow-up of this I will look into discovering earlier the primary induction variable for counting down loops, so that this can also be tail-folded.

Diff Detail

Event Timeline

SjoerdMeijer created this revision.Jan 7 2020, 5:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2020, 5:13 AM
samparker added inline comments.Jan 8 2020, 12:39 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
7516

The ordering of the predicates tests seems to be a bit off from a readability point-of-view. If we're only thinking about predication, I would expect an early return if PredicateOptDisabled, which would also include Hints.getPredicate() == LoopVectorizeHints::FK_Disable. The last piece of logic would then only contain all the values that we require to enable the folding.

7652–7653

nit: why not just pass as reference?

SjoerdMeijer updated this revision to Diff 236797.EditedJan 8 2020, 5:21 AM

Thanks for looking at this! And also for encouraging me to look at this (my own) spaghetti logic again. But to be fair, we have quite a few factors that play a role here: optimising for minsize takes precedence over the prefer predicate options, which take precedence over the loop hints, which take precedence over the TTI hook. I have explained this in the comments, and have reshuffled the logic accordingly. I am now bailing earlier on PredicateOptDisabled, as you suggested, loop hints need to be checked lastly, and thus this addresses your comments, I think.

samparker added inline comments.Jan 8 2020, 7:26 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
7538

If a hint is provided to disable folding, then we shouldn't even look at any of the Prefer stuff, right? So PredicateOptDisabled = (PreferPredicateOverEpilog.getNumOccurrences() && !PreferPredicateOverEpilog) || Hints.getPredicate() == LoopVectorizeHints::FK_Disabled)

SjoerdMeijer marked an inline comment as done.Jan 8 2020, 8:13 AM
SjoerdMeijer added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
7538

We have these test cases:

Transforms/LoopVectorize/ARM/tail-loop-folding.ll
Transforms/LoopVectorize/X86/tail_loop_folding.ll

That have functions with loop hint predicate.enable=false and also option -prefer-predicate-over-epilog set. The expected output (in these tests) is that this will enable predication, and thus the option overrides the loop hint. That's why I didn't move the loophint check to PredicateOptDisabled.

samparker accepted this revision.Jan 9 2020, 12:44 AM

LGTM

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

Bah, I've missed some brackets - sorry!

This revision is now accepted and ready to land.Jan 9 2020, 12:44 AM
This revision was automatically updated to reflect the committed changes.