[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 1 08:17:06 PST 2021
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))
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
More information about the cfe-commits