From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69245 invoked by alias); 16 Nov 2015 09:02:21 -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 69233 invoked by uid 89); 16 Nov 2015 09:02:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Mon, 16 Nov 2015 09:02:19 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 65C77AC27; Mon, 16 Nov 2015 09:01:53 +0000 (UTC) Date: Mon, 16 Nov 2015 09:02:00 -0000 From: Richard Biener To: Senthil Kumar Selvaraj cc: gcc-patches@gcc.gnu.org, law@redhat.com Subject: Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode In-Reply-To: <20151114181044.GA1297@jaguar.atmel.com> Message-ID: References: <20151114071148.GA16647@jaguar.atmel.com> <20151114084928.GA749@jaguar.atmel.com> <595F0CF8-27C1-4A63-8339-F70CB5E76CAB@suse.de> <20151114181044.GA1297@jaguar.atmel.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2015-11/txt/msg01906.txt.bz2 On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote: > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj wrote: > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > >> > > >> >This patch came out of a discussion held in the gcc mailing list > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > >> > > > >> >The patch restricts folding of conditional exprs with lhs previously > > >> >set by a type conversion to occur only if the source of the type > > >> >conversion's mode is word mode or smaller. > > >> > > > >> >Bootstrapped and reg tested on x86_64 (with > > >--enable-languages=c,c++). > > >> > > > >> >If ok, could you commit please? I don't have commit access. > > >> > > > >> >Regards > > >> >Senthil > > >> > > > >> >gcc/ChangeLog > > >> > > > >> >2015-11-11 Senthil Kumar Selvaraj > > > > > >> > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > >> > if innerop's mode is word_mode or smaller. > > >> > > > >> > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > > >> >index e2393e4..c139bc6 100644 > > >> >--- gcc/tree-vrp.c > > >> >+++ gcc/tree-vrp.c > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > >> > innerop = gimple_assign_rhs1 (def_stmt); > > >> > > > >> > if (TREE_CODE (innerop) == SSA_NAME > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > > >> >+ <= GET_MODE_SIZE(word_mode)) > > >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > >> > { > > >> > value_range *vr = get_value_range (innerop); > > >> > > >> I thought the result of the discussion was that the transformation is > > >ok if > > >> either it is narrowing or it widens but to something no bigger than > > >> word_mode. So you should have 2 comparisons, or 1 with a max. > > > > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed > > >"widening upto word-mode", not the narrowing. > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. > > My bad. Here's a revised patch that checks for both conditions, using > max as Marc suggested to limit to word_mode or narrowing conversions. > > Bootstrapped and regtested for x86_64 with c and c++. > > Is this ok? If yes, would you commit it > for me please? I don't have commit access. > > gcc/ChangeLog > 2015-11-14 Senthil Kumar Selvaraj > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > if innerop's mode smaller or equal to word_mode or op0's mode. > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > index e2393e4..cfd90e7 100644 > --- a/gcc/tree-vrp.c > +++ b/gcc/tree-vrp.c > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt) > innerop = gimple_assign_rhs1 (def_stmt); > > if (TREE_CODE (innerop) == SSA_NAME > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop))) > + <= std::max (GET_MODE_SIZE (word_mode), > + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))) Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and add a comment as to what we are testing here and why. Btw, ideally we'd factor out a bool desired_pro_or_demotion_p (tree to_type, tree from_type) {} function somewhere as we have similar tests throughout the compiler that we might want to unify (and also have a central place to eventually add a target hook if ever desired). In fact in other places we also check that the type we promote/demote to matches its mode precision or the type we promote/demote from already does not. I'd suggest tree.[ch] for that function. Please also add a testcase. Thanks, Richard. > { > value_range *vr = get_value_range (innerop); > > > Regards > Senthil > > > > Richard. > > > > >Richard? > > > > > >Regards > > >Senthil > > >> > > >> -- > > >> Marc Glisse > > > > > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)