[llvm-commits] [PATCH] Add va_argpack intrinsics

[Chandler Carruth]

I think the latest patch is good, Eli, do you have any concerns before we
move forward and start working on the Clang side of things?

Ben, one think we might do to help address Eli's concerns is add some scare
notes to the documentation for these intrinsics saying that they are
extremely dangerous, etc etc.

Once we get the Clang side rigged up, it'll be a lot easier to do testing...
-Chandler

On Tue, May 15, 2012 at 12:03 PM, Chandler Carruth <chandlerc at google.com>wrote:

> On Tue, May 15, 2012 at 11:36 AM, Eli Friedman <eli.friedman at gmail.com>wrote:
>
>> On Tue, May 15, 2012 at 10:03 AM, Benjamin Kramer
>> <benny.kra at googlemail.com> wrote:
>> >
>> > On 15.05.2012, at 18:16, Chandler Carruth wrote:
>> >
>> >> On Tue, May 15, 2012 at 9:08 AM, Benjamin Kramer <
>> benny.kra at googlemail.com> wrote:
>> >> This will be used to implement __builtin_va_arg_pack[len] in clang
>> (PR7219).
>> >>
>> >> Awesome, thanks for working on this...
>> >>
>> >> Since this is used to model a pretty crazy gcc extension (used by
>> glibc) we have
>> >> to expand the intrinsics in the inliner. When it replaces a vararg
>> call site with
>> >> a copy of the function another pass over all instructions is performed
>> which
>> >> replaces any pack intrinsics with the values from the call site. There
>> are some
>> >> rather narrow requirements where the intrinsics can occur, documented
>> in LangRef
>> >> and enforced by the verifier.
>> >>
>> >> This patch has two other side effects:
>> >> - DAE is not allowed to remove varargs if a va_pack intrinsic is used.
>> >> - Inliner can now inline vararg functions iff they don't use va_start.
>> This can
>> >>  happen if DAE doesn't remove the varargs because there is a va_pack
>> intrinsic
>> >>  or if the AlwaysInliner tries to inline the function.
>> >>
>> >>
>> >> I'm pretty OK wit this strategy. It actually has some other
>> interesting artifacts I'd like to explore later. Specifically, I think we
>> can generalize this to allow inlining of varargs functions when the
>> va_start is in dead code...
>>
>> I'm actually mildly concerned about this strategy: the LLVM IR inliner
>> can't actually reason correctly about the ABI rules for a varargs
>> call, and will therefore screw up if you try to pass anything
>> complicated (_Complex, structs) through the varargs list.  This might
>> not matter in practice if it isn't used for anything other than the
>> printf and scanf families of functions, but it's still worth thinking
>> about.
>
>
> I share your concerns here regarding the pack and pack_len builtins...
> However, I think in the extremely limited context they are used, this is
> safe.
>
> The idea is that these builtins can only be used inside of some other
> vararg function, and the only arguments we move from the outer call to the
> inner are ones that would already have gone through Clang's vararg
> lowering. The goal seems specifically to re-use the way that the outer call
> is lowered when setting up the inlined inner call.
>
> I've been trying to think of a construct that breaks this, but so far I
> can't come up with any...
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120515/10639cd2/attachment.html>