[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Builtins.def:649
 BUILTIN(__builtin_elementwise_min, "v.", "nct")
+BUILTIN(__builtin_elementwise_ceil, "v.", "nct")
 BUILTIN(__builtin_reduce_max, "v.", "nct")
----------------
If we're doing ceil, should we be doing floor at the same time given how much overlap there should be in the implementations?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:16727-16728
   QualType TyA = A.get()->getType();
-  if (checkMathBuiltinElementType(*this, ArgLoc, TyA))
+  if (checkMathBuiltinElementType(*this, ArgLoc, TyA) ||
+      ExtraCheck(TyA, ArgLoc))
     return true;
----------------
In some ways, it's a not really ideal that we split the type checking like this. `SemaBuiltinElementwiseMathOneArg()` checks some type validity and the caller checks some more type validity, so there's no one location to know what types are actually expected for any given builtin calling this.

Would it be reasonable to instead hoist all the type checking code out of `SemaBuiltinElementwiseMathOneArg()` and not pass in a functor to it, but instead check the `CallExpr` type after the call returns? We could also fix up the function name to be a bit less nonsensical at the same time, like renaming it to `PrepareBuiltinElementwiseMathOneArgCall()` or something along those lines?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114688/new/

https://reviews.llvm.org/D114688