From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26835 invoked by alias); 28 Jan 2011 10:07:37 -0000 Received: (qmail 26823 invoked by uid 22791); 28 Jan 2011 10:07:36 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 28 Jan 2011 10:07:32 +0000 Received: (qmail 12484 invoked from network); 28 Jan 2011 10:07:30 -0000 Received: from unknown (HELO ?192.168.0.104?) (ams@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Jan 2011 10:07:30 -0000 Message-ID: <4D42955C.1060707@codesourcery.com> Date: Fri, 28 Jan 2011 13:06:00 -0000 From: Andrew Stubbs User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: Richard Earnshaw CC: gcc-patches@gcc.gnu.org Subject: Re: [patch][ARM] Fix 16-bit -> 64-bit multiply and accumulate References: <4D01018F.3020108@codesourcery.com> <1296153619.9738.16.camel@e102346-lin.cambridge.arm.com> In-Reply-To: <1296153619.9738.16.camel@e102346-lin.cambridge.arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: 2011-01/txt/msg02099.txt.bz2 On 27/01/11 18:40, Richard Earnshaw wrote: > I'm worried about this change. The proposed RTL might be what GCC > generates for this particular case, but the pattern hardly looks > canonical: a sign-extend of a multiply that contained sign-extended > operands. > > Are you sure that fixing this particular case doesn't introduce a > regression elsewhere? > > Alternatively, is there some part of the manual that describes the form > you have as the correct canonical form? I don't think I said it was the canonical form (although the other similar smulbb patch was about that), but if it weren't in the canonical form then combine would not work (which it does with my patch), so I don't believe that's the problem here. Anyway, combine only canonicalizes the *order* or the expressions, not the modes, which is what I'm adjusting here. Or have I missed something? The problem here is that the expand pass used mulhisi3 which has the 'internal' modes set one way, and then combine tries to insert that into a multiply-and-accumulate form, but recog rejects it because the 'internal' modes of maddhidi4 are set a different way. Now, mulhisi3 is currently written using MULT:SI mode, and I can't see how this could be any other way - it's an SI mode insn. If we want to be able to insert that into maddhidi4, and we do, I don't see any way to do that if it uses a MULT:DI - it just isn't compatible. I have regression tested the patch for correctness, but I haven't proven that I haven't produced worse code in some other case. I could introduce a second pattern, say "*maddhidi4_2", so that both patterns match and then there could be no regressions, right? However, I don't believe it's necessary. Andrew