This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH i386] Extend sibcall peepholes to allow source in %eax
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: Alexander Monakov <amonakov at ispras dot ru>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, Jeff Law <law at redhat dot com>, Uros Bizjak <ubizjak at gmail dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Rich Felker <dalias at libc dot org>
- Date: Wed, 13 May 2015 22:02:25 +0200
- Subject: Re: [PATCH i386] Extend sibcall peepholes to allow source in %eax
- Authentication-results: sourceware.org; auth=none
- References: <1430757479-14241-1-git-send-email-amonakov at ispras dot ru> <1430757479-14241-4-git-send-email-amonakov at ispras dot ru> <20150510165402 dot GH9659 at atrey dot karlin dot mff dot cuni dot cz> <alpine dot LNX dot 2 dot 11 dot 1505112039590 dot 22867 at monopod dot intra dot ispras dot ru> <20150511180038 dot GA22960 at kam dot mff dot cuni dot cz> <CAFULd4aVBT6iLR4DZtAFRqYSg3ERzD5wKLRMgbN4K-X07sfSag at mail dot gmail dot com> <55510774 dot 5030107 at redhat dot com> <20150511201555 dot GA49471 at kam dot mff dot cuni dot cz> <alpine dot LNX dot 2 dot 11 dot 1505132122350 dot 22867 at monopod dot intra dot ispras dot ru>
> 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" } } */