At 2011-08-18 10:21:01,"Ramana Radhakrishnan" <ramana.radhakrish...@linaro.org> wrote: > On 14 July 2011 08:45, Xinyu Qi <x...@marvell.com> 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.
> > > (*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_wunpckihh, > 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_wsadbz, > iwmmxt_wsadhz): Revise. > > Revise to do what ? Sorry for late response. Some of them have incorrect RTL templates. For example, see iwmmxt_uavgv8qi3 Its old RTL template is: [(set (match_operand:V8QI 0 "register_operand" "=y") (ashiftrt:V8QI (plus:V8QI (match_operand:V8QI 1 "register_operand" "y") (match_operand:V8QI 2 "register_operand" "y")) (const_int 1)))] According to the assembly behavior of wavg2b, the correct one should be: [(set (match_operand:V8QI 0 "register_operand" "=y") (truncate:V8QI (lshiftrt:V8HI (plus:V8HI (zero_extend:V8HI (match_operand:V8QI 1 "register_operand" "y")) (zero_extend:V8HI (match_operand:V8QI 2 "register_operand" "y"))) (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 => 0x00. 0x00 > 1 => 0x00 While the correct result should be 0x80. 0x01 => 0x0001, 0xff => 0x00ff. 0x0001 + 0x00ff => 0x0100. 0x0100 > 1 => 0x0080, 0x0080 => 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 version gcc. Maybe this internal bug has been fixed, but such modification is harmless. Rests of them are only revised for their format. > > > (define_insn "*iwmmxt_movsi_insn" > > - [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,rk, > m,z,r,?z,Uy,z") > >- (match_operand:SI 1 "general_operand" "rk, I,K,mi,rk,r,z,Uy,z, > z"))] > >+ [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,r,rk, > m,z,r,?z,?Uy,?z,t,r,?t,?z,t") > >+ (match_operand:SI 1 "general_operand" " rk,I,K,N,mi,rk,r,z,Uy, z, > z,r,t, z, t,t"))] > > "TARGET_REALLY_IWMMXT > >- && ( register_operand (operands[0], SImode) > >- || register_operand (operands[1], SImode))" > >- "* > >- switch (which_alternative) > >+ && ((register_operand (operands[0], SImode) > >+ && (!reload_completed > >+ || REGNO_REG_CLASS (REGNO (operands[0])) == IWMMXT_GR_REGS)) > >+ || (register_operand (operands[1], SImode) > >+ && (!reload_completed > > > > >+ || REGNO_REG_CLASS (REGNO (operands[1])) == IWMMXT_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 that 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 iwmmxt2 though there are two options "-mcpu=iwmmxt" and "-mcpu=iwmmxt2". It seems if "-mcpu=iwmmxt" 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. (arm_output_iwmmxt_tinsr): Likewise. * config/arm/arm-protos.h (arm_output_iwmmxt_shift_immediate): Declare. (arm_output_iwmmxt_tinsr): Likewise. * config/arm/iwmmxt.md (WCGR0, WCGR1, WCGR2, WCGR3): New constant. (iwmmxt_psadbw, iwmmxt_walign, iwmmxt_tmrc, iwmmxt_tmcr): Delete. (iwmmxt_tbcstqi, iwmmxt_tbcsthi, iwmmxt_tbcstsi): Likewise (*iwmmxt_clrv8qi, *iwmmxt_clrv4hi, *iwmmxt_clrv2si): Likewise. (tbcstv8qi, tbcstv4hi, tbsctv2si): New pattern. (iwmmxt_clrv8qi, iwmmxt_clrv4hi, iwmmxt_clrv2si): Likewise. (*and<mode>3_iwmmxt, *ior<mode>3_iwmmxt, *xor<mode>3_iwmmxt): Likewise. (rori<mode>3, ashri<mode>3_iwmmxt, lshri<mode>3_iwmmxt): Likewise. (ashli<mode>3_iwmmxt, iwmmxt_waligni, iwmmxt_walignr): Likewise. (iwmmxt_walignr0, iwmmxt_walignr1, iwmmxt_walignr2, iwmmxt_walignr3): Likewise. (iwmmxt_setwcgr0, iwmmxt_setwcgr1, iwmmxt_setwcgr2, iwmmxt_setwcgr3): Likewise. (iwmmxt_getwcgr0, iwmmxt_getwcgr1, iwmmxt_getwcgr2, iwmmxt_getwcgr3): Likewise. (All instruction patterns): Add wtype attribute. (*iwmmxt_arm_movdi, *iwmmxt_movsi_insn): iWMMXt coexist with vfp. (iwmmxt_uavgrndv8qi3, iwmmxt_uavgrndv4hi3): Revise the pattern. (iwmmxt_uavgv8qi3, iwmmxt_uavgv4hi3): Likewise. (iwmmxt_tinsrb, iwmmxt_tinsrh, iwmmxt_tinsrw):Likewise. (eqv8qi3, eqv4hi3, eqv2si3, gtuv8qi3): Likewise. (gtuv4hi3, gtuv2si3, gtv8qi3, gtv4hi3, gtv2si3): Likewise. (iwmmxt_wunpckihh, iwmmxt_wunpckihw, iwmmxt_wunpckilh): Likewise. (iwmmxt_wunpckilw, iwmmxt_wunpckehub, iwmmxt_wunpckehuh): Likewise. (iwmmxt_wunpckehuw, iwmmxt_wunpckehsb, iwmmxt_wunpckehsh): Likewise. (iwmmxt_wunpckehsw, iwmmxt_wunpckelub, iwmmxt_wunpckeluh): Likewise. (iwmmxt_wunpckeluw, iwmmxt_wunpckelsb, iwmmxt_wunpckelsh): Likewise. (iwmmxt_wunpckelsw, iwmmxt_wmadds, iwmmxt_wmaddu): Likewise. (iwmmxt_wsadb, iwmmxt_wsadh, iwmmxt_wsadbz, iwmmxt_wsadhz): Likewise. (iwmmxt2.md): Include. * config/arm/iwmmxt2.md: New file. * config/arm/iterators.md (VMMX2): New mode_iterator. * config/arm/arm.md (wtype): New attribute. (UNSPEC_WMADDS, UNSPEC_WMADDU): Delete. (UNSPEC_WALIGNI): New unspec. * config/arm/t-arm (MD_INCLUDES): Add iwmmxt2.md. Thanks, Xinyu
4_wmmx_md.diff.gz
Description: 4_wmmx_md.diff.gz