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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to