From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116775 invoked by alias); 11 May 2015 17:50:09 -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 116723 invoked by uid 89); 11 May 2015 17:50:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: smtp.ispras.ru Received: from smtp.ispras.ru (HELO smtp.ispras.ru) (83.149.199.79) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 11 May 2015 17:50:07 +0000 Received: from [10.10.3.121] (unknown [83.149.199.91]) by smtp.ispras.ru (Postfix) with ESMTP id 4F792214EC; Mon, 11 May 2015 20:50:04 +0300 (MSK) Date: Mon, 11 May 2015 17:50:00 -0000 From: Alexander Monakov To: Jan Hubicka cc: gcc-patches@gcc.gnu.org, Uros Bizjak , Rich Felker Subject: Re: [PATCH i386] Extend sibcall peepholes to allow source in %eax In-Reply-To: <20150510165402.GH9659@atrey.karlin.mff.cuni.cz> Message-ID: 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> User-Agent: Alpine 2.11 (LNX 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2015-05/txt/msg00977.txt.bz2 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) >