From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24745 invoked by alias); 10 Feb 2015 01:57:28 -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 24726 invoked by uid 89); 10 Feb 2015 01:57:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f169.google.com Received: from mail-lb0-f169.google.com (HELO mail-lb0-f169.google.com) (209.85.217.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 10 Feb 2015 01:57:26 +0000 Received: by mail-lb0-f169.google.com with SMTP id f15so33432518lbj.0 for ; Mon, 09 Feb 2015 17:57:22 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.152.28.227 with SMTP id e3mr20408556lah.117.1423533442652; Mon, 09 Feb 2015 17:57:22 -0800 (PST) Received: by 10.25.21.213 with HTTP; Mon, 9 Feb 2015 17:57:22 -0800 (PST) In-Reply-To: <00f001d044d4$23f37e20$6bda7a60$@arm.com> References: <00f001d044d4$23f37e20$6bda7a60$@arm.com> Date: Tue, 10 Feb 2015 01:57:00 -0000 Message-ID: Subject: Re: [PATCH 1/2, combine] Try REG_EQUAL for nonzero_bits From: Andrew Pinski To: "Thomas Preud'homme" Cc: Eric Botcazou , GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-02/txt/msg00596.txt.bz2 On Mon, Feb 9, 2015 at 5:51 PM, Thomas Preud'homme wrote: > Hi Eric, > > I'm taking over Zhenqiang's work on this. Comments and updated patch > below. > >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- >> owner@gcc.gnu.org] On Behalf Of Eric Botcazou >> > + rtx reg_equal = insn ? find_reg_equal_equiv_note (insn) : NULL_RTX; >> > + unsigned HOST_WIDE_INT bits = nonzero_bits (src, >> nonzero_bits_mode); >> >> Note that "src" has taken the SHORT_IMMEDIATES_SIGN_EXTEND path >> here. >> >> > + if (reg_equal) >> > + { >> > + unsigned int num = num_sign_bit_copies (XEXP (reg_equal, 0), >> > + GET_MODE (x)); >> > + bits &= nonzero_bits (XEXP (reg_equal, 0), nonzero_bits_mode); >> >> But XEXP (reg_equal, 0) hasn't here. If we want to treat the datum of >> the >> REG_EQUAL or REG_EQUIV note as equivalent to the SET_SRC (set), and >> I think we >> should (see for example combine.c:9650), there is a problem. >> >> So I think we should create a new function, something along of: >> >> /* If MODE has a precision lower than PREC and SRC is a non-negative >> constant >> that would appear negative in MODE, sign-extend SRC for use in >> nonzero_bits >> because some machines (maybe most) will actually do the sign- >> extension and >> this is the conservative approach. >> >> ??? For 2.5, try to tighten up the MD files in this regard >> instead of this kludge. */ >> >> rtx >> sign_extend_short_imm (rtx src, machine_mode mode, unsigned int >> prec) >> { >> if (GET_MODE_PRECISION (mode) < prec >> && CONST_INT_P (src) >> && INTVAL (src) > 0 >> && val_signbit_known_set_p (mode, INTVAL (src))) >> src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (mode)); >> >> return src; >> } >> >> and calls it from combine.c:1702 >> >> #ifdef SHORT_IMMEDIATES_SIGN_EXTEND >> src = sign_extend_short_imm (src, GET_MODE (x), BITS_PER_WORD); >> #endif >> >> and from combine.c:9650 >> >> #ifdef SHORT_IMMEDIATES_SIGN_EXTEND >> tem = sign_extend_short_imm (tem, GET_MODE (x), >> GET_MODE_PRECISION (mode)); >> #endif >> >> Once this is done, the same thing needs to be applied to XEXP >> (reg_equal, 0) >> before it is sent to nonzero_bits. > > I did this as you suggested and decided to split the patch in 2 to make it easier > to review. Part 1 does this reorganization while part 2 concern the REG_EQUAL > matter. > > ChangeLog entry for part 1 is as follows: > > *** gcc/ChangeLog *** > > 2015-02-09 Thomas Preud'homme > > * combine.c (sign_extend_short_imm): New. > (set_nonzero_bits_and_sign_copies): Use above new function for sign > extension of src short immediate. > (reg_nonzero_bits_for_combine): Likewise for tem. > > diff --git a/gcc/combine.c b/gcc/combine.c > index ad3bed0..f2b26c2 100644 > --- a/gcc/combine.c > +++ b/gcc/combine.c > @@ -1640,6 +1640,26 @@ setup_incoming_promotions (rtx_insn *first) > } > } > > +#ifdef SHORT_IMMEDIATES_SIGN_EXTEND > +/* If MODE has a precision lower than PREC and SRC is a non-negative constant > + that would appear negative in MODE, sign-extend SRC for use in nonzero_bits > + because some machines (maybe most) will actually do the sign-extension and > + this is the conservative approach. > + > + ??? For 2.5, try to tighten up the MD files in this regard instead of this > + kludge. */ I don't know if this has been mentioned and even though you are just copying a comment from below but would it make sense to look fixing what the comment says we should look at after GCC 2.5 (which was over 20 years ago)? Or maybe just remove the comment if it no longer applies. Thanks, Andrew > + > +static rtx > +sign_extend_short_imm (rtx src, machine_mode mode, unsigned int prec) > +{ > + if (GET_MODE_PRECISION (mode) < prec && CONST_INT_P (src) > + && INTVAL (src) > 0 && val_signbit_known_set_p (mode, INTVAL (src))) > + src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (mode)); > + > + return src; > +} > +#endif > + > /* Called via note_stores. If X is a pseudo that is narrower than > HOST_BITS_PER_WIDE_INT and is being set, record what bits are known zero. > > @@ -1719,20 +1739,7 @@ set_nonzero_bits_and_sign_copies (rtx x, const_rtx set, void *data) > rtx src = SET_SRC (set); > > #ifdef SHORT_IMMEDIATES_SIGN_EXTEND > - /* If X is narrower than a word and SRC is a non-negative > - constant that would appear negative in the mode of X, > - sign-extend it for use in reg_stat[].nonzero_bits because some > - machines (maybe most) will actually do the sign-extension > - and this is the conservative approach. > - > - ??? For 2.5, try to tighten up the MD files in this regard > - instead of this kludge. */ > - > - if (GET_MODE_PRECISION (GET_MODE (x)) < BITS_PER_WORD > - && CONST_INT_P (src) > - && INTVAL (src) > 0 > - && val_signbit_known_set_p (GET_MODE (x), INTVAL (src))) > - src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (GET_MODE (x))); > + src = sign_extend_short_imm (src, GET_MODE (x), BITS_PER_WORD); > #endif > > /* Don't call nonzero_bits if it cannot change anything. */ > @@ -9770,20 +9777,8 @@ reg_nonzero_bits_for_combine (const_rtx x, machine_mode mode, > if (tem) > { > #ifdef SHORT_IMMEDIATES_SIGN_EXTEND > - /* If X is narrower than MODE and TEM is a non-negative > - constant that would appear negative in the mode of X, > - sign-extend it for use in reg_nonzero_bits because some > - machines (maybe most) will actually do the sign-extension > - and this is the conservative approach. > - > - ??? For 2.5, try to tighten up the MD files in this regard > - instead of this kludge. */ > - > - if (GET_MODE_PRECISION (GET_MODE (x)) < GET_MODE_PRECISION (mode) > - && CONST_INT_P (tem) > - && INTVAL (tem) > 0 > - && val_signbit_known_set_p (GET_MODE (x), INTVAL (tem))) > - tem = GEN_INT (INTVAL (tem) | ~GET_MODE_MASK (GET_MODE (x))); > + tem = sign_extend_short_imm (tem, GET_MODE (x), > + GET_MODE_PRECISION (mode)); > #endif > return tem; > } > > Is this ok for next stage1? > > Best regards, > > Thomas > > >