[llvm-commits] [PATCH] Add va_argpack intrinsics

[Eli Friedman]

On Tue, May 15, 2012 at 11:03 AM, 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...

Consider the following on x86-64:
void f(int x, int y, ...);
struct A { long long x,y; };
void g(int a, int b, int c, int d, ...) {
  //f(a,b,__builtin_va_pack());
  f(a,b,(struct A){},(struct A){});
}
void h() {
  struct A x;
  g(0,0,0,0,x,x);
}

-Eli