Re: [PATCH] Fix incorrect devirtualization (PR middle-end/48661)

[Richard Guenther <rguenther at suse dot de>]

On Tue, 19 Apr 2011, Jan Hubicka wrote:

> > I thought the idea was to use __builtin_va_arg_pack and friends.
> > Of course the inliner would still need to know how to inline such
> > a va-arg forwarder, and we would need a way to expand them (well,
> > or just go the existing special casing).  We might need this
> > kind of inliner support anyway because of the partial inlining
> > opportunity in libquantum which is for a varargs function like
> > 
> > void foo (...)
> > {
> >   if (always-zero-static)
> >     return;
> > 
> >   ...
> > }
> > 
> > thus partial inlining would create such a forwarding call.
> 
> Yep, I know this is related issue.  We have builtlin_apply and
> builtin_va_arg_pack. Neither of those solves the problem here.
> 
> With __builtin_va_arg_pack the thunk would be something like:
> int b(int *this, ...);
> 
> int a(int *this, ...)
> { 
>   return b(this+1, __builtin_va_arg_pack ());
> }
> and similarly for your libquantum testcase. However one gets:
> jh@gcc10:~/trunk/build/gcc$ gcc -O2 t.c
> t.c: In function 'a':
> t.c:5: error: invalid use of '__builtin_va_arg_pack ()'
> becuase va_arg_pack works only when eliminated by the inliner.
> 
> Consequentely thunks would be forced to be always inline and this contradits
> their purpose to be addressed from virtual tables.
> 
> So we would need to develop equivalent of va_arg_pack that works
> when not inlining. It can not be implemented generally, only for tail
> calls and making nice interface for this seems tricky.
> 
> __builtin_apply don't fit here because it 
>  1) produces lousy code
>  2) requires knowledge about size of argument block
>  3) doesn't let you to modify one of arguments.
> 
> We could go by deriving __builtlin_return into somethign like:
> 
> int a(int *this, ...)
> { 
>   __builtin_return_va_arg_pack (b, this + 1)
> }
> 
> that would have semantic of tail calling the function with the same arguments as the function was called with
> with overwritting the arguments it is given that would be required to match the first few named arguments
> of the function.
> 
> Since this construct is departed enough from gimple_call we probably would want
> to have something like gimple_tailcall_va_arg_pack or so.  We will need to
> develop expand machinery - I am not sure one can implement expander of this
> without introducing new macros. Maybe it is - it essentially means to load
> argument registers from original argument registers and tailcall and even for
> funnier architectures I think one can just copy what calls.c does already for
> tailcalls with the slight semantics changes...
> I am not really aware with all details of all calling conventions we support to
> be sure however.
> 
> I am not really sure if it is really prettier than the irritating special case
> in cgraph, however?

Well, even for the partial inlining case I'd devise a scheme that
if a call to such a forwarder remains it gets expanded as a call to
the original (non-split) function (similar to emitting a call to
the asm-thunk instead of expanding the gimple representation of the 
thunk).  I realize this is another special case, but I like it more
as it appears to be more flexible to me ;)  (it's similar to
the redefined extern inline function case, one function body for
inlining and another one for call targets)

> > But well, it's just my random 2 cents on the issue ;)
> :)
:)

Richard.