rnk added inline comments.
================
Comment at: lib/CodeGen/CodeGenFunction.cpp:2395
+
+ llvm::CallInst *Result = Builder.CreateCall(FuncToReturn, Args);
+
----------------
rnk wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > rnk wrote:
> > > > This approach is... not going to work in the general case. Consider,
> > > > for example, varargs. Then consider 32-bit x86 inalloca, which is quite
> > > > widespread. Any non-trivially copyable object is not going to be
> > > > passable through such a thunk. You might consider looking at
> > > > CodeGenFunction::EmitMustTailThunk instead.
> > > Oh dear...
> > > I'm unfamiliar with EmitMustTailThunk, is it self explanatory? Any
> > > chance you can expound? Should I call/use that function, or copy out of
> > > it?
> > I looked through that function, and it seems to do very similar things to
> > this...
> >
> > It FIRST does the small-vector conversion to a value* array like I do.
> >
> > Second, it 'adjusts' the 'this' parameter. This doesn't seem like it needs
> > to happen, because the type isn't changing, right? my 'this' pointer is
> > still pass-on-able.
> >
> > Third, it creates the call, then marks it TCK_MustTail.
> >
> > Forth, it sets the attributes/calling convention of the 'call' object, but
> > I'd expect that to come from the llvm::Function object in the 'CreateCall',
> > right? I can add them after the fact I guess, if we see value.*(see below)
> >
> > Finally, it does the CreateRetVoid/CreateRet like this function does below.
> >
> > *I DO have a test that claims that my IR is broken if I set the call
> > attributes. Curiously only 1 of my tests, but the error is:
> > "cannot guarantee tail call due to mismatched ABI impacting function
> > attributes"
> >
> > The only difference in attributes is target-features and target-cpu though,
> > so I don't know what causes this.
> Right, the this-adjustment is definitely unnecessary for this kind of CPU
> dispatch thunk.
>
> The important thing is that marking the call as `musttail` ensures that you
> get perfect forwarding of all arguments, even varargs and inalloca. Maybe
> it's enough to just mark the call you are creating that way. I think the code
> you are generating obeys all the musttail invariants, so that might be best.
>
> The "cannot guarantee tail call due to mismatched ABI impacting function
> attributes" verifier check is only supposed to fire when parameter attributes
> like `byval` and `inreg` differ between the call site and the dispatcher
> function. It shouldn't care about target-cpu or target-features.
It's probably worth adding a test that uses inalloca, since that's why we're
doing this nonsense. It would look like:
```
struct Foo {
Foo();
Foo(const Foo &o);
~Foo();
int x;
};
int __attribute__((target("default"))) bar(Foo o) { return o.x; }
int __attribute__((target("sse4.2"))) bar(Foo o) { return o.x + 1; }
int __attribute__((target("arch=ivybridge"))) bar(Foo o) { return o.x + 2; }
```
Targetting i686-windows-msvc.
https://reviews.llvm.org/D53586
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
