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

[Jan Hubicka <hubicka@ucw.cz>]

> On Mon, 11 May 2015, Jan Hubicka wrote:
> > Yes, to make my original email clear, I think we are safe to remove
> > peep2_reg_dead_p.
> > 
> > I would however introduce a check that the call target is not also among
> > parameters of the function. In this case the peephole would remove the load
> > and make the parameter unefined.
> > 
> > While current mainline don't seem to be able to translate the testcase above
> > that way, perhaps future improvements to LRA/postreload gcse may make it happen
> > and generally RTL patterns are better to be safe by definition not
> > only for the actual RTL we are able to generate. I suppose reg_mentioned_p
> > on call usage is enough.
> 
> Thanks.  I have bootstrapped and regtested the following patch.  OK?
> 
> 	* config/i386/i386.md (sibcall_memory): Check that register with
> 	callee address is not also used as one of the arguments, instead
> 	of checking that it is not live after the sibcall.
> 	(sibcall_pop_memory): Ditto.
> 	(sibcall_value_memory): Ditto.
> 	(sibcall_value_pop_memory): Ditto.
> testsuite:
> 	* gcc.target/i386/sibcall-7.c: New test.

Thank you! This looks fine.  Please add also the testcase that should break if
the new test was wrong andosmeone fixed postreload to allow use of the same register
this check will prevent wrong code?

OK with that change.
Honza
> 
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 0959aef..9c1aa7d 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -11673,7 +11673,8 @@
>     (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (1)))"
>    [(parallel [(call (mem:QI (match_dup 1))
>                     (match_dup 3))
>               (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
> @@ -11685,7 +11686,8 @@
>     (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (2)))"
>    [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
>     (parallel [(call (mem:QI (match_dup 1))
>                     (match_dup 3))
> @@ -11744,7 +11746,8 @@
>                    (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (1)))"
>    [(parallel [(call (mem:QI (match_dup 1))
>                     (match_dup 3))
>               (set (reg:SI SP_REG)
> @@ -11762,7 +11765,8 @@
>                    (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (2)))"
>    [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
>     (parallel [(call (mem:QI (match_dup 1))
>                     (match_dup 3))
> @@ -11838,7 +11842,8 @@
>     (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (1)))"
>    [(parallel [(set (match_dup 2)
>                    (call (mem:QI (match_dup 1))
>                          (match_dup 3)))
> @@ -11852,7 +11857,8 @@
>        (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (2)))"
>    [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
>     (parallel [(set (match_dup 2)
>                    (call (mem:QI (match_dup 1))
> @@ -11917,7 +11923,8 @@
>                    (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (1)))"
>    [(parallel [(set (match_dup 2)
>                    (call (mem:QI (match_dup 1))
>                          (match_dup 3)))
> @@ -11937,7 +11944,8 @@
>                    (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])"
> +   && !reg_mentioned_p (operands[0],
> +                       CALL_INSN_FUNCTION_USAGE (peep2_next_insn (2)))"
>    [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
>     (parallel [(set (match_dup 2)
>                    (call (mem:QI (match_dup 1))
> diff --git a/gcc/testsuite/gcc.target/i386/sibcall-7.c b/gcc/testsuite/gcc.target/i386/sibcall-7.c
> index e69de29..72fdaff 100644
> --- a/gcc/testsuite/gcc.target/i386/sibcall-7.c
> +++ b/gcc/testsuite/gcc.target/i386/sibcall-7.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile { target { { ! x32 } } } } */
> +/* { dg-options "-O2" } */
> +
> +int foo()
> +{
> +  int (**bar)(void);
> +  asm("":"=a"(bar));
> +  return (*bar)();
> +}
> +
> +/* { dg-final { scan-assembler-not "mov" } } */