erichkeane added a comment. In D140639#4028900 <https://reviews.llvm.org/D140639#4028900>, @arsenm wrote:
> In D140639#4028883 <https://reviews.llvm.org/D140639#4028883>, @erichkeane > wrote: > >> 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, > > The point of every builtin is direct access to llvm intrinsics, in this case > llvm.copysign. I need it on vectors and not the set of 3 scalars it handles > now. I'm unfamiliar with the semantics of that builtin other than what i can read here: https://llvm.org/docs/LangRef.html#llvm-copysign-intrinsic ================ Comment at: clang/lib/Sema/SemaChecking.cpp:2660 + + ExprResult Magnitude = UsualUnaryConversions(TheCall->getArg(0)); + ExprResult Sign = UsualUnaryConversions(TheCall->getArg(1)); ---------------- arsenm wrote: > erichkeane wrote: > > 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? > Copied from the other elementwise intrinsics, and this is at the edges of my > frontend knowledge (I guess it's to avoid qualifiers mattering?). The tests > seem to behave as I expect It really depends on what behavior you're looking to get out of this. UnaryConversions are usually for operators, not 'function like' things, so it is a touch jarring to me. I guess I would have expected DefaultFunctionArrayLValueConversion (which calls DefaultLValueConversion after doing array-to-pointer conversions). If the idea is for this builtin to act more like a variadic arg, I'd expect to see DefaultArgumentPromotion. @aaron.ballman I think is smarter than me in regards to how these should work, so perhaps he can comment here? I DO note one of the things that UsualUnaryConversions is doing is removing 'half' types on platforms without a 'native' half type. I would expect those sorts of conversions wouldn't be right? ================ Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + + if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), ---------------- curleys not used for single-statement if-statement bodies. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140639/new/ https://reviews.llvm.org/D140639 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
