[PATCH] D67159: [clang] New __attribute__((__clang_builtin)).

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

Sorry about that – I didn't want to put the discussion of rationale in too many different places. The commit message for the followup patch D67161 <https://reviews.llvm.org/D67161> discusses it a bit.

The usual thing in previous intrinsics systems like NEON is that you have a wrapper function in the header file which calls the builtin. That already doesn't work in every situation, because some intrinsics need one of their arguments to be a compile-time integer constant expression, and with a wrapper function in the way, the constantness property is lost between the user's call site and the builtin. So then you implement //those// builtins as macros instead of wrapper functions.

The MVE intrinsics spec adds the wrinkle that there are polymorphic functions – with more concise names, overloaded on the argument types. For example, `vaddq(v,w)` can behave like `vaddq_s32` or `vaddq_f16` or whatever, depending on what vector types you give it. Those have to be implemented using either `__attribute__((overloadable))` or `_Generic`.

`__attribute__((overloadable))` works fine for wrapper functions in the header file, but not for that awkward subset of the functions that have to fall back to being macros. For those, you //have// to use `_Generic` (if you're not willing to do what I've done here).

`_Generic` is immensely awkward, because the difficult thing about it is that all the selection branches //not// taken still have to type-check successfully. So you can't just say something like 'if this is a `uint16x8_t` then expand to `vfooq_u16`, else `vfooq_f16`, etc', because all but one of those will be type errors. Instead you have to pile in endless workarounds in which each branch of the `_Generic` is filled with sub-Generics that coerce the arguments in untaken branches to something that is nonsense but won't cause a type-check error – and //hope// that none of your 'replace it with validly typed nonsense' workarounds won't accidentally trigger in any case where the branch //is// taken.

Also, if you need to disambiguate based on //more// than one of the argument types (which can happen in this spec), then you have the same problem again: you can't nest a `_Generic` switching on argument 2 inside each branch of an outer one switching on argument 1, because as soon as the set of legal type pairs isn't a full Cartesian product, the inner Generic of an untaken branch fails to type-check again, so you need yet more layers of workaround.

And after you've done all that, the error reporting is //hideous//. It's almost as bad as those C++ error messages gcc used to generate in which it had expanded out some STL type in the user's code into three whole pages of library-internals nonsense.

Doing it //this// way, what happens is that first clang resolves the `__attribute__((overloadable))` based on all the argument types at once; then, once it's decided which function declaration is the interesting one, it looks up the `BuiltinId` that I put there using this mechanism; and then it's looking at a call directly to that builtin from the user's code, so it can check constant arguments at their original call site. It needs almost no code; no enormous chain of ugly workarounds; and if the user passes an invalid list of argument types, the error report is //beautiful//: clang will show you each declaration of the polymorphic name from the header file, and legibly explain why each one doesn't match the arguments the user actually passed.

So, I hear you: this is something clang has always found a way to do without before. But if I can't do it this way, could you suggest a way that I could get anything like that really nice error reporting, under all those constraints at once?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159