From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21738 invoked by alias); 7 Jul 2011 09:59:06 -0000 Received: (qmail 21728 invoked by uid 22791); 7 Jul 2011 09:59:06 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_TM X-Spam-Check-By: sourceware.org Received: from mail-wy0-f175.google.com (HELO mail-wy0-f175.google.com) (74.125.82.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 07 Jul 2011 09:58:48 +0000 Received: by wyg30 with SMTP id 30so626010wyg.20 for ; Thu, 07 Jul 2011 02:58:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.55.66 with SMTP id t2mr512639wbg.109.1310032727323; Thu, 07 Jul 2011 02:58:47 -0700 (PDT) Received: by 10.227.36.212 with HTTP; Thu, 7 Jul 2011 02:58:47 -0700 (PDT) In-Reply-To: <4E11CCD1.4010505@codesourcery.com> References: <4E034EF2.3070503@codesourcery.com> <4E03504B.9060305@codesourcery.com> <4E044559.5000105@linaro.org> <1A77B5B39081C241A68E6CF16983025F020906F6@EU1-MAIL.mgc.mentorg.com> <4E09B142.4020402@codesourcery.com> <4E09FDEA.3000004@gmail.com> <1A77B5B39081C241A68E6CF16983025F0209071D@EU1-MAIL.mgc.mentorg.com> <4E11CCD1.4010505@codesourcery.com> Date: Thu, 07 Jul 2011 10:00:00 -0000 Message-ID: Subject: Re: [PATCH (3/7)] Widening multiply-and-accumulate pattern matching From: Richard Guenther To: Andrew Stubbs Cc: Michael Matz , gcc-patches@gcc.gnu.org, patches@linaro.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: 2011-07/txt/msg00439.txt.bz2 On Mon, Jul 4, 2011 at 4:23 PM, Andrew Stubbs wrote: > On 01/07/11 13:25, Richard Guenther wrote: >> >> Well - some operations work the same on both signedness if you >> just care about the twos-complement result. =A0This includes >> multiplication (but not for example division). =A0For this special >> case I suggest to not bother trying to invent a generic predicate >> but do something local in tree-ssa-math-opts.c. > > OK, here's my updated patch. > > I've taken the view that we *know* what size and signedness the result of > the multiplication is, and we know what size the input to the addition mu= st > be, so all the check has to do is make sure it does that same conversion, > even if by a roundabout means. > > What I hadn't grasped before is that when extending a value it's the sour= ce > type that is significant, not the destination, so the checks are not as > complex as I had thought. > > So, this patch adds a test to ensure that: > > =A01. the type is not truncated so far that we lose any information; and > > =A02. the type is only ever extended in the proper signedness. > > Also, just to be absolutely sure, I've also added a little bit of logic to > permit extends that are then undone by a truncate. I'm really not sure wh= at > guarantees there are about what sort of cast sequences can exist? Is this > necessary? I haven't managed to coax it to generated any examples of exte= nds > followed by truncates myself, but in any case, it's hardly any code and > it'll make sure it's future proofed. > > OK? I think you should assume that series of widenings, (int)(short)char_variab= le are already combined. Thus I believe you only need to consider a single conversion in valid_types_for_madd_p. +/* Check the input types, TYPE1 and TYPE2 to a widening multiply, what are those types? Is TYPE1 the result type and TYPE2 the operand type? If so why + initial_bitsize =3D TYPE_PRECISION (type1) + TYPE_PRECISION (type2); this?! + initial_unsigned =3D TYPE_UNSIGNED (type1) && TYPE_UNSIGNED (type2); that also looks odd. So probably TYPE1 isn't the result type. If they are the types of the operands, then what operand is EXPR for? I didn't look at the actual implementation of the function because of the lack of understanding of the inputs. - if (TREE_CODE (rhs1) =3D=3D SSA_NAME) + for (tmp =3D rhs1, rhs1_code =3D ERROR_MARK; + TREE_CODE (tmp) =3D=3D SSA_NAME + && (CONVERT_EXPR_CODE_P (rhs1_code) || rhs1_code =3D=3D ERROR_MARK); + tmp =3D gimple_assign_rhs1 (rhs1_stmt)) { - rhs1_stmt =3D SSA_NAME_DEF_STMT (rhs1); - if (is_gimple_assign (rhs1_stmt)) - rhs1_code =3D gimple_assign_rhs_code (rhs1_stmt); + rhs1_stmt =3D SSA_NAME_DEF_STMT (tmp); + if (!is_gimple_assign (rhs1_stmt)) + break; + rhs1_code =3D gimple_assign_rhs_code (rhs1_stmt); } the result looks a bit like spaghetti code ... and lacks a comment on what it is trying to do. It looks like it sees through an arbitrary number of conversions - possibly ones that will make the macc invalid, as for (short)int-var * short-var + int-var. So you'll be pessimizing code by doing that unconditionally. As I said above you should at most consider one intermediate conversion. I believe the code should be arranged such that only valid conversions are looked through in the first place. Valid, in that the resulting types should still match the macc constraints. Richard. > Andrew >