From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126855 invoked by alias); 17 Nov 2015 19:53:10 -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 126842 invoked by uid 89); 17 Nov 2015 19:53:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eusmtp01.atmel.com Received: from eusmtp01.atmel.com (HELO eusmtp01.atmel.com) (212.144.249.243) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 17 Nov 2015 19:53:07 +0000 Received: from HNOCHT02.corp.atmel.com (10.161.30.162) by eusmtp01.atmel.com (10.161.101.31) with Microsoft SMTP Server (TLS) id 14.3.235.1; Tue, 17 Nov 2015 20:53:00 +0100 Received: from jaguar.atmel.com (10.161.30.18) by HNOCHT02.corp.atmel.com (10.161.30.162) with Microsoft SMTP Server (TLS) id 14.3.235.1; Tue, 17 Nov 2015 20:53:02 +0100 Date: Tue, 17 Nov 2015 19:53:00 -0000 From: Senthil Kumar Selvaraj To: Richard Biener CC: , Subject: Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode Message-ID: <20151117195241.GA5579@jaguar.atmel.com> References: <20151114071148.GA16647@jaguar.atmel.com> <20151114084928.GA749@jaguar.atmel.com> <595F0CF8-27C1-4A63-8339-F70CB5E76CAB@suse.de> <20151114181044.GA1297@jaguar.atmel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02135.txt.bz2 On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote: > 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. How does the below patch look? Bootstrapped, but not regtested yet. The testcase was rather tricky to write - I wasn't sure how to reliably get a type bigger than a word for all targets. I resorted to __int128, not sure it's a good idea though - I should probably add dg-skip-if for targets that don't support that. Do you know of a better way to write that? Regards Senthil diff --git gcc/testsuite/gcc.dg/tree-ssa/vrp98.c gcc/testsuite/gcc.dg/tree-ssa/vrp98.c new file mode 100644 index 0000000..448ceba --- /dev/null +++ gcc/testsuite/gcc.dg/tree-ssa/vrp98.c @@ -0,0 +1,40 @@ +/* { dg-do compile } */ +/* { dg-options "-Os -fdump-tree-vrp1-details" } */ + +#include +#include + +typedef unsigned int word __attribute__((mode(word))); +typedef unsigned __int128 bigger_than_word; + +int +foo (bigger_than_word a, word b, uint8_t c) +{ + /* Must fold use of t1 into use of b, as b is no wider than word_mode. */ + const uint8_t t1 = b % UCHAR_MAX; + + /* Must NOT fold use of t2 into use of a, as a is wider than word_mode. */ + const uint8_t t2 = a % UCHAR_MAX; + + /* Must fold use of t3 into use of c, as c is narrower than t3. */ + const uint32_t t3 = (const uint32_t)(c >> 1); + + uint16_t ret = 0; + + if (t1 == 1) + ret = 20; + else if (t2 == 2) + ret = 30; + else if (t3 == 3) + ret = 40; + /* Th extra condition below is necessary to prevent a prior pass from + folding away the cast. Ignored in scan-tree-dump. */ + else if (t3 == 4) + ret = 50; + + return ret; +} + +/* { dg-final { scan-tree-dump "Folded into: if \\(_\[0-9\]+ == 1\\)" "vrp1" } } */ +/* { dg-final { scan-tree-dump-not "Folded into: if \\(_\[0-9\]+ == 2\\)" "vrp1" } } */ +/* { dg-final { scan-tree-dump "Folded into: if \\(_\[0-9\]+ == 3\\)" "vrp1" } } */ diff --git gcc/tree-vrp.c gcc/tree-vrp.c index 5d085b4..4513b88 100644 --- gcc/tree-vrp.c +++ gcc/tree-vrp.c @@ -9467,7 +9467,8 @@ 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)) + && desired_pro_or_demotion_p (innerop, op0)) { value_range *vr = get_value_range (innerop); diff --git gcc/tree.h gcc/tree.h index 0b9c3b9..91099ed 100644 --- gcc/tree.h +++ gcc/tree.h @@ -5316,4 +5316,18 @@ get_decl_source_range (tree decl) return get_range_from_loc (line_table, loc); } +/* Return true if it makes sense to promote/demote from_type to to_type. */ +inline bool +desired_pro_or_demotion_p (tree to_type, tree from_type) +{ + unsigned int to_type_precision = TYPE_PRECISION (TREE_TYPE (to_type)); + + /* OK to promote if to_type is no bigger than word_mode. */ + if (to_type_precision <= GET_MODE_PRECISION (word_mode)) + return true; + + /* Otherwise, allow only if narrowing or same precision conversions. */ + return to_type_precision <= TYPE_PRECISION (TREE_TYPE (from_type)); +} + #endif /* GCC_TREE_H */ > > 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)