[PATCH] D57635: Add a new intrinsic and attribute to implement `__builtin_va_arg_pack{, _len}` in Clang
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 2 14:18:22 PST 2019
efriedma added a comment.
Maybe a silly question, but is this actually worth implementing, at this point? The new IR additions have unusual semantics that transforms are continually going to trip over; I'm not convinced you found all the places that currently need checks. And there isn't any existing code we need to be compatible with, I think. If we were designing a frontend extension from scratch, we would probably take a different approach.
================
Comment at: llvm/docs/LangRef.rst:1408
+ arguments of the caller will be forwarded into the call instruction. This
+ attribute is used to implement `__builtin_va_arg_pack`.
``jumptable``
----------------
I'm not really a fan of using an attribute like this. Currently, there are basically two ways to refer to a varargs list: va_start, and a musttail call. This is adding a third, in a way that's very easy to miss.
Not sure what it should look like, though. Maybe instead of calling the function directly, we should call an intrinsic? I guess any approach has odd semantic implications.
================
Comment at: llvm/docs/LangRef.rst:10475
+difference of the total number of arguments at the call site and the number of
+declared parameters.
+
----------------
You're computing the difference here using the number of IR arguments? That's not going to return the correct result in general; you need the frontend to compute this.
-------
I wonder if we should add an attribute to denote "weird" intrinsics that can't be treated as equivalent to a function call in general. (For example, I think you have to forbid function merging on any function that calls this intrinsic.)
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57635/new/
https://reviews.llvm.org/D57635
More information about the llvm-commits
mailing list