erichkeane added inline comments.
================ Comment at: clang/lib/Sema/SemaChecking.cpp:2660 + + ExprResult Magnitude = UsualUnaryConversions(TheCall->getArg(0)); + ExprResult Sign = UsualUnaryConversions(TheCall->getArg(1)); ---------------- aaron.ballman wrote: > erichkeane wrote: > > 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? > I think we need to do the usual unary conversions because that's what handles > the floating point evaluation method stuff, and if I'm reading this properly, > it seems that `copysign()` does perform these conversions: > https://godbolt.org/z/eWjEqvvjd. I would not expect to handle this with > default argument promotion given the signature of `copysign()`. Got it, thanks for the clarification. 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
