From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96789 invoked by alias); 13 May 2015 20:02:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 96777 invoked by uid 89); 13 May 2015 20:02:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 13 May 2015 20:02:29 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 2611F544199; Wed, 13 May 2015 22:02:26 +0200 (CEST) Date: Wed, 13 May 2015 20:04:00 -0000 From: Jan Hubicka To: Alexander Monakov Cc: Jan Hubicka , Jeff Law , Uros Bizjak , "gcc-patches@gcc.gnu.org" , Rich Felker Subject: Re: [PATCH i386] Extend sibcall peepholes to allow source in %eax Message-ID: <20150513200225.GA20903@kam.mff.cuni.cz> References: <1430757479-14241-1-git-send-email-amonakov@ispras.ru> <1430757479-14241-4-git-send-email-amonakov@ispras.ru> <20150510165402.GH9659@atrey.karlin.mff.cuni.cz> <20150511180038.GA22960@kam.mff.cuni.cz> <55510774.5030107@redhat.com> <20150511201555.GA49471@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2015-05/txt/msg01268.txt.bz2 > 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" } } */