From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 105199 invoked by alias); 4 Nov 2015 13:33:03 -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 102826 invoked by uid 89); 4 Nov 2015 13:33:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Nov 2015 13:33:00 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-23-EsMFSTqdRw2Ma8UPzzC7dw-1; Wed, 04 Nov 2015 13:32:53 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 4 Nov 2015 13:32:53 +0000 Message-ID: <563A0905.2020506@arm.com> Date: Wed, 04 Nov 2015 13:33:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Jeff Law , gcc Patches CC: Segher Boessenkool Subject: Re: [PATCH][combine][RFC] Don't transform sign and zero extends inside mults References: <56376FFF.3070008@arm.com> <5637E426.1080805@redhat.com> <5639EDFB.8030504@arm.com> In-Reply-To: <5639EDFB.8030504@arm.com> X-MC-Unique: EsMFSTqdRw2Ma8UPzzC7dw-1 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg00341.txt.bz2 On 04/11/15 11:37, Kyrill Tkachov wrote: > > On 02/11/15 22:31, Jeff Law wrote: >> On 11/02/2015 07:15 AM, Kyrill Tkachov wrote: >>> Hi all, >>> >>> This patch attempts to restrict combine from transforming ZERO_EXTEND >>> and SIGN_EXTEND operations into and-bitmask >>> and weird SUBREG expressions when they appear inside MULT expressions. >>> This is because a MULT rtx containing these >>> extend operations is usually a well understood widening multiply operat= ion. >>> However, if the costs for simple zero or sign-extend moves happen to >>> line up in a particular way, >>> expand_compound_operation will end up mangling a perfectly innocent >>> extend+mult+add rtx like: >>> (set (reg/f:DI 393) >>> (plus:DI (mult:DI (sign_extend:DI (reg/v:SI 425 [ selected ])) >>> (sign_extend:DI (reg:SI 606))) >>> (reg:DI 600))) >>> >>> into: >>> (set (reg/f:DI 393) >>> (plus:DI (mult:DI (and:DI (subreg:DI (reg:SI 606) 0) >>> (const_int 202 [0xca])) >>> (sign_extend:DI (reg/v:SI 425 [ selected ]))) >>> (reg:DI 600))) >> Going to leave the review side of this for Segher. >> >> If you decide to go forward, there's a section in md.texi WRT canonicali= zation of these RTL codes that probably would need updating. Just search fo= r "canonicalization" section and read down a ways. >> > > You mean document a canonical form for these operations as (mult (extend = op1) (extend op2)) ? > Looking at the issue more deeply I think the concrete problem is the code i= n expand_compound_operation: /* If we reach here, we want to return a pair of shifts. The inner shift is a left shift of BITSIZE - POS - LEN bits. The outer shift is a right shift of BITSIZE - LEN bits. It is arithmetic or logical depending on the value of UNSIGNEDP. If this was a ZERO_EXTEND or ZERO_EXTRACT, this pair of shifts will be converted into an AND of a shift. We must check for the case where the left shift would have a negative count. This can happen in a case like (x >> 31) & 255 on machines that can't shift by a constant. On those machines, we would first combine the shift with the AND to produce a variable-position extraction. Then the constant of 31 would be substituted in to produce such a position. */ modewidth =3D GET_MODE_PRECISION (GET_MODE (x)); if (modewidth >=3D pos + len) { machine_mode mode =3D GET_MODE (x); tem =3D gen_lowpart (mode, XEXP (x, 0)); if (!tem || GET_CODE (tem) =3D=3D CLOBBER) return x; tem =3D simplify_shift_const (NULL_RTX, ASHIFT, mode, tem, modewidth - pos - len); tem =3D simplify_shift_const (NULL_RTX, unsignedp ? LSHIFTRT : ASHIF= TRT, mode, tem, modewidth - len); } else if (unsignedp && len < HOST_BITS_PER_WIDE_INT) tem =3D simplify_and_const_int (NULL_RTX, GET_MODE (x), simplify_shift_const (NULL_RTX, LSHIFTRT, GET_MODE (x), XEXP (x, 0), pos), ((unsigned HOST_WIDE_INT) 1 << len) - 1); else /* Any other cases we can't handle. */ return x; this code ends up unconditionally transforming (zero_extend:DI (reg:SI 125)= ) into: (and:DI (subreg:DI (reg:SI 125) 0) (const_int 1 [0x1])) which then gets substituted as an operand of the mult and nothing matches a= fter that. archaeology shows this code has been there since at least 1992. I guess my = question is why do we do this unconditionally? Should we be checking whether the zero_e= xtend form is cheaper than whatever simplify_shift_const returns? I don't think what expand_compound_operatio is trying to do here is canonic= alisation... Thanks, Kyrill >> >> Jeff >> >> >