This is an archive of the discontinued LLVM Phabricator instance.

[NFC][LoopVectorize] Remove unnecessary VF.isScalable asserts
ClosedPublic

Authored by david-arm on Apr 1 2021, 7:02 AM.

Details

Summary

There are a few places in LoopVectorize.cpp where we have been too
cautious in adding VF.isScalable() asserts and it can be confusing.
It also makes it more difficult to see the genuine places where
work needs doing to improve scalable vectorization support.

This patch changes getMemInstScalarizationCost to return an
invalid cost instead of firing an assert for scalable vectors. Also,
vectorizeInterleaveGroup had multiple asserts all for the same
thing. I have removed all but one assert near the start of the
function, and added a new assert that we aren't dealing with masks
for scalable vectors.

Diff Detail

Event Timeline

david-arm created this revision.Apr 1 2021, 7:02 AM
david-arm requested review of this revision.Apr 1 2021, 7:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2021, 7:02 AM

Makes sense to me, though I'll defer acceptance since I'm not actively working on this code right now. I assume you can multiply InstructionCost by an integer even if the cost is invalid?

vkmr added a subscriber: vkmr.Apr 4 2021, 8:30 PM

Hi @frasercrmck yeah that's right, we can multiply an invalid cost by another value and the cost remains invalid.

sdesmalen added inline comments.Apr 9 2021, 7:05 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
2713

You can remove this assert, because VF is already guaranteed to be not-scalable from line 2656.

david-arm updated this revision to Diff 336472.Apr 9 2021, 8:21 AM
  • Removed extra assert for case where we are interleaving with masks
david-arm marked an inline comment as done.Apr 9 2021, 8:21 AM
sdesmalen accepted this revision.Apr 9 2021, 8:22 AM

Thanks, LGTM!

This revision is now accepted and ready to land.Apr 9 2021, 8:22 AM