From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8667 invoked by alias); 18 Nov 2011 23:36:32 -0000 Received: (qmail 8239 invoked by uid 22791); 18 Nov 2011 23:36:30 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,TW_IW,TW_MX,TW_VF,TW_VG,TW_XF X-Spam-Check-By: sourceware.org Received: from mail-yw0-f47.google.com (HELO mail-yw0-f47.google.com) (209.85.213.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Nov 2011 23:36:16 +0000 Received: by ywt2 with SMTP id 2so3411700ywt.20 for ; Fri, 18 Nov 2011 15:36:15 -0800 (PST) MIME-Version: 1.0 Received: by 10.50.94.229 with SMTP id df5mr5108782igb.27.1321659375476; Fri, 18 Nov 2011 15:36:15 -0800 (PST) Received: by 10.50.236.8 with HTTP; Fri, 18 Nov 2011 15:36:15 -0800 (PST) In-Reply-To: <4737A960563B524DA805CA602BE04B30602611FB90@SC-VEXCH2.marvell.com> References: <4737A960563B524DA805CA602BE04B306010E1F4E9@SC-VEXCH2.marvell.com> <4737A960563B524DA805CA602BE04B30602611FB90@SC-VEXCH2.marvell.com> Date: Sat, 19 Nov 2011 02:43:00 -0000 Message-ID: Subject: Re: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description From: Ramana Radhakrishnan To: Xinyu Qi Cc: "gcc-patches@gcc.gnu.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-11/txt/msg01971.txt.bz2 Hi Xinyu, This doesn't apply cleanly currently on trunk and the reject appears to come from iwmmxt.md and I've not yet investigated why. Can you have a look ? cheers Ramana On 26 September 2011 04:22, Xinyu Qi wrote: > Ping. > > http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00279.html > > =A0 =A0 =A0 =A0* config/arm/arm.c (arm_output_iwmmxt_shift_immediate): Ne= w function. > =A0 =A0 =A0 =A0(arm_output_iwmmxt_tinsr): Likewise. > =A0 =A0 =A0 =A0* config/arm/arm-protos.h (arm_output_iwmmxt_shift_immedia= te): Declare. > =A0 =A0 =A0 =A0(arm_output_iwmmxt_tinsr): Likewise. > =A0 =A0 =A0 =A0* config/arm/iwmmxt.md (WCGR0, WCGR1, WCGR2, WCGR3): New c= onstant. > =A0 =A0 =A0 =A0(iwmmxt_psadbw, iwmmxt_walign, iwmmxt_tmrc, iwmmxt_tmcr): = Delete. > =A0 =A0 =A0 =A0(iwmmxt_tbcstqi, iwmmxt_tbcsthi, iwmmxt_tbcstsi): Likewise > =A0 =A0 =A0 =A0(*iwmmxt_clrv8qi, *iwmmxt_clrv4hi, *iwmmxt_clrv2si): Likew= ise. > =A0 =A0 =A0 =A0(tbcstv8qi, tbcstv4hi, tbsctv2si): New pattern. > =A0 =A0 =A0 =A0(iwmmxt_clrv8qi, iwmmxt_clrv4hi, iwmmxt_clrv2si): Likewise. > =A0 =A0 =A0 =A0(*and3_iwmmxt, *ior3_iwmmxt, *xor3_iwmmx= t): Likewise. > =A0 =A0 =A0 =A0(rori3, ashri3_iwmmxt, lshri3_iwmmxt): L= ikewise. > =A0 =A0 =A0 =A0(ashli3_iwmmxt, iwmmxt_waligni, iwmmxt_walignr): Lik= ewise. > =A0 =A0 =A0 =A0(iwmmxt_walignr0, iwmmxt_walignr1): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_walignr2, iwmmxt_walignr3): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_setwcgr0, iwmmxt_setwcgr1): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_setwcgr2, iwmmxt_setwcgr3): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_getwcgr0, iwmmxt_getwcgr1): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_getwcgr2, iwmmxt_getwcgr3): Likewise. > =A0 =A0 =A0 =A0(All instruction patterns): Add wtype attribute. > =A0 =A0 =A0 =A0(*iwmmxt_arm_movdi, *iwmmxt_movsi_insn): iWMMXt coexist wi= th vfp. > =A0 =A0 =A0 =A0(iwmmxt_uavgrndv8qi3, iwmmxt_uavgrndv4hi3): Revise the pat= tern. > =A0 =A0 =A0 =A0(iwmmxt_uavgv8qi3, iwmmxt_uavgv4hi3): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_tinsrb, iwmmxt_tinsrh, iwmmxt_tinsrw):Likewise. > =A0 =A0 =A0 =A0(eqv8qi3, eqv4hi3, eqv2si3, gtuv8qi3): Likewise. > =A0 =A0 =A0 =A0(gtuv4hi3, gtuv2si3, gtv8qi3, gtv4hi3, gtv2si3): Likewise. > =A0 =A0 =A0 =A0(iwmmxt_wunpckihh, iwmmxt_wunpckihw, iwmmxt_wunpckilh): Li= kewise. > =A0 =A0 =A0 =A0(iwmmxt_wunpckilw, iwmmxt_wunpckehub, iwmmxt_wunpckehuh): = Likewise. > =A0 =A0 =A0 =A0(iwmmxt_wunpckehuw, iwmmxt_wunpckehsb, iwmmxt_wunpckehsh):= Likewise. > =A0 =A0 =A0 =A0(iwmmxt_wunpckehsw, iwmmxt_wunpckelub, iwmmxt_wunpckeluh):= Likewise. > =A0 =A0 =A0 =A0(iwmmxt_wunpckeluw, iwmmxt_wunpckelsb, iwmmxt_wunpckelsh):= Likewise. > =A0 =A0 =A0 =A0(iwmmxt_wunpckelsw, iwmmxt_wmadds, iwmmxt_wmaddu): Likewis= e. > =A0 =A0 =A0 =A0(iwmmxt_wsadb, iwmmxt_wsadh, iwmmxt_wsadbz, iwmmxt_wsadhz)= : Likewise. > =A0 =A0 =A0 =A0(iwmmxt2.md): Include. > =A0 =A0 =A0 =A0* config/arm/iwmmxt2.md: New file. > =A0 =A0 =A0 =A0* config/arm/iterators.md (VMMX2): New mode_iterator. > =A0 =A0 =A0 =A0* config/arm/arm.md (wtype): New attribute. > =A0 =A0 =A0 =A0(UNSPEC_WMADDS, UNSPEC_WMADDU): Delete. > =A0 =A0 =A0 =A0(UNSPEC_WALIGNI): New unspec. > =A0 =A0 =A0 =A0* config/arm/t-arm (MD_INCLUDES): Add iwmmxt2.md. > > At 2011-09-05 17:55:34,"Xinyu Qi" wrote: >> At 2011-08-18 10:21:01,"Ramana Radhakrishnan" >> wrote: >> > On 14 July 2011 08:45, Xinyu Qi wrote: >> > >> Hi, >> > >> >> > >> It is the fourth part of iWMMXt maintenance. >> > >> >> > >> > Can this be broken down further. ? I'll have to do this again but >> > there are some initial comments below for some discussion. >> >> > >> > > =A0(*iwmmxt_arm_movdi, *iwmmxt_movsi_insn, iwmmxt_uavgrndv8qi3, >> > iwmmxt_uavgrndv4hi3, iwmmxt_uavgv8qi3, iwmmxt_uavgv4hi3, iwmmxt_tinsrb, >> > iwmmxt_tinsrh, iwmmxt_tinsrw, eqv8qi3, eqv4hi3, eqv2si3, gtuv8qi3, >> gtuv4hi3, >> > gtuv2si3, gtv8qi3, gtv4hi3, gtv2si3, iwmmxt_wunpckihb, iwmmxt_wunpckih= h, >> > iwmmxt_wunpckihw, iwmmxt_wunpckilb, iwmmxt_wunpckilh, iwmmxt_wunpckilw, >> > iwmmxt_wunpckehub, iwmmxt_wunpckehuh, iwmmxt_wunpckehuw, >> iwmmxt_wunpckehsb, >> > iwmmxt_wunpckehsh, iwmmxt_wunpckehsw, iwmmxt_wunpckelub, >> iwmmxt_wunpckeluh, >> > iwmmxt_wunpckeluw, iwmmxt_wunpckelsb, iwmmxt_wunpckelsh, >> iwmmxt_wunpckelsw, >> > iwmmxt_wmadds, iwmmxt_wmaddu, iwmmxt_wsadb, iwmmxt_wsadh, iwmmxt_wsadb= z, >> > iwmmxt_wsadhz): Revise. >> > >> > Revise to do what ? >> >> Sorry for late response. >> >> Some of them have incorrect RTL templates. For example, see iwmmxt_uavgv= 8qi3 >> Its old RTL template is: >> =A0 [(set (match_operand:V8QI =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 0 "registe= r_operand" "=3Dy") >> =A0 =A0 =A0 =A0 (ashiftrt:V8QI (plus:V8QI >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (match_operand:V8QI = 1 "register_operand" "y") >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(matc= h_operand:V8QI 2 "register_operand" "y")) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (const_int 1)))] >> >> According to the assembly behavior of wavg2b, the correct one should be: >> =A0 [(set (match_operand:V8QI =A00 "register_operand" "=3Dy") >> =A0 =A0 =A0 =A0 (truncate:V8QI >> =A0 =A0 =A0 =A0 =A0 =A0(lshiftrt:V8HI >> =A0 =A0 =A0 =A0 =A0 =A0 =A0(plus:V8HI (zero_extend:V8HI (match_operand:V= 8QI 1 >> "register_operand" "y")) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (zero_extend:V8HI (match= _operand:V8QI 2 >> "register_operand" "y"))) >> =A0 =A0 =A0 =A0 =A0 =A0(const_int 1))))] >> >> Consider the case: >> The Operation on element 0x01 and 0xff: gcc with old RTL template would = optimize >> to the result 0x00.That is: >> 0x01 + 0xff =3D> 0x00. 0x00 > 1 =3D> 0x00 >> While the correct result should be 0x80. >> 0x01 =3D> 0x0001, 0xff =3D> 0x00ff. 0x0001 + 0x00ff =3D> 0x0100. 0x0100 = > 1 =3D> 0x0080, >> 0x0080 =3D> 0x80 >> >> iwmmxt_wmadds and iwmmxt_wmaddu are modified to use detailed RTL template >> instead of unspec. >> >> For some of the wunpck patterns, change the order of zero_extend and vec= _select >> in order to avoid a vec_select optimization internal error in old versio= n gcc. >> Maybe this internal bug has been fixed, but such modification is harmles= s. >> >> Rests of them are only revised for their format. >> >> > >> > > (define_insn "*iwmmxt_movsi_insn" >> > > - =A0[(set (match_operand:SI 0 "nonimmediate_operand" "=3Drk,r,r,rk, >> > m,z,r,?z,Uy,z") >> > >- =A0(match_operand:SI 1 "general_operand" =A0 =A0 =A0"rk, I,K,mi,rk,= r,z,Uy,z, >> > z"))] >> > >+ =A0[(set (match_operand:SI 0 "nonimmediate_operand" "=3Drk,r,r,r,rk, >> > m,z,r,?z,?Uy,?z,t,r,?t,?z,t") >> > >+ =A0(match_operand:SI 1 "general_operand" =A0 =A0 =A0" rk,I,K,N,mi,r= k,r,z,Uy, >> z, >> > z,r,t, z, t,t"))] >> > > =A0 "TARGET_REALLY_IWMMXT >> > >- =A0 && ( =A0 register_operand (operands[0], SImode) >> > >- =A0 =A0 =A0 || register_operand (operands[1], SImode))" >> > >- =A0"* >> > >- =A0 switch (which_alternative) >> > >+ =A0 && ((register_operand (operands[0], SImode) >> > >+ =A0&& (!reload_completed >> > >+ =A0 =A0 =A0|| REGNO_REG_CLASS (REGNO (operands[0])) =3D=3D IWMMXT_G= R_REGS)) >> > >+ =A0 =A0 =A0 || (register_operand (operands[1], SImode) >> > >+ =A0 =A0 && (!reload_completed >> > >> > >> > >> > >+ =A0 =A0 =A0 =A0 || REGNO_REG_CLASS (REGNO (operands[1])) =3D=3D IWM= MXT_GR_REGS)))" >> > >> > I don't like this at all - what you are doing is assuming that after >> > reg-alloc you are going to be able to rely on whether something has a >> > particular register class and then turn on and off it's matching. So >> > this matches before reload and doesn't do so after reload for the >> > cases where *iwmmxt_movsi_insn is really in a core register. I don't >> > think you can do it this way. If you really want to do this properly - >> > have an arch field for iwmmxt as well in the arch attribute and then >> > add these alternatives to existing patterns. >> > >> > If I understand what you are trying to do here - you are trying to use >> > *arm_movsi_insn and other patterns in the rest of the backend and let >> > things like "predicable" kick in right after reload for all cases >> > other than the ones you enumerate. In which case get rid of all the >> > other constaints in this pattern other than the constraints that are >> > valid for registers of class IWMMXT_REGS >> >> This piece of code is added to make iwmmxt coexist with vfp when iwmmxt = and >> vfp are enabled together. Agree, I don't think it is a good fix. >> Add adequate constrains to *iwmmxt_movsi_insn and *iwmmxt_arm_movdi so t= hat >> don't need to change their conditions. >> >> > >> > Also the definition of output_move_double has changed now and hence >> > this needs some rework. >> >> Done. >> >> > Should there be a distinction between iwmmxt and iwmmxt2 ? Is it a >> > user visible option ? >> >> I don't think users need to know the distinction between iwmmxt and iwmm= xt2 >> though there are two options "-mcpu=3Diwmmxt" and "-mcpu=3Diwmmxt2". It = seems if >> "-mcpu=3Diwmmxt" is specified in gcc, the assembler cannot recognize the= wmmx2 >> insns. >> >> >> > >> > Just in case it wasn't clear please don't commit any patch in this >> > series until all the patches have been completely reviewed. >> > >> > cheers >> > Ramana >> >> >> The new diff attached. New ChangLog: >> >> * config/arm/arm.c (arm_output_iwmmxt_shift_immediate): New function. >> =A0 (arm_output_iwmmxt_tinsr): Likewise. >> * config/arm/arm-protos.h (arm_output_iwmmxt_shift_immediate): Declare. >> =A0 (arm_output_iwmmxt_tinsr): Likewise. >> * config/arm/iwmmxt.md (WCGR0, WCGR1, WCGR2, WCGR3): New constant. >> =A0 (iwmmxt_psadbw, iwmmxt_walign, iwmmxt_tmrc, iwmmxt_tmcr): Delete. >> =A0 (iwmmxt_tbcstqi, iwmmxt_tbcsthi, iwmmxt_tbcstsi): Likewise >> =A0 (*iwmmxt_clrv8qi, *iwmmxt_clrv4hi, *iwmmxt_clrv2si): Likewise. >> =A0 (tbcstv8qi, tbcstv4hi, tbsctv2si): New pattern. >> =A0 (iwmmxt_clrv8qi, iwmmxt_clrv4hi, iwmmxt_clrv2si): Likewise. >> =A0 (*and3_iwmmxt, *ior3_iwmmxt, *xor3_iwmmxt): Likewi= se. >> =A0 (rori3, ashri3_iwmmxt, lshri3_iwmmxt): Likewise. >> =A0 (ashli3_iwmmxt, iwmmxt_waligni, iwmmxt_walignr): Likewise. >> =A0 (iwmmxt_walignr0, iwmmxt_walignr1, iwmmxt_walignr2, iwmmxt_walignr3): >> Likewise. >> =A0 (iwmmxt_setwcgr0, iwmmxt_setwcgr1, iwmmxt_setwcgr2, iwmmxt_setwcgr3): >> Likewise. >> =A0 (iwmmxt_getwcgr0, iwmmxt_getwcgr1, iwmmxt_getwcgr2, iwmmxt_getwcgr3): >> Likewise. >> =A0 (All instruction patterns): Add wtype attribute. >> =A0 (*iwmmxt_arm_movdi, *iwmmxt_movsi_insn): iWMMXt coexist with vfp. >> =A0 (iwmmxt_uavgrndv8qi3, iwmmxt_uavgrndv4hi3): Revise the pattern. >> =A0 (iwmmxt_uavgv8qi3, iwmmxt_uavgv4hi3): Likewise. >> =A0 (iwmmxt_tinsrb, iwmmxt_tinsrh, iwmmxt_tinsrw):Likewise. >> =A0 (eqv8qi3, eqv4hi3, eqv2si3, gtuv8qi3): Likewise. >> =A0 (gtuv4hi3, gtuv2si3, gtv8qi3, gtv4hi3, gtv2si3): Likewise. >> =A0 (iwmmxt_wunpckihh, iwmmxt_wunpckihw, iwmmxt_wunpckilh): Likewise. >> =A0 (iwmmxt_wunpckilw, iwmmxt_wunpckehub, iwmmxt_wunpckehuh): Likewise. >> =A0 (iwmmxt_wunpckehuw, iwmmxt_wunpckehsb, iwmmxt_wunpckehsh): Likewise. >> =A0 (iwmmxt_wunpckehsw, iwmmxt_wunpckelub, iwmmxt_wunpckeluh): Likewise. >> =A0 (iwmmxt_wunpckeluw, iwmmxt_wunpckelsb, iwmmxt_wunpckelsh): Likewise. >> =A0 (iwmmxt_wunpckelsw, iwmmxt_wmadds, iwmmxt_wmaddu): Likewise. >> =A0 (iwmmxt_wsadb, iwmmxt_wsadh, iwmmxt_wsadbz, iwmmxt_wsadhz): Likewise. >> =A0 (iwmmxt2.md): Include. >> * config/arm/iwmmxt2.md: New file. >> * config/arm/iterators.md (VMMX2): New mode_iterator. >> * config/arm/arm.md (wtype): New attribute. >> =A0 (UNSPEC_WMADDS, UNSPEC_WMADDU): Delete. >> =A0 (UNSPEC_WALIGNI): New unspec. >> * config/arm/t-arm (MD_INCLUDES): Add iwmmxt2.md. >> >> Thanks, >> Xinyu >