[PATCH] D53927: [AArch64] Enable libm vectorized functions via SLEEF
Stefan Teleman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 26 16:04:48 PST 2019
steleman marked 3 inline comments as done.
steleman added inline comments.
================
Comment at: include/llvm/IR/Intrinsics.td:516-533
+ def int_acos : Intrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>]>;
+ def int_asin : Intrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>]>;
+ def int_atan : Intrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>]>;
+ def int_atan2 : Intrinsic<[llvm_anyfloat_ty],
+ [LLVMMatchType<0>, LLVMMatchType<0>]>;
+ def int_tan : Intrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>]>;
def int_pow : Intrinsic<[llvm_anyfloat_ty],
----------------
echristo wrote:
> steleman wrote:
> > arsenm wrote:
> > > I would split adding the new intrinsics into a separate patch
> > That won't work.
> >
> >
> "Why not" is always a good follow up to that sort of response :)
Because:
- It adds unnecessary complexity to this changeset **and** to D53928.
- There is no obvious benefit to this added complexity.
- There are many drawbacks to this added complexity.
- Splitting it based on new vs. existing intrinsics seems arbitrary. One might as well ask for a separate changeset for each and every new intrinsic.
And, now it's my turn to ask:
- Why should it be split?
- What is the benefit of splitting it?
- What will be accomplished by splitting it?
- Is increased complexity - with all its drawbacks and pitfalls - a goal?
- Does integrating it //as is// prevent, or hinder something else?
- Why does this come up now, after this changeset - and its friend D53928 - have been reviewed for 4+ months, and already approved, and re-approved, and there are 21 subscribers to this changeset?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53927/new/
https://reviews.llvm.org/D53927
More information about the llvm-commits
mailing list