Page MenuHomePhabricator

[LV] Make sure VF doesn't exceed compile time known TC
AcceptedPublic

Authored by ebrevnov on Wed, Nov 24, 5:08 AM.

Details

Summary

For the simple copy loop (see test case) vectorizer selects VF equal to 32 while the loop is known to have 17 iterations only. Such behavior makes no sense to me since such vector loop will never be executed. The only case we may want to select VF large than TC is masked vectoriztion. So I haven't touched that case.

Diff Detail

Event Timeline

ebrevnov created this revision.Wed, Nov 24, 5:08 AM
ebrevnov requested review of this revision.Wed, Nov 24, 5:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 24, 5:08 AM
ebrevnov edited the summary of this revision. (Show Details)Wed, Nov 24, 5:22 AM
ebrevnov added reviewers: fhahn, anna, Ayal.

Please note we still vectorize epilog with VF 8 while the remainder number of iterations is known to be 1. That should be addressed separately though..

Sounds sensible to me.

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

foldTailByMasking -> FoldTailByMasking

6076–6077

Can you update this comment..

6085

.. and this debug message.

llvm/test/Transforms/LoopVectorize/X86/known_TC.ll
6–7

This can be removed now?

ebrevnov marked 4 inline comments as done.Thu, Nov 25, 9:36 PM
ebrevnov added inline comments.
llvm/test/Transforms/LoopVectorize/X86/known_TC.ll
6–7

Yep, missed that...

ebrevnov updated this revision to Diff 389902.Thu, Nov 25, 9:42 PM
ebrevnov marked an inline comment as done.

Updated

dmgreen accepted this revision.Fri, Dec 3, 2:21 AM

LGTM then, if there are not more comments. Thanks.

This revision is now accepted and ready to land.Fri, Dec 3, 2:21 AM
fhahn added inline comments.Fri, Dec 3, 2:53 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
6085

Shouldn't this be PowerOf2Floor(ConstTripCount)?

dmgreen added inline comments.Fri, Dec 3, 3:05 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
6085

I think this is saying "the trip count is "ConstTripCount", so we are rounding that down to a power of 2 (left as an exercise to the reader)."

It may be better to be clearer about the values it will use though. Having what it thinks the original ConstTripCount printed somewhere does sound useful.

ebrevnov added inline comments.Fri, Dec 3, 3:46 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
6085

I think this is saying "the trip count is "ConstTripCount", so we are rounding that down to a power of 2 (left as an exercise to the reader)."

While I agree this is valid interpretation I did mean to provide resulting value. So the printing PowerOf2Floor(ConstTripCount) here sounds reasonable to me. Do you think we should change wording?

It may be better to be clearer about the values it will use though. Having what it thinks the original ConstTripCount printed somewhere does sound useful.

It is printed out at the beginning of computeFeasibleMaxVF.