From: Xinyu Qi <xyqi@marvell.com>
To: Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description
Date: Mon, 26 Sep 2011 05:42:00 -0000 [thread overview]
Message-ID: <4737A960563B524DA805CA602BE04B30602611FB90@SC-VEXCH2.marvell.com> (raw)
In-Reply-To: <CACUk7=W6FJrj6UoKCUCxw2ikLO6tE_2qYga2CjMPWBH4i6yT9w@mail.gmail.com>
Ping.
http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00279.html
* 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): Likewise.
(iwmmxt_walignr2, iwmmxt_walignr3): Likewise.
(iwmmxt_setwcgr0, iwmmxt_setwcgr1): Likewise.
(iwmmxt_setwcgr2, iwmmxt_setwcgr3): Likewise.
(iwmmxt_getwcgr0, iwmmxt_getwcgr1): Likewise.
(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.
At 2011-09-05 17:55:34,"Xinyu Qi" <xyqi@marvell.com> wrote:
> At 2011-08-18 10:21:01,"Ramana Radhakrishnan"
> <ramana.radhakrishnan@linaro.org> wrote:
> > On 14 July 2011 08:45, Xinyu Qi <xyqi@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
next prev parent reply other threads:[~2011-09-26 3:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-14 9:02 Xinyu Qi
2011-08-18 9:14 ` Ramana Radhakrishnan
2011-09-05 9:57 ` Xinyu Qi
2011-09-26 5:42 ` Xinyu Qi [this message]
2011-11-19 2:43 ` PING: " Ramana Radhakrishnan
2011-11-24 9:56 ` Xinyu Qi
2011-12-14 17:48 ` Richard Earnshaw
2011-12-22 6:50 ` Xinyu Qi
2011-12-22 10:12 ` Richard Earnshaw
2011-12-29 6:24 ` Xinyu Qi
2012-02-03 2:11 ` Xinyu Qi
2012-03-13 8:57 ` Xinyu Qi
2011-10-20 8:12 ` Xinyu Qi
2011-07-29 5:02 Xinyu Qi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4737A960563B524DA805CA602BE04B30602611FB90@SC-VEXCH2.marvell.com \
--to=xyqi@marvell.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ramana.radhakrishnan@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).