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

[Jan Hubicka <hubicka@ucw.cz>]

> 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.

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);
}

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..

Honza
> 
> 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)