Page MenuHomePhabricator

[LV] Fix versioning-for-unit-stide of loops with small trip count
ClosedPublic

Authored by Ayal on Jul 9 2020, 3:29 AM.

Details

Summary

This patch fixes D81345 and PR46652.

If a loop with a small trip count is compiled w/o -Os/-Oz, Loop Access Analysis still generates runtime checks for unit strides that will version the loop.

In such cases, the loop vectorizer should either re-run the analysis or bail-out from vectorizing the loop, as done prior to D81345. The latter is chosen for now as the former requires refactoring.

Diff Detail

Event Timeline

Ayal created this revision.Jul 9 2020, 3:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 9 2020, 3:29 AM
fhahn accepted this revision.Jul 9 2020, 9:58 AM

LGTM, thanks!

In such cases, the loop vectorizer should either re-run the analysis or bail-out from vectorizing the loop, as done prior to D81345. The latter is chosen for now as the former requires refactoring.

As already discussed in D81345, ideally LV would have more flexibility to drive LAA, but this requires non-trivial refactoring. Which we should do, but until then the patch looks like a reasonable fix to the crash.

llvm/test/Transforms/LoopVectorize/optsize.ll
239

Better to use a non-undef constant/value?

This revision is now accepted and ready to land.Jul 9 2020, 9:58 AM
This revision was automatically updated to reflect the committed changes.
Ayal marked an inline comment as done.Jul 12 2020, 10:48 AM

LGTM, thanks!

In such cases, the loop vectorizer should either re-run the analysis or bail-out from vectorizing the loop, as done prior to D81345. The latter is chosen for now as the former requires refactoring.

As already discussed in D81345, ideally LV would have more flexibility to drive LAA, but this requires non-trivial refactoring. Which we should do, but until then the patch looks like a reasonable fix to the crash.

Indeed, seems like LV would need to re-run analyzeLoop(), which would affect other users.

llvm/test/Transforms/LoopVectorize/optsize.ll
239

Sure, done, thanks.