RE: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

RE: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]

> Hi,
> 
> It is the fourth part of iWMMXt maintenance.
> 

Since "*cond_iwmmxt_movsi_insn" would be got rid of soon, I keep it unchanged.

*config/arm/arm.c (arm_output_iwmmxt_shift_immediate): New function.
 (arm_output_iwmmxt_tinsr): Ditto.
*config/arm/arm-protos.h (arm_output_iwmmxt_shift_immediate): New prototype.
 (arm_output_iwmmxt_tinsr): Ditto.
*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, *iwmmxt_clrv8qi, *iwmmxt_clrv4hi, *iwmmxt_clrv2si): Rename...
 (tbcstv8qi, tbcstv4hi, tbsctv2si, iwmmxt_clrv8qi, iwmmxt_clrv4hi, iwmmxt_clrv2si): ...New pattern.
 (*and<mode>3_iwmmxt, *ior<mode>3_iwmmxt, *xor<mode>3_iwmmxt, rori<mode>3, ashri<mode>3_iwmmxt, lshri<mode>3_iwmmxt, ashli<mode>3_iwmmxt, iwmmxt_waligni, iwmmxt_walignr, iwmmxt_walignr0, iwmmxt_walignr1, iwmmxt_walignr2, iwmmxt_walignr3, iwmmxt_setwcgr0, iwmmxt_setwcgr1, iwmmxt_setwcgr2, iwmmxt_setwcgr3, iwmmxt_getwcgr0, iwmmxt_getwcgr1, iwmmxt_getwcgr2, iwmmxt_getwcgr3): New pattern.
(All instruction patterns): Add wtype attribute.
 (*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.
 (iwmmxt2.md): Include.
*config/arm/iwmmxt2.md: New file.
*config/arm/iterators.md (VMMX2): New mode_iterator.
*config/arm/arm.md (wtype): New attribute.
*config/arm/t-arm (MD_INCLUDES): Add iwmmxt2.md.

Thanks,
Xinyu

[-- Attachment #2: 4_wmmx_md.diff.gz --]
[-- Type: application/x-gzip, Size: 13061 bytes --]

Re: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Ramana Radhakrishnan]

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.

>
> Since "*cond_iwmmxt_movsi_insn" would be got rid of soon, I keep it unchanged.
>
> *config/arm/arm.c (arm_output_iwmmxt_shift_immediate): New function.
>  (arm_output_iwmmxt_tinsr): Ditto.

s/Ditto/Likewise

> *config/arm/arm-protos.h (arm_output_iwmmxt_shift_immediate): New prototype.
s/New prototype/Declare.

>  (arm_output_iwmmxt_tinsr): Ditto.
> *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, *iwmmxt_clrv8qi, *iwmmxt_clrv4hi, *iwmmxt_clrv2si): Rename...



>  (tbcstv8qi, tbcstv4hi, tbsctv2si, iwmmxt_clrv8qi, iwmmxt_clrv4hi, iwmmxt_clrv2si): ...New pattern.
s/...//

>  (*and<mode>3_iwmmxt, *ior<mode>3_iwmmxt, *xor<mode>3_iwmmxt, rori<mode>3, ashri<mode>3_iwmmxt, lshri<mode>3_iwmmxt, ashli<mode>3_iwmmxt, iwmmxt_waligni, iwmmxt_walignr, iwmmxt_walignr0, iwmmxt_walignr1, iwmmxt_walignr2, iwmmxt_walignr3, iwmmxt_setwcgr0, iwmmxt_setwcgr1, iwmmxt_setwcgr2, iwmmxt_setwcgr3, iwmmxt_getwcgr0, iwmmxt_getwcgr1, iwmmxt_getwcgr2, iwmmxt_getwcgr3): New pattern.

Break this up into multiple lines. Each line should only have upto 80
characters. Put this on one line and say New and add the others in a
row afterwards and use Likewise.

Look at other patches in the near past which set up Changelogs.


>  (*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 ?

> (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



>  (iwmmxt2.md): Include.
> *config/arm/iwmmxt2.md: New file.
> *config/arm/iterators.md (VMMX2): New mode_iterator.
> *config/arm/arm.md (wtype): New attribute.
> *config/arm/t-arm (MD_INCLUDES): Add iwmmxt2.md.
>
> Thanks,
> Xinyu
>


>+	       || 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.


The documentation states with respect to conditions for a define_insn :

`For nameless patterns, the condition is applied only when matching an
individual insn, and only after the insn has matched the pattern's
recognition template.  The insn's operands may be found in the vector
@code{operands}.  For an insn where the condition has once matched, it
can't be used to control register allocation, for example by excluding
certain hard registers or hard register combinations.'

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

Also the definition of output_move_double has changed now and hence
this needs some rework.

Should there be a distinction between iwmmxt and iwmmxt2 ? Is it a
user visible option ?

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

RE: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

[-- Attachment #1: Type: text/plain, Size: 7669 bytes --]

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

[-- Attachment #2: 4_wmmx_md.diff.gz --]
[-- Type: application/x-gzip, Size: 13186 bytes --]

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

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

Re: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Ramana Radhakrishnan]

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 <xyqi@marvell.com> wrote:
> 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
>

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

[-- Attachment #1: Type: text/plain, Size: 12078 bytes --]

Hi Ramana,

I solve the conflict, please try again. The new diff is attached.

Thanks,
Xinyu

At 2011-11-19 07:36:15,"Ramana Radhakrishnan" <ramana.radhakrishnan@linaro.org> wrote:
> 
> 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 <xyqi@marvell.com> wrote:
> > 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
> >

[-- Attachment #2: 4_wmmx_md.diff.gz --]
[-- Type: application/x-gzip, Size: 13183 bytes --]

Re: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Richard Earnshaw]

On 24/11/11 01:33, Xinyu Qi wrote:
> Hi Ramana,
> 
> I solve the conflict, please try again. The new diff is attached.
> 
> Thanks,
> Xinyu
> 
> At 2011-11-19 07:36:15,"Ramana Radhakrishnan" <ramana.radhakrishnan@linaro.org> wrote:
>>
>> 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 ?
>>

This patch is NOT ok.

You're adding features that were new in iWMMXt2 (ie not in the original
implementation) but you've provided no means by which the compiler can
detect which operations are only available on the new cores.

Further, I don't like the way you have separate patterns for the rotates
with immediate.  Please investigate using the alternative enable feature
to reduce these down to single patterns.  Once you've done all this,
some of your tricky builtin expand code in patch 3/5 should then
simplify significantly as you won't need separate expand codes for those
alternatives.

R.

>> cheers
>> Ramana
>>
>> On 26 September 2011 04:22, Xinyu Qi <xyqi@marvell.com> wrote:
>>> 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
>> >

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

[-- Attachment #1: Type: text/plain, Size: 16950 bytes --]

At 2011-12-15 01:32:13,"Richard Earnshaw" <rearnsha@arm.com> wrote:
> On 24/11/11 01:33, Xinyu Qi wrote:
> > Hi Ramana,
> >
> > I solve the conflict, please try again. The new diff is attached.
> >
> > Thanks,
> > Xinyu
> >
> > At 2011-11-19 07:36:15,"Ramana Radhakrishnan"
> <ramana.radhakrishnan@linaro.org> wrote:
> >>
> >> 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 ?
> >>
>
> This patch is NOT ok.
>
> You're adding features that were new in iWMMXt2 (ie not in the original
> implementation) but you've provided no means by which the compiler can
> detect which operations are only available on the new cores.

Hi Richard,

All of the WMMX chips support WMMX2 instructions.
What I do is to complement the WMMX2 intrinsic support in GCC.
I don't think it is necessary for users to consider whether one WMMX insn is a WMMX2 insn or not.

> Further, I don't like the way you have separate patterns for the rotates
> with immediate.  Please investigate using the alternative enable feature
> to reduce these down to single patterns.  Once you've done all this,
> some of your tricky builtin expand code in patch 3/5 should then
> simplify significantly as you won't need separate expand codes for those
> alternatives.

Done. Combine immediate and register alternative into one pattern for all shift patterns.
Also update the part three. Send it in another mail.
The new diff is attached. The 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.
        (rorv4hi3, rorv2si3, rordi3): Likewise.
        (rorv4hi3_di, rorv2si3_di, rordi3_di): Likewise.
        (ashrv4hi3_di, ashrv2si3_di, ashrdi3_di): Likewise.
        (lshrv4hi3_di, lshrv2si3_di, lshrdi3_di): Likewise.
        (ashlv4hi3_di, ashlv2si3_di, ashldi3_di): Likewise.
        (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.
        (ror<mode>3, ror<mode>3_di): Likewise.
        (ashr<mode>3_di, lshr<mode>3_di, ashl<mode>3_di): 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.
        (ashr<mode>3_iwmmxt, ashl<mode>3_iwmmxt, lshr<mode>3_iwmmxt): 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.
        * config/arm/predicates.md (imm_or_reg_operand): New predicate.

Thanks,
Xinyu

> R.
>
> >> cheers
> >> Ramana
> >>
> >> On 26 September 2011 04:22, Xinyu Qi <xyqi@marvell.com> wrote:
> >>> 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
> >> >
>


[-- Attachment #2: 4_wmmx_md.diff.gz --]
[-- Type: application/x-gzip, Size: 13077 bytes --]

Re: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Richard Earnshaw]

On 22/12/11 06:38, Xinyu Qi wrote:
> At 2011-12-15 01:32:13,"Richard Earnshaw" <rearnsha@arm.com> wrote:
>> On 24/11/11 01:33, Xinyu Qi wrote:
>>> Hi Ramana,
>>>
>>> I solve the conflict, please try again. The new diff is attached.
>>>
>>> Thanks,
>>> Xinyu
>>>
>>> At 2011-11-19 07:36:15,"Ramana Radhakrishnan"
>> <ramana.radhakrishnan@linaro.org> wrote:
>>>>
>>>> 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 ?
>>>>
>>
>> This patch is NOT ok.
>>
>> You're adding features that were new in iWMMXt2 (ie not in the original
>> implementation) but you've provided no means by which the compiler can
>> detect which operations are only available on the new cores.
> 
> Hi Richard,
> 
> All of the WMMX chips support WMMX2 instructions.

This may be true for Marvell's current range of processors, but I find
it hard to reconcile with the assembler support in GAS, which clearly
distinguishes between iWMMXT and iWMMXT2 instruction sets.  Are you
telling me that no cores were ever manufactured (even by Intel) that
only supported iWMMXT?

I'm concerned that this patch will break support for existing users who
have older chips (for GCC we have to go through a deprecation cycle if
we want to drop support for something we now believe is no-longer worth
maintaining).

> What I do is to complement the WMMX2 intrinsic support in GCC.

I understand that, and I'm not saying the patch can never go in; just
that it needs to separate out the support for the different architecture
variants.

> I don't think it is necessary for users to consider whether one WMMX insn is a WMMX2 insn or not.

Users don't (unless they want their code to run on legacy processors
that only support the original instruction set), but the compiler surely
must know what it is targeting.  Remember that the instruction patterns
are not entirely black boxes, the compiler can do optimizations on
intrinsics (it's one of the reasons why they are better than inline
assembly).  Unless the compiler knows exactly what instructions are
legal, it could end up optimizing something that started as a WMMX insn
into something that's a WMMX2 insn (for example, propagating a constant
into a vector shift expression).

R.

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

[-- Attachment #1: Type: text/plain, Size: 6481 bytes --]

At 2011-12-22 17:53:45,"Richard Earnshaw" <rearnsha@arm.com> wrote: 
> On 22/12/11 06:38, Xinyu Qi wrote:
> > At 2011-12-15 01:32:13,"Richard Earnshaw" <rearnsha@arm.com> wrote:
> >> On 24/11/11 01:33, Xinyu Qi wrote:
> >>> Hi Ramana,
> >>>
> >>> I solve the conflict, please try again. The new diff is attached.
> >>>
> >>> Thanks,
> >>> Xinyu
> >>>
> >>> At 2011-11-19 07:36:15,"Ramana Radhakrishnan"
> >> <ramana.radhakrishnan@linaro.org> wrote:
> >>>>
> >>>> 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 ?
> >>>>
> >>
> >> This patch is NOT ok.
> >>
> >> You're adding features that were new in iWMMXt2 (ie not in the original
> >> implementation) but you've provided no means by which the compiler can
> >> detect which operations are only available on the new cores.
> >
> > Hi Richard,
> >
> > All of the WMMX chips support WMMX2 instructions.
> 
> This may be true for Marvell's current range of processors, but I find
> it hard to reconcile with the assembler support in GAS, which clearly
> distinguishes between iWMMXT and iWMMXT2 instruction sets.  Are you
> telling me that no cores were ever manufactured (even by Intel) that
> only supported iWMMXT?
> 
> I'm concerned that this patch will break support for existing users who
> have older chips (for GCC we have to go through a deprecation cycle if
> we want to drop support for something we now believe is no-longer worth
> maintaining).
> 
> > What I do is to complement the WMMX2 intrinsic support in GCC.
> 
> I understand that, and I'm not saying the patch can never go in; just
> that it needs to separate out the support for the different architecture
> variants.
> 
> > I don't think it is necessary for users to consider whether one WMMX insn is a
> WMMX2 insn or not.
> 
> Users don't (unless they want their code to run on legacy processors
> that only support the original instruction set), but the compiler surely
> must know what it is targeting.  Remember that the instruction patterns
> are not entirely black boxes, the compiler can do optimizations on
> intrinsics (it's one of the reasons why they are better than inline
> assembly).  Unless the compiler knows exactly what instructions are
> legal, it could end up optimizing something that started as a WMMX insn
> into something that's a WMMX2 insn (for example, propagating a constant
> into a vector shift expression).
> 
> R.

Hi, Richard,

You are right. There exist the chips that only support WMMX instructions in the history.
I distinguish the iWMMXt and iWMMXt2 in the patch update this time.

In current GCC, -march=iwmmxt and -march=iwmmxt2 (or -mcpu=iwmmxt and -mcpu=iwmmxt2) are almost no difference in the compiling stage.
I take advantage of them to do the work, that is, make -march=iwmmxt (or -mcpu=iwmmxt) only support iWMMXt intrinsic iWMMXt built in and WMMX instructions,
and make -march=iwmmxt2 (or -mcpu=iwmmxt2) support fully iWMMXt2.

Define a new flag FL_IWMMXT2 to represent the chip support iWMMXt2 extension, which directly controls the iWMMXt2 built in initialization and the followed defines.
Define __IWMMXT2__ in TARGET_CPU_CPP_BUILTINS to control the access of iWMMXt2 intrinsics.
Define TARGET_REALLY_IWMMXT2 to control the access of WMMX2 instructions' machine description.
In arm.md, define iwmmxt2 in "arch" attr to control the access of the alternative in shift patterns. 

The updated patch 4/5 is attached here. 1/5, 2/5 and 3/5 are updated accordingly. Attach them in related mails.
Please take a look if such modification is proper.

Changelog:

	* 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.
	(rorv4hi3, rorv2si3, rordi3): Likewise.
	(rorv4hi3_di, rorv2si3_di, rordi3_di): Likewise.
	(ashrv4hi3_di, ashrv2si3_di, ashrdi3_di): Likewise.
	(lshrv4hi3_di, lshrv2si3_di, lshrdi3_di): Likewise.
	(ashlv4hi3_di, ashlv2si3_di, ashldi3_di): Likewise.
	(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.
	(ror<mode>3, ror<mode>3_di): Likewise.
	(ashr<mode>3_di, lshr<mode>3_di, ashl<mode>3_di): 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.
	(ashr<mode>3_iwmmxt, ashl<mode>3_iwmmxt, lshr<mode>3_iwmmxt): 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.
	* config/arm/predicates.md (imm_or_reg_operand): New predicate.

Thanks,
Xinyu

[-- Attachment #2: 4_wmmx_md.diff.gz --]
[-- Type: application/x-gzip, Size: 13102 bytes --]

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

PING

http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01786.html

At 2011-12-29 14:12:44,"Xinyu Qi" <xyqi@marvell.com> wrote:
> At 2011-12-22 17:53:45,"Richard Earnshaw" <rearnsha@arm.com> wrote:
> > On 22/12/11 06:38, Xinyu Qi wrote:
> > > At 2011-12-15 01:32:13,"Richard Earnshaw" <rearnsha@arm.com> wrote:
> > >> On 24/11/11 01:33, Xinyu Qi wrote:
> > >>> Hi Ramana,
> > >>>
> > >>> I solve the conflict, please try again. The new diff is attached.
> > >>>
> > >>> Thanks,
> > >>> Xinyu
> > >>>
> > >>> At 2011-11-19 07:36:15,"Ramana Radhakrishnan"
> > >> <ramana.radhakrishnan@linaro.org> wrote:
> > >>>>
> > >>>> 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 ?
> > >>>>
> > >>
> > >> This patch is NOT ok.
> > >>
> > >> You're adding features that were new in iWMMXt2 (ie not in the
> > >> original
> > >> implementation) but you've provided no means by which the compiler
> > >> can detect which operations are only available on the new cores.
> > >
> > > Hi Richard,
> > >
> > > All of the WMMX chips support WMMX2 instructions.
> >
> > This may be true for Marvell's current range of processors, but I find
> > it hard to reconcile with the assembler support in GAS, which clearly
> > distinguishes between iWMMXT and iWMMXT2 instruction sets.  Are you
> > telling me that no cores were ever manufactured (even by Intel) that
> > only supported iWMMXT?
> >
> > I'm concerned that this patch will break support for existing users
> > who have older chips (for GCC we have to go through a deprecation
> > cycle if we want to drop support for something we now believe is
> > no-longer worth maintaining).
> >
> > > What I do is to complement the WMMX2 intrinsic support in GCC.
> >
> > I understand that, and I'm not saying the patch can never go in; just
> > that it needs to separate out the support for the different
> > architecture variants.
> >
> > > I don't think it is necessary for users to consider whether one WMMX
> > > insn is a
> > WMMX2 insn or not.
> >
> > Users don't (unless they want their code to run on legacy processors
> > that only support the original instruction set), but the compiler
> > surely must know what it is targeting.  Remember that the instruction
> > patterns are not entirely black boxes, the compiler can do
> > optimizations on intrinsics (it's one of the reasons why they are
> > better than inline assembly).  Unless the compiler knows exactly what
> > instructions are legal, it could end up optimizing something that
> > started as a WMMX insn into something that's a WMMX2 insn (for
> > example, propagating a constant into a vector shift expression).
> >
> > R.
> 
> Hi, Richard,
> 
> You are right. There exist the chips that only support WMMX instructions in the
> history.
> I distinguish the iWMMXt and iWMMXt2 in the patch update this time.
> 
> In current GCC, -march=iwmmxt and -march=iwmmxt2 (or -mcpu=iwmmxt and
> -mcpu=iwmmxt2) are almost no difference in the compiling stage.
> I take advantage of them to do the work, that is, make -march=iwmmxt (or
> -mcpu=iwmmxt) only support iWMMXt intrinsic iWMMXt built in and WMMX
> instructions, and make -march=iwmmxt2 (or -mcpu=iwmmxt2) support fully
> iWMMXt2.
> 
> Define a new flag FL_IWMMXT2 to represent the chip support iWMMXt2
> extension, which directly controls the iWMMXt2 built in initialization and the
> followed defines.
> Define __IWMMXT2__ in TARGET_CPU_CPP_BUILTINS to control the access of
> iWMMXt2 intrinsics.
> Define TARGET_REALLY_IWMMXT2 to control the access of WMMX2
> instructions' machine description.
> In arm.md, define iwmmxt2 in "arch" attr to control the access of the
> alternative in shift patterns.
> 
> The updated patch 4/5 is attached here. 1/5, 2/5 and 3/5 are updated
> accordingly. Attach them in related mails.
> Please take a look if such modification is proper.
> 
> Changelog:
> 
> 	* 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.
> 	(rorv4hi3, rorv2si3, rordi3): Likewise.
> 	(rorv4hi3_di, rorv2si3_di, rordi3_di): Likewise.
> 	(ashrv4hi3_di, ashrv2si3_di, ashrdi3_di): Likewise.
> 	(lshrv4hi3_di, lshrv2si3_di, lshrdi3_di): Likewise.
> 	(ashlv4hi3_di, ashlv2si3_di, ashldi3_di): Likewise.
> 	(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.
> 	(ror<mode>3, ror<mode>3_di): Likewise.
> 	(ashr<mode>3_di, lshr<mode>3_di, ashl<mode>3_di): 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.
> 	(ashr<mode>3_iwmmxt, ashl<mode>3_iwmmxt, lshr<mode>3_iwmmxt):
> 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.
> 	* config/arm/predicates.md (imm_or_reg_operand): New predicate.
> 
> Thanks,
> Xinyu

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

PING

At 2012-02-03 10:09:55,"Xinyu Qi" <xyqi@marvell.com> wrote:
> PING
> 
> http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01786.html
> 
> At 2011-12-29 14:12:44,"Xinyu Qi" <xyqi@marvell.com> wrote:
> > At 2011-12-22 17:53:45,"Richard Earnshaw" <rearnsha@arm.com> wrote:
> > > On 22/12/11 06:38, Xinyu Qi wrote:
> > > > At 2011-12-15 01:32:13,"Richard Earnshaw" <rearnsha@arm.com>
> wrote:
> > > >> On 24/11/11 01:33, Xinyu Qi wrote:
> > > >>> Hi Ramana,
> > > >>>
> > > >>> I solve the conflict, please try again. The new diff is attached.
> > > >>>
> > > >>> Thanks,
> > > >>> Xinyu
> > > >>>
> > > >>> At 2011-11-19 07:36:15,"Ramana Radhakrishnan"
> > > >> <ramana.radhakrishnan@linaro.org> wrote:
> > > >>>>
> > > >>>> 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 ?
> > > >>>>
> > > >>
> > > >> This patch is NOT ok.
> > > >>
> > > >> You're adding features that were new in iWMMXt2 (ie not in the
> > > >> original
> > > >> implementation) but you've provided no means by which the compiler
> > > >> can detect which operations are only available on the new cores.
> > > >
> > > > Hi Richard,
> > > >
> > > > All of the WMMX chips support WMMX2 instructions.
> > >
> > > This may be true for Marvell's current range of processors, but I find
> > > it hard to reconcile with the assembler support in GAS, which clearly
> > > distinguishes between iWMMXT and iWMMXT2 instruction sets.  Are you
> > > telling me that no cores were ever manufactured (even by Intel) that
> > > only supported iWMMXT?
> > >
> > > I'm concerned that this patch will break support for existing users
> > > who have older chips (for GCC we have to go through a deprecation
> > > cycle if we want to drop support for something we now believe is
> > > no-longer worth maintaining).
> > >
> > > > What I do is to complement the WMMX2 intrinsic support in GCC.
> > >
> > > I understand that, and I'm not saying the patch can never go in; just
> > > that it needs to separate out the support for the different
> > > architecture variants.
> > >
> > > > I don't think it is necessary for users to consider whether one WMMX
> > > > insn is a
> > > WMMX2 insn or not.
> > >
> > > Users don't (unless they want their code to run on legacy processors
> > > that only support the original instruction set), but the compiler
> > > surely must know what it is targeting.  Remember that the instruction
> > > patterns are not entirely black boxes, the compiler can do
> > > optimizations on intrinsics (it's one of the reasons why they are
> > > better than inline assembly).  Unless the compiler knows exactly what
> > > instructions are legal, it could end up optimizing something that
> > > started as a WMMX insn into something that's a WMMX2 insn (for
> > > example, propagating a constant into a vector shift expression).
> > >
> > > R.
> >
> > Hi, Richard,
> >
> > You are right. There exist the chips that only support WMMX instructions in
> the
> > history.
> > I distinguish the iWMMXt and iWMMXt2 in the patch update this time.
> >
> > In current GCC, -march=iwmmxt and -march=iwmmxt2 (or -mcpu=iwmmxt
> and
> > -mcpu=iwmmxt2) are almost no difference in the compiling stage.
> > I take advantage of them to do the work, that is, make -march=iwmmxt (or
> > -mcpu=iwmmxt) only support iWMMXt intrinsic iWMMXt built in and WMMX
> > instructions, and make -march=iwmmxt2 (or -mcpu=iwmmxt2) support fully
> > iWMMXt2.
> >
> > Define a new flag FL_IWMMXT2 to represent the chip support iWMMXt2
> > extension, which directly controls the iWMMXt2 built in initialization and the
> > followed defines.
> > Define __IWMMXT2__ in TARGET_CPU_CPP_BUILTINS to control the access
> of
> > iWMMXt2 intrinsics.
> > Define TARGET_REALLY_IWMMXT2 to control the access of WMMX2
> > instructions' machine description.
> > In arm.md, define iwmmxt2 in "arch" attr to control the access of the
> > alternative in shift patterns.
> >
> > The updated patch 4/5 is attached here. 1/5, 2/5 and 3/5 are updated
> > accordingly. Attach them in related mails.
> > Please take a look if such modification is proper.
> >
> > Changelog:
> >
> > 	* 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.
> > 	(rorv4hi3, rorv2si3, rordi3): Likewise.
> > 	(rorv4hi3_di, rorv2si3_di, rordi3_di): Likewise.
> > 	(ashrv4hi3_di, ashrv2si3_di, ashrdi3_di): Likewise.
> > 	(lshrv4hi3_di, lshrv2si3_di, lshrdi3_di): Likewise.
> > 	(ashlv4hi3_di, ashlv2si3_di, ashldi3_di): Likewise.
> > 	(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.
> > 	(ror<mode>3, ror<mode>3_di): Likewise.
> > 	(ashr<mode>3_di, lshr<mode>3_di, ashl<mode>3_di): 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.
> > 	(ashr<mode>3_iwmmxt, ashl<mode>3_iwmmxt, lshr<mode>3_iwmmxt):
> > 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.
> > 	* config/arm/predicates.md (imm_or_reg_operand): New predicate.
> >
> > Thanks,
> > Xinyu

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

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.

RE: PING: [PATCH, ARM, iWMMXt][4/5]: WMMX machine description

[Xinyu Qi]

Ping.

http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01105.html

At 2011-07-14 15:45:09,"Xinyu Qi" <xyqi@marvell.com> wrote:
> > Hi,
> >
> > It is the fourth part of iWMMXt maintenance.
> >
> 
> Since "*cond_iwmmxt_movsi_insn" would be got rid of soon, I keep it unchanged.
> 
> *config/arm/arm.c (arm_output_iwmmxt_shift_immediate): New function.
>  (arm_output_iwmmxt_tinsr): Ditto.
> *config/arm/arm-protos.h (arm_output_iwmmxt_shift_immediate): New
> prototype.
>  (arm_output_iwmmxt_tinsr): Ditto.
> *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, *iwmmxt_clrv8qi,
> *iwmmxt_clrv4hi, *iwmmxt_clrv2si): Rename...
>  (tbcstv8qi, tbcstv4hi, tbsctv2si, iwmmxt_clrv8qi, iwmmxt_clrv4hi,
> iwmmxt_clrv2si): ...New pattern.
>  (*and<mode>3_iwmmxt, *ior<mode>3_iwmmxt, *xor<mode>3_iwmmxt, rori<mode>3,
> ashri<mode>3_iwmmxt, lshri<mode>3_iwmmxt, ashli<mode>3_iwmmxt,
> iwmmxt_waligni, iwmmxt_walignr, iwmmxt_walignr0, iwmmxt_walignr1,
> iwmmxt_walignr2, iwmmxt_walignr3, iwmmxt_setwcgr0, iwmmxt_setwcgr1,
> iwmmxt_setwcgr2, iwmmxt_setwcgr3, iwmmxt_getwcgr0, iwmmxt_getwcgr1,
> iwmmxt_getwcgr2, iwmmxt_getwcgr3): New pattern.
> (All instruction patterns): Add wtype attribute.
>  (*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.
>  (iwmmxt2.md): Include.
> *config/arm/iwmmxt2.md: New file.
> *config/arm/iterators.md (VMMX2): New mode_iterator.
> *config/arm/arm.md (wtype): New attribute.
> *config/arm/t-arm (MD_INCLUDES): Add iwmmxt2.md.
> 
> Thanks,
> Xinyu