[PATCH] D41335: [InlineFunction] Inline vararg functions that do not access varargs.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1838-1843
+          SmallVector<Value *, 6> Params(CI->arg_operands());
+          Params.append(VarArgsToForward.begin(), VarArgsToForward.end());
+          CallInst *Call =
+              CallInst::Create(CI->getCalledFunction() ? CI->getCalledFunction()
+                                                       : CI->getCalledValue(),
+                               Params, "", CI);
----------------
fhahn wrote:
> rnk wrote:
> > I think you're forgetting to transfer argument attributes. Consider this C++ test case:
> > ```
> > struct ByVal { char x; short y; };
> > struct Foo {
> >   virtual int foo(struct ByVal o) { return o.x + o.y; }
> > };
> > int main() {
> >   ByVal o = {3, 5};
> >   auto mp = &Foo::foo;
> >   return (Foo().*mp)(o);
> > }
> > ```
> > 
> > Compile it with `clang -cc1 t.cpp  -emit-llvm -triple i686-windows-msvc` to get LLVM IR, and there will be a thunk called `??_9Foo@@$BA at AE` that we should be able to inline after your change. However, it's important that we retain the `byval` attribute on the `o` argument, or we will have mismatched calling conventions.
> > 
> > Argument forwarding is actually quite involved. Take a look at ArgumentPromotion and DeadArgElim for examples of how to do it as correctly as we know how to.
> Yep thanks. I was planning to fix the attributes  & co, but I'll do it before this patch lands.
I've added D41555 and D41556 to forward attributes & calling conventions. I could also put everything in this patch, but I thought this way it would be easier to review.


https://reviews.llvm.org/D41335