-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VP] Mark llvm.experimental.vp.splice as having no functional equivalent #70647
Conversation
llvm.experimental.vp.splice has different semantics from llvm.experimental.splice (since it takes uses the EVL arguments in a way other than just masking the tail elements), so it shouldn't be expanded to the unpredicated version. Coincidentally there's no support for llvm.experimental.vp.splice in ExpandVectorPredication, so it wasn't getting expanded, but we shouldn't mark it as functionally equivalent anyway since there's other users of the property now e.g. in VectorCombine.
@llvm/pr-subscribers-llvm-ir Author: Luke Lau (lukel97) Changesllvm.experimental.vp.splice has different semantics from Coincidentally there's no support for llvm.experimental.vp.splice in Full diff: https://github.com/llvm/llvm-project/pull/70647.diff 1 Files Affected:
diff --git a/llvm/include/llvm/IR/VPIntrinsics.def b/llvm/include/llvm/IR/VPIntrinsics.def
index 55a68ff5768ddb0..0ca60bcfbb3d03f 100644
--- a/llvm/include/llvm/IR/VPIntrinsics.def
+++ b/llvm/include/llvm/IR/VPIntrinsics.def
@@ -699,7 +699,7 @@ VP_PROPERTY_NO_FUNCTIONAL
END_REGISTER_VP(vp_merge, VP_MERGE)
BEGIN_REGISTER_VP(experimental_vp_splice, 3, 5, EXPERIMENTAL_VP_SPLICE, -1)
-VP_PROPERTY_FUNCTIONAL_INTRINSIC(experimental_vector_splice)
+VP_PROPERTY_NO_FUNCTIONAL
END_REGISTER_VP(experimental_vp_splice, EXPERIMENTAL_VP_SPLICE)
///// } Shuffles
|
Should we just drop handling for it for now or is it possible to create a test case? |
We don't currently handle it in ExpandVectorPredication.cpp to begin with, but I guess we might in future? |
So would we be better off removing it until we have an actual use case? |
By "it" do you mean removing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation! LGTM
llvm.experimental.vp.splice has different semantics from
llvm.experimental.splice (since it takes uses the EVL arguments in a way other
than just masking the tail elements), so it shouldn't be expanded to the
unpredicated version.
Coincidentally there's no support for llvm.experimental.vp.splice in
ExpandVectorPredication, so it wasn't getting expanded, but we shouldn't mark
it as functionally equivalent anyway since there's other users of the property
now e.g. in VectorCombine.