> On Sun, 10 May 2015, Jan Hubicka wrote:
>
> > > On i386, peepholes that transform memory load and register-indirect jump
> > > into
> > > memory-indirect jump are overly restrictive in that they don't allow
> > > combining
> > > when the jump target is loaded into %eax, and the called function returns
> > > a
> > > value (also in %eax, so it's not dead after the call). Fix this by
> > > checking
> > > for same source and output register operands separately.
> > >
> > > OK?
> > > * config/i386/i386.md (sibcall_value_memory): Extend peepholes to
> > > allow memory address in %eax.
> > > (sibcall_value_pop_memory): Likewise.
> >
> > Why do we need the check for liveness after all? There is SIBLING_CALL_P
> > (peep2_next_insn (1)) so we know that the function terminates by the call
> > and there are no other uses of the value.
>
> Indeed. Uros, the peep2_reg_dead_p check was added by your patch as svn
> revision 211776, git commit e51f8b8fed. Would you agree that the check is not
> necessary for sibcalls as Honza explains? Would you approve a patch that
> removes it in the sibcall peepholes I modify in the patch under discussion?
>
> > Don't we however need to check that operands[0] is not used by the
> > call_insn as
> > parameter of the call? I.e. something like
> >
> > void
> > test(void (*callback ()))
> > {
> > callback(callback);
> > }
>
> You need a pointer-to-pointer-to-function to trigger the peephole. Something
> like this:
>
> void foo()
> {
> void (**bar)(void*);
> asm("":"=r"(bar));
> (*bar)(*bar);
> }
>
> > I think instead of peep2_reg_dead_p we want to check that the parameter is
> > not in
> > CALL_INSN_FUNCTION_USAGE of the sibcall..
>
> Playing with the above testcase I can't induce failure. It seems today GCC
> won't allocate the same register as callee address and one of the arguments.
> Do you want me to implement such a check anyway?
Hmm, only way I can trigger same register is:
void foo()
{
void (**bar)(void*);
asm("":"=r"(bar));
register void (*var)(void *) asm("%eax");
var=*bar;
asm("":"+r"(var));
var(var);
}
removing the second asm makes CSE to forward propagae the memory operand
to call that makes the call different from the register variable.
Still I would check for that, but this is more Uros' area.
Honza
