This is an archive of the discontinued LLVM Phabricator instance.

Make SLPVectorizer pass the right type argument to getCmpSelInstrCost()
ClosedPublic

Authored by jonpa on Apr 3 2017, 8:07 AM.

Details

Reviewers
nadav
mssimpso
Summary

(This was also on llvm-commits last week without test case: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170327/442107.html)

I found that the SLP vectorizer does not call getCmpSelInstrCost() for a
compare with the proper type. It currently passes i1 or a vector of i1s,
instead of the type of the compare operand, like LoopVectorizer does.

My patch fixes this by getting the right scalar type for compares. No
regressions after applying this patch.

The test case is for SystemZ, and depends on https://reviews.llvm.org/D29631#09b6b393.

Diff Detail

Event Timeline

jonpa created this revision.Apr 3 2017, 8:07 AM
mssimpso edited edge metadata.Apr 6 2017, 11:29 AM

Hi Jonas,

This makes sense to me - just a couple of minor comments.

lib/Transforms/Vectorize/SLPVectorizer.cpp
1705

Why not use an "else if" here?

test/Transforms/SLPVectorizer/SystemZ/SLP-cmp-cost-query.ll
1

You'll need a "REQUIRES: asserts" line to check debug output.

jonpa updated this revision to Diff 94488.Apr 6 2017, 11:52 PM

changed to "if else".
Test updated with '; REQUIRES: asserts'

jonpa marked 2 inline comments as done.Apr 6 2017, 11:53 PM
mssimpso accepted this revision.Apr 7 2017, 10:22 AM

LGTM.

This revision is now accepted and ready to land.Apr 7 2017, 10:22 AM
jonpa closed this revision.Apr 12 2017, 6:49 AM

Thanks for review!
r300061