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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits