[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign
Erich Keane via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 5 07:48:21 PST 2023
erichkeane added a comment.
1 nit, and 1 trying to see what is going on. I don't have a good feeling what the purpose of this builtin is, nor whether it matches the desire/intent of this builtin, I'm hopeful one of the other reviewers has the ability to check that.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:2048
+ if (!EltTy->isRealFloatingType()) {
+ S.Diag(Loc, diag::err_builtin_invalid_arg_type)
+ << ArgIndex << /* vector or float ty*/ 5 << ArgTy;
----------------
you can just do "return S.Diag", which always returns 'true'. This will save a line, and the need for curleys.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:2660
+
+ ExprResult Magnitude = UsualUnaryConversions(TheCall->getArg(0));
+ ExprResult Sign = UsualUnaryConversions(TheCall->getArg(1));
----------------
What is the point of the Unary Conversions here? Its a touch surprising to see a builtin do that? Note that it does quite a bit of FP related conversions, so are you sure you want those?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140639/new/
https://reviews.llvm.org/D140639
More information about the cfe-commits
mailing list