From patchwork Tue Jul 27 16:59:55 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ramana Radhakrishnan X-Patchwork-Id: 60014 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id B1B8EB70C0 for ; Wed, 28 Jul 2010 03:00:26 +1000 (EST) 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> Date: Tue, 27 Jul 2010 17:59:55 +0100 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-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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)