On 05/11/2015 01:46 PM, Uros Bizjak wrote:
On Mon, May 11, 2015 at 8:00 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
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.
This is from [1], and reading this reference, it looks to me that the
check was introduced due to:
- Adds check that eliminated register is really dead after the call
(maybe an overkill, but some hard-to-debug problems surfaced due to
missing liveness checks in the past)
Going down that memory lane, it looks like a safety check for
something that *might* happen. Looking at the comment, I'd say we can
remove the check, but we should look for possible fallout.
I'd tend to agree.
jeff
