[PATCH, ARM] [1/3] iWMMXt intrinsics maintenance and pipeline description

Richard Earnshaw rearnsha@arm.com
Wed Jun 29 10:16:00 GMT 2011


On 29/06/11 09:48, Xinyu Qi wrote:
> Hi,
> 
> This patch maintains iWMMXt intrinsics code, and adds WMMX pipeline description.
> 
> *config/arm/arm.c (arm_option_override): Enable iWMMt with VFP
>  (enum arm_builtins): Add/fix iWMMXT/iWMMXT2 intrinsics.
>  (builtin_description bdesc_2arg): Same.
>  (builtin_description bdesc_1arg): Same.
>  (arm_init_iwmmxt_builtins): Same.
>  (arm_expand_binop_builtin): Same.
>  (arm_expand_builtin): Same.
>  (arm_output_iwmmxt_shift_immediate): New function. Same.
>  (arm_output_iwmmxt_tinsr): New function. Same.
> *config/arm/iterators.md: Same.
> *config/arm/mmintrin.h: Same.
> *config/arm/arm.md: Same. Add wtype.
> *config/arm/iwmmxt.md: Same.
> *config/arm/iwmmxt2.md: New file. Same.
> *config/arm/arm-protos.h: Add new functions protos.
> *config/arm/marvell-f-iwmmxt.md: New file. 
>  Add Marvell WMMX pipeline description.
> 
> Thanks,
> Xinyu


This is still too big to review (if a patch is not a mechanical change
and you need to compress it before posting, then it's almost certainly
too big).

I suggest you subdivide it along the following lines.

1) The new pipeline description
2) Changes to generic ARM code
3) New additions for iwmmxt

I think you also need to expand on your change log entry (it doesn't
help me understand the patch's contents).

Please also use 'diff -up -F "^(define"' to generate your diff files (so
that there's some function context on each hunk).

Finally, I note that you've made some changes along the lines of
 (define_insn "ssaddv8qi3"
   [(set (match_operand:V8QI               0 "register_operand" "=y")
-        (ss_plus:V8QI (match_operand:V8QI 1 "register_operand"  "y")
+        (ss_plus:V8QI (match_operand:V8QI 1 "register_operand"  "%y")
 		      (match_operand:V8QI 2 "register_operand"  "y")))]

Marking such patterns as commutative doesn't give any better results,
even though the operation is commutative.  Why?  well all marking the
pattern like this is that it tells reload to try swapping the order of
the commutative operands if reloading is needed (to see if the result
will need fewer reloads); but you've only got a single register class
for both alternatives and swapping them won't make any difference to
that answer -- so the result is simply that the compiler will spend more
time working out that there are still the same number of reloads to deal
with.

R.



More information about the Gcc-patches mailing list