This is an archive of the discontinued LLVM Phabricator instance.

[SLP] Use getExtractWithExtendCost() to compute the scalar cost of extractelement/ext pair
ClosedPublic

Authored by haicheng on Apr 9 2018, 6:09 PM.

Details

Summary

We use getExtractWithExtendCost to calculate the cost of extractelement and s|zext together when computing the extract cost after vectorization, but we calculate the cost of extractelement and s|zext separately when computing the scalar cost which is larger than it should be.

Diff Detail

Repository
rL LLVM

Event Timeline

haicheng created this revision.Apr 9 2018, 6:09 PM
ABataev added inline comments.Apr 10 2018, 6:22 AM
test/Transforms/SLPVectorizer/AArch64/ext-trunc.ll
2 ↗(On Diff #141772)

Could you regenerate test checks without your patch and without -instcombine first and commit as NFC? And after that generate these checks with your patch

haicheng updated this revision to Diff 142022.Apr 11 2018, 8:37 AM
haicheng marked an inline comment as done.

Rebased the test case. Thank you for taking a look.

ABataev added inline comments.Apr 11 2018, 9:52 AM
lib/Transforms/Vectorize/SLPVectorizer.cpp
2199 ↗(On Diff #142022)

I think you need to check that Ext user is GEP and Ext is used as GEP index.

haicheng updated this revision to Diff 142063.Apr 11 2018, 1:32 PM

Check the users of Ext now.

ABataev added inline comments.Apr 12 2018, 6:57 AM
lib/Transforms/Vectorize/SLPVectorizer.cpp
2201 ↗(On Diff #142063)

I think you can use just GetElementPtr::classof here as a predicate. Also, it would be good to have negative test(s) for the situation where this should not work.

haicheng updated this revision to Diff 142442.Apr 13 2018, 11:14 AM
haicheng marked an inline comment as done.

Address Alexey's comments.

ABataev added inline comments.Apr 13 2018, 11:18 AM
lib/Transforms/Vectorize/SLPVectorizer.cpp
2201 ↗(On Diff #142063)

No, I meant that instead of lambda you can just use GetElementPtrInst::classof

llvm::all_of(Ext->users(), GetElementPtrInst::classof)
haicheng added inline comments.Apr 13 2018, 12:39 PM
lib/Transforms/Vectorize/SLPVectorizer.cpp
2201 ↗(On Diff #142063)

I tried, but I got this error when compiling

llvm/ADT/STLExtras.h:846:6: note: candidate template ignored: couldn't infer template argument 'UnaryPredicate'

ABataev added inline comments.Apr 13 2018, 12:44 PM
lib/Transforms/Vectorize/SLPVectorizer.cpp
2201 ↗(On Diff #142063)

Ok then, revert back to isa

haicheng updated this revision to Diff 142481.Apr 13 2018, 4:20 PM
haicheng marked 3 inline comments as done.

Address Alexey's comments. Thank you.

This revision is now accepted and ready to land.Apr 16 2018, 6:55 AM
This revision was automatically updated to reflect the committed changes.