Re: [PATCH i386] Extend sibcall peepholes to allow source in %eax

[Alexander Monakov <amonakov@ispras.ru>]

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?

> > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> > index 729db75..7f81bcc 100644
> > --- a/gcc/config/i386/i386.md
> > +++ b/gcc/config/i386/i386.md
> > @@ -11872,13 +11872,14 @@
> >    [(set (match_operand:W 0 "register_operand")
> >  	(match_operand:W 1 "memory_operand"))
> >     (set (match_operand 2)
> >     (call (mem:QI (match_dup 0))
> >  		 (match_operand 3)))]
> >    "!TARGET_X32 && SIBLING_CALL_P (peep2_next_insn (1))
> > -   && peep2_reg_dead_p (2, operands[0])"
> > +   && (REGNO (operands[2]) == REGNO (operands[0])
> > +       || peep2_reg_dead_p (2, operands[0]))"
> >    [(parallel [(set (match_dup 2)
> >  		   (call (mem:QI (match_dup 1))
> >  			 (match_dup 3)))
> >  	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
> >  
> >  (define_peephole2
> > @@ -11886,13 +11887,14 @@
> >  	(match_operand:W 1 "memory_operand"))
> >     (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
> >     (set (match_operand 2)
> >  	(call (mem:QI (match_dup 0))
> >  	      (match_operand 3)))]
> >    "!TARGET_X32 && SIBLING_CALL_P (peep2_next_insn (2))
> > -   && peep2_reg_dead_p (3, operands[0])"
> > +   && (REGNO (operands[2]) == REGNO (operands[0])
> > +       || peep2_reg_dead_p (3, operands[0]))"
> >    [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
> >     (parallel [(set (match_dup 2)
> >  		   (call (mem:QI (match_dup 1))
> >  			 (match_dup 3)))
> >  	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
> >  
> > @@ -11951,13 +11953,14 @@
> >  		   (call (mem:QI (match_dup 0))
> >  			 (match_operand 3)))
> >  	      (set (reg:SI SP_REG)
> >  		   (plus:SI (reg:SI SP_REG)
> >  			    (match_operand:SI 4 "immediate_operand")))])]
> >    "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))
> > -   && peep2_reg_dead_p (2, operands[0])"
> > +   && (REGNO (operands[2]) == REGNO (operands[0])
> > +       || peep2_reg_dead_p (2, operands[0]))"
> >    [(parallel [(set (match_dup 2)
> >  		   (call (mem:QI (match_dup 1))
> >  			 (match_dup 3)))
> >  	      (set (reg:SI SP_REG)
> >  		   (plus:SI (reg:SI SP_REG)
> >  			    (match_dup 4)))
> > @@ -11971,13 +11974,14 @@
> >  		   (call (mem:QI (match_dup 0))
> >  			 (match_operand 3)))
> >  	      (set (reg:SI SP_REG)
> >  		   (plus:SI (reg:SI SP_REG)
> >  			    (match_operand:SI 4 "immediate_operand")))])]
> >    "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))
> > -   && peep2_reg_dead_p (3, operands[0])"
> > +   && (REGNO (operands[2]) == REGNO (operands[0])
> > +       || peep2_reg_dead_p (3, operands[0]))"
> >    [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
> >     (parallel [(set (match_dup 2)
> >  		   (call (mem:QI (match_dup 1))
> >  			 (match_dup 3)))
> >  	      (set (reg:SI SP_REG)
> >  		   (plus:SI (reg:SI SP_REG)
>