From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2602 invoked by alias); 25 Jun 2014 07:50:15 -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 2592 invoked by uid 89); 25 Jun 2014 07:50:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 25 Jun 2014 07:50:14 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5P7oBDN015135 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 25 Jun 2014 03:50:12 -0400 Received: from tucnak.zalov.cz (ovpn-116-32.ams2.redhat.com [10.36.116.32]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5P7o93P020167 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 25 Jun 2014 03:50:10 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.8/8.14.7) with ESMTP id s5P7o7Z4028505; Wed, 25 Jun 2014 09:50:08 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.8/8.14.8/Submit) id s5P7o5a4028504; Wed, 25 Jun 2014 09:50:05 +0200 Date: Wed, 25 Jun 2014 07:50:00 -0000 From: Jakub Jelinek To: Kugan Cc: Richard Henderson , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH 1/2] Enable setting sign and unsigned promoted mode (SPR_SIGNED_AND_UNSIGNED) Message-ID: <20140625075005.GY31640@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <53A9658F.2070304@linaro.org> <53A96657.1030901@linaro.org> <20140624121812.GW31640@tucnak.redhat.com> <53AA7864.9020500@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <53AA7864.9020500@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg01972.txt.bz2 On Wed, Jun 25, 2014 at 05:21:08PM +1000, Kugan wrote: > The problem with SRP_POINTER 0, SRP_SIGNED 1, SRP_UNSIGNED 2, > SRP_SIGNED_AND_UNSIGNED 3 (as I understand) is that, it will be > incompatible with TYPE_UNSIGNED (tree) and defines of > POINTER_EXTEND_UNSIGNED values. We will have to then translate while > setting to SRP_* values . Also SUBREG_PROMOTED_SIGNED_P is now checked > in some cases for != 0 (meaning SRP_POINTER or SRP_UNSIGNED) and in some > cases > 0 (meaning SRP_UNSIGNED). > > Since our aim is to perform single bit checks, why don’t we just use > this representation internally (i.e. _rtx->unchanging = 1 if SRP_SIGNED > and _rtx->volatil = 1 if SRP_UNSIGNED). As for SUBREG_PROMOTED_SIGNED_P, > we still have to return -1 or 1 depending on SRP_POINTER or SRP_UNSIGNED. Why don't you make SUBREG_PROMOTED_UNSIGNED_P just return 0/1 (i.e. the single bit), and for places where it would like to match both SRP_UNSIGNED and SRP_POINTER use SUBREG_PROMOTED_GET () & SRP_UNSIGNED or so? > --- a/gcc/ifcvt.c > +++ b/gcc/ifcvt.c > @@ -1448,8 +1448,11 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, > || byte_vtrue != byte_vfalse > || (SUBREG_PROMOTED_VAR_P (vtrue) > != SUBREG_PROMOTED_VAR_P (vfalse)) > - || (SUBREG_PROMOTED_UNSIGNED_P (vtrue) > - != SUBREG_PROMOTED_UNSIGNED_P (vfalse))) > + || ((SUBREG_PROMOTED_UNSIGNED_P (vtrue) > + != SUBREG_PROMOTED_UNSIGNED_P (vfalse)) > + && (SUBREG_PROMOTED_SIGNED_P (vtrue) > + != SUBREG_PROMOTED_SIGNED_P (vfalse)))) Shouldn't this be SUBREG_PROMOTED_GET (vtrue) != SUBREG_PROMOTED_GET (vfalse) ? > +const unsigned int SRP_POINTER = -1; > +const unsigned int SRP_SIGNED = 0; Inconsistent whitespace, just use space instead of multiple spaces and/or tabs. > +const unsigned int SRP_UNSIGNED = 1; > +const unsigned int SRP_SIGNED_AND_UNSIGNED = 2; > +/* Predicate to check if RTX of SUBREG_PROMOTED_VAR_P() is promoted > + for SIGNED type. */ > +#define SUBREG_PROMOTED_SIGNED_P(RTX) \ > + (RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_SIGNED_P", (RTX), SUBREG)->unchanging == 1) Why the " == 1" ? > + > +/* Predicate to check if RTX of SUBREG_PROMOTED_VAR_P() is promoted > + for UNSIGNED type. In case of SRP_POINTER, SUBREG_PROMOTED_UNSIGNED_P > + returns -1 as this is in most cases handled like unsigned extension, > + except for generating instructions where special code is emitted for > + (ptr_extend insns) on some architectures. */ > #define SUBREG_PROMOTED_UNSIGNED_P(RTX) \ > - ((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P", (RTX), SUBREG)->volatil) \ > - ? -1 : (int) (RTX)->unchanging) > + ((((RTL_FLAG_CHECK1 ("SUBREG_PROMOTED_UNSIGNED_P", (RTX), SUBREG)->volatil)\ > + + (RTX)->unchanging) == 0) ? -1 : ((RTX)->volatil == 1)) > + > +/* Checks if RTX of SUBREG_PROMOTED_VAR_P() is promotd for given SIGN. */ > +#define SUBREG_CHECK_PROMOTED_SIGN(RTX, SIGN) \ Use space rather than tab. Also, why do we need this macro? Can't you just use SUBREG_PROMOTED_GET () == sign ? I mean, sign in that case is typically just 0 or 1. Jakub