From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5154 invoked by alias); 27 Jul 2010 17:00:18 -0000 Received: (qmail 5117 invoked by uid 22791); 27 Jul 2010 17:00:13 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_SV,TW_VN,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cam-admin0.cambridge.arm.com (HELO cam-admin0.cambridge.arm.com) (217.140.96.50) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 27 Jul 2010 17:00:05 +0000 Received: from cam-owa1.Emea.Arm.com (cam-owa1.emea.arm.com [10.1.255.62]) by cam-admin0.cambridge.arm.com (8.12.6/8.12.6) with ESMTP id o6RGxuF9023063; Tue, 27 Jul 2010 17:59:56 +0100 (BST) Received: from [10.1.66.29] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Tue, 27 Jul 2010 17:59:56 +0100 Subject: Re: Extend widening_mul pass to handle fixed-point types From: Ramana Radhakrishnan Reply-To: ramana.radhakrishnan@arm.com To: Bernd Schmidt Cc: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com In-Reply-To: <4C4F009B.6040201@codesourcery.com> References: <87fwzhro8i.fsf@firetop.home> <4C436835.20307@codesourcery.com> <87oce3s4kv.fsf@firetop.home> <4C44C948.3050801@codesourcery.com> <1280233130.18689.17.camel@e102325-lin.cambridge.arm.com> <4C4F009B.6040201@codesourcery.com> Content-Type: multipart/mixed; boundary="=-JVuOthg8Jmjidd9imxSM" Date: Tue, 27 Jul 2010 17:10:00 -0000 Message-Id: <1280249995.18689.48.camel@e102325-lin.cambridge.arm.com> Mime-Version: 1.0 X-IsSubscribed: yes 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 X-SW-Source: 2010-07/txt/msg02103.txt.bz2 --=-JVuOthg8Jmjidd9imxSM Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-length: 2913 On Tue, 2010-07-27 at 17:51 +0200, Bernd Schmidt wrote: > On 07/27/2010 02:18 PM, Ramana Radhakrishnan wrote: > > > > On Mon, 2010-07-19 at 23:53 +0200, Bernd Schmidt wrote: > >> That's pretty much what I had in mind. Ok if it passes testing. > > > > This broke bootstraps on arm-linux-gnueabi and caused > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45067 . > > > > Taking a brief look at it this morning, I think the problem here is that > > the call to optab_for_tree_code is made with the wrong type of the > > expression. The presence of a widening optab for a WIDEN_MULT_PLUS_EXPR > > should be based on the type of ops->op2 rather than opnd0 because you > > have something like intval0 = shortval1 * shortval2 + intval1 whereas > > the expander is testing for a widening mult and plus where the results > > are into a HImode value and thus effectively for a widening operation > > into an HImode value. > > > > The assert that fails is because > > gcc_assert (icode != CODE_FOR_nothing); > > > > Something like this fixes the problem for the test-cases under question > > or would you prefer something else. A full bootstrap and test is > > underway. > > > > Ok to commit if no regressions ? > > I think this is consistent with the code in tree-ssa-math-opts (there we > use the type of the lhs, which should match that of operand 2). So, OK > - I think the ARM testsuite has some widening-macc cases so you should > notice if there's something wrong. Yep so I thought as well. Yes the tests in gcc.target/arm should cover these cases. Bootstrap proceeded further and ICE'd again in stage2 because we were applying gimple_assign_lhs on a non GIMPLE_ASSIGN statement. Attached is a simple test case that shows this problem with a cross-compiler as well. I am testing with this extra patch[1] applied on top of my previous patch which I think is obvious and will see what else is needed to be done to deal with the fallout of this patch with bootstrap on arm-linux-gnueabi. Verified with a simple testcase that we still do generate widening multiplies for the original testcase. I'll submit both these after I'm sure that bootstrap and regression testing completes. cheers Ramana 1. New patch being tested on top. 2010-07-27 Ramana Radhakrishnan * tree-ssa-math-opts.c (is_widening_mult_p): Return false if not an assignment. [ramrad01@e102325-lin gcc]$ svndiff tree-ssa-math-opts.c Index: tree-ssa-math-opts.c =================================================================== --- tree-ssa-math-opts.c (revision 162571) +++ tree-ssa-math-opts.c (working copy) @@ -1323,6 +1323,9 @@ is_widening_mult_p (gimple stmt, { tree type; + if (!is_gimple_assign (stmt)) + return false; + type = TREE_TYPE (gimple_assign_lhs (stmt)); if (TREE_CODE (type) != INTEGER_TYPE && TREE_CODE (type) != FIXED_POINT_TYPE) --=-JVuOthg8Jmjidd9imxSM Content-Disposition: attachment; filename=reduced.c Content-Type: text/x-csrc; name=reduced.c; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 2320 /* ./cc1 -O2 -mcpu=cortex-a9 -quiet reduced.c */ typedef union tree_node *tree; enum tree_code { ERROR_MARK, }; enum tree_code_class { tcc_type, }; extern const enum tree_code_class tree_code_type[]; struct tree_base { __extension__ enum tree_code code : 16; unsigned unsigned_flag : 1; }; struct tree_type { unsigned int precision : 10; }; union tree_node { struct tree_base base; struct tree_type type; }; enum integer_type_kind { itk_int, itk_none }; extern tree integer_types[itk_none]; typedef struct cpp_reader cpp_reader; enum c_tree_index { CTI_INTMAX_TYPE, CTI_MAX }; extern tree c_global_trees[CTI_MAX]; static void builtin_define_constants (const char *, tree); static void builtin_define_stdint_macros (void) { builtin_define_constants ("__INTMAX_C", c_global_trees[CTI_INTMAX_TYPE]); } void c_cpp_builtins (cpp_reader *pfile) { builtin_define_stdint_macros (); } static const char * type_suffix (tree type) { static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" }; int unsigned_suffix; int is_long; unsigned_suffix = (__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1084, __FUNCTION__); __t; })->base.unsigned_flag); if ((__extension__ ({ __typeof (type) const __t = (type); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision) < (__extension__ ({ __typeof (integer_types[itk_int]) const __t = (integer_types[itk_int]); if (tree_code_type[(int) (((enum tree_code) (__t)->base.code))] != (tcc_type)) tree_class_check_failed (__t, (tcc_type), "/home/ramrad01/sources/trunk/gcc/c-family/c-cppbuiltin.c", 1085, __FUNCTION__); __t; })->type.precision)) return suffixes[is_long * 2 + unsigned_suffix]; } static void builtin_define_constants (const char *macro, tree type) { const char *suffix; char *buf; suffix = type_suffix (type); if (suffix[0] == 0) { buf = (char *) __builtin_alloca(strlen (macro) + 6); } } --=-JVuOthg8Jmjidd9imxSM--