beanz added inline comments.
================ Comment at: clang/include/clang/Basic/Builtins.def:1697 +// HLSL +LANGBUILTIN(WaveActiveCountBits, "Uib", "nc", HLSL_LANG) + ---------------- beanz wrote: > python3kgae wrote: > > Anastasia wrote: > > > python3kgae wrote: > > > > Anastasia wrote: > > > > > FYI we most of time try to add a builtin name using a reserved > > > > > identifier which is not part of the language standard (mostly > > > > > prefixed by `__`). Then we add regular function that just calls the > > > > > clang builtins. This way provides more flexibility to the > > > > > implementers. However you might not need this... in which case using > > > > > original name avoids an extra call. > > > > Yes. For this one, it is without prefix to avoid extra call. > > > > I'm following things like enqueue_kernel in opencl. > > > > For other things support overload which has to make extra call, I'll > > > > add the prefix. > > > Ok, although `enqueue_kernel` was implemented as a builtin because it has > > > a weird variadic prototype that can't be implemented using normal > > > features of C/C++ languages. Hence it is a builtin with custom > > > SemaChecking. > > I see. > > Since HLSL also has things cannot implemented using HLSL itself, we cannot > > put all HLSL intrinsic in one place anyway. > > So when it is possible to reduce an extra call, I just reduce it. > I don't think this was the right decision. It is trivial for the optimizer to > remove a function that is more or less a tail-call of another function, and > since HLSL has no call semantics anyways they will disappear. > > It would be much better if this was a function that called the builtin, this > could even be trivially implemented as a sub-header of hlsl.h as a wrapper > function. Let me expand a little here. I think we need to change this. We should name builtins as builtins (i.e. __builtin_hlsl_...), and we can implement a wrapper function in one of the headers from hlsl.h. There are two reasons I think we should take this approach: 1) Matching existing builtin patterns makes it clear in the code 2) Clang builtins don't populate in the AST for AST queries, which impacts how the function for IDE tooling. Having the HLSL user-surfaced functions exist in the AST will allow for better IDE tooling for autocomplete. If hlsl.h gets too slow to parse, there are other approaches we can take like using the external sema source, modules or PCH to speed it up later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126857/new/ https://reviews.llvm.org/D126857 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
