From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7684 invoked by alias); 9 Aug 2017 20:47:13 -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 7464 invoked by uid 89); 9 Aug 2017 20:46:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=08am, 08AM X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Aug 2017 20:46:57 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id v79Kkn35018483; Wed, 9 Aug 2017 15:46:49 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id v79KkjbK018463; Wed, 9 Aug 2017 15:46:45 -0500 Date: Wed, 09 Aug 2017 21:06:00 -0000 From: Segher Boessenkool To: Will Schmidt Cc: GCC Patches , Richard Biener , Bill Schmidt , David Edelsohn Subject: Re: [PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding Message-ID: <20170809204644.GX13471@gate.crashing.org> References: <1502226896.6577.12.camel@brimstone.rchland.ibm.com> <1502289548.6577.39.camel@brimstone.rchland.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1502289548.6577.39.camel@brimstone.rchland.ibm.com> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00702.txt.bz2 On Wed, Aug 09, 2017 at 09:39:08AM -0500, Will Schmidt wrote: > I also fixed the (missing) space after > cast for the existing debug code. Thanks for that :-) > * config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb > to indicate when early gimple folding has been disabled. The colon goes after the closing parenthesis. > (rs6000_invalid_builtin): whitespace. Capital W. Or, say what this does? "Fix whitespace." > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 1fb9861..f970f9e 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p) > { > warning (0, N_("-maltivec=le not allowed for big-endian targets")); > rs6000_altivec_element_order = 0; > } > > + if (!rs6000_fold_gimple) > + fprintf (stderr, > + "gimple folding of rs6000 builtins has been disabled.\n"); That first " should line up with the s in stderr. Should this print a message though? > + size_t uns_fncode = (size_t) fn_code; > + enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; > + const char *fn_name1 = rs6000_builtin_info[uns_fncode].name; > + const char *fn_name2 = ((icode != CODE_FOR_nothing) > + ? get_insn_name ((int) icode) > + : "nothing"); Similarly, the ? and : should line up with the second (. (You can also remove the superfluous outer parens). > + if (TARGET_DEBUG_BUILTIN) > + { > + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n", > + fn_code, fn_name1, fn_name2); > + } Wrong indent on the {}; but you don't need those here anyway, it's a single statement. > default: > + if (TARGET_DEBUG_BUILTIN) > + fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n", > + fn_code, fn_name1, fn_name2); No space before \n please. > +mfold-gimple > +Target Report Var(rs6000_fold_gimple, 1) Init(1) > +Enable early gimple folding of builtins. You can drop the ", 1" I think? Okay with those nits fixed. Segher From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61172 invoked by alias); 9 Aug 2017 21:47:08 -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 61156 invoked by uid 89); 9 Aug 2017 21:47:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=unavailable version=3.3.2 spammy=HX-detected-operating-system:2.6.x X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (208.118.235.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Aug 2017 21:47:06 +0000 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dfYoe-0002Rw-Hp for gcc-patches@gcc.gnu.org; Wed, 09 Aug 2017 17:47:04 -0400 Received: from gate.crashing.org ([63.228.1.57]:47123) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dfYod-0002QN-Dd for gcc-patches@gcc.gnu.org; Wed, 09 Aug 2017 17:47:00 -0400 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id v79Kkn35018483; Wed, 9 Aug 2017 15:46:49 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id v79KkjbK018463; Wed, 9 Aug 2017 15:46:45 -0500 Date: Thu, 10 Aug 2017 00:04:00 -0000 From: Segher Boessenkool To: Will Schmidt Cc: GCC Patches , Richard Biener , Bill Schmidt , David Edelsohn Subject: Re: [PATCH, rs6000] (v2) enable early debug and disable switch for gimple folding Message-ID: <20170809204644.GX13471@gate.crashing.org> References: <1502226896.6577.12.camel@brimstone.rchland.ibm.com> <1502289548.6577.39.camel@brimstone.rchland.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1502289548.6577.39.camel@brimstone.rchland.ibm.com> User-Agent: Mutt/1.4.2.3i X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x [fuzzy] X-Received-From: 63.228.1.57 X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00707.txt.bz2 Message-ID: <20170810000400.5ZXu_l8KmogdxB2eWtUFBbzfsi2aztkBxl84_onern0@z> On Wed, Aug 09, 2017 at 09:39:08AM -0500, Will Schmidt wrote: > I also fixed the (missing) space after > cast for the existing debug code. Thanks for that :-) > * config/rs6000/rs6000.c: (rs6000_option_override_internal) Add blurb > to indicate when early gimple folding has been disabled. The colon goes after the closing parenthesis. > (rs6000_invalid_builtin): whitespace. Capital W. Or, say what this does? "Fix whitespace." > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 1fb9861..f970f9e 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -4180,10 +4180,14 @@ rs6000_option_override_internal (bool global_init_p) > { > warning (0, N_("-maltivec=le not allowed for big-endian targets")); > rs6000_altivec_element_order = 0; > } > > + if (!rs6000_fold_gimple) > + fprintf (stderr, > + "gimple folding of rs6000 builtins has been disabled.\n"); That first " should line up with the s in stderr. Should this print a message though? > + size_t uns_fncode = (size_t) fn_code; > + enum insn_code icode = rs6000_builtin_info[uns_fncode].icode; > + const char *fn_name1 = rs6000_builtin_info[uns_fncode].name; > + const char *fn_name2 = ((icode != CODE_FOR_nothing) > + ? get_insn_name ((int) icode) > + : "nothing"); Similarly, the ? and : should line up with the second (. (You can also remove the superfluous outer parens). > + if (TARGET_DEBUG_BUILTIN) > + { > + fprintf (stderr, "rs6000_gimple_fold_builtin %d %s %s\n", > + fn_code, fn_name1, fn_name2); > + } Wrong indent on the {}; but you don't need those here anyway, it's a single statement. > default: > + if (TARGET_DEBUG_BUILTIN) > + fprintf (stderr, "gimple builtin intrinsic not matched:%d %s %s \n", > + fn_code, fn_name1, fn_name2); No space before \n please. > +mfold-gimple > +Target Report Var(rs6000_fold_gimple, 1) Init(1) > +Enable early gimple folding of builtins. You can drop the ", 1" I think? Okay with those nits fixed. Segher