(release 20.x non-upstream patch) [LV] Teach the vectorizer to cost and vectorize llvm.sincos intrinsics #84
Conversation
|
Depends on #82. This PR is a copy of the LLVM PR llvm/llvm-project#123210, which was not possible to upstream for the release. |
|
Does this need to be kept in-sync with the upstream version - llvm/llvm-project#123210? |
It's not hugely important to as all the changes to the upstream version have all been NFCs (so far), but I will update it anyway once #84 lands. |
MacDue
force-pushed
the
release_20_sincos_vec
branch
from
78601b9 to
ceec695
Compare
This teaches the loop vectorizer that `llvm.sincos` is trivially vectorizable. Additionally, this patch updates the cost model to cost intrinsics that return multiple values correctly. Previously, the cost model only thought intrinsics that return `VectorType` need scalarizing, which meant it cost intrinsics that return multiple vectors (that need scalarizing) way too cheap (giving it the cost of a single function call). The `llvm.sincos` intrinsic also has a custom cost when a vector function library is available, as certain VFs can be expanded (later in code-gen) to a vector function, reducing the cost to a single call (+ the possible loads from the vector function returns values via output pointers).
MacDue
force-pushed
the
release_20_sincos_vec
branch
from
ceec695 to
0763c9f
Compare
kiranchandramohan
requested review from
DavidTruby,
kiranchandramohan and
pawosm-arm
There was a problem hiding this comment.
LGTM. Assuming this is a cherry-pick with all changes from upstream.
What about llvm/llvm-project#128035 and llvm/llvm-project#128885. Are they also required?
Yes, all of this is cherry-picked from patches that have landed in LLVM upstream.
llvm/llvm-project#128035 is part of this PR -- it was in the original upstream PR too, but split off for review purposes (but I don't think that's necessary here). llvm/llvm-project#128885 is not required (it's just test changes). |
|
I'd like to hold off landing this till tomorrow morning, just in case there's any reports upstream from the changes in llvm/llvm-project#128035, but otherwise this is ready to go. |
|
@kiranchandramohan Can you merge this PR? |
This teaches the loop vectorizer that
llvm.sincosis trivially vectorizable. Additionally, this patch updates the cost model to cost intrinsics that return multiple values correctly. Previously, the cost model only thought intrinsics that returnVectorTypeneed scalarizing, which meant it cost intrinsics that return multiple vectors (that need scalarizing) way too cheap (giving it the cost of a single function call).The
llvm.sincosintrinsic also has a custom cost when a vector function library is available, as certain VFs can be expanded (later in code-gen) to a vector function, reducing the cost to a single call (+ the possible loads from the vector function returns values via output pointers).Downstream issue: #87