RE: [PATCH, ARM, iWMMXt][1/5]: ARM code generic change

[Xinyu Qi <xyqi at marvell dot com>]

At 2011-12-15 00:47:48,"Richard Earnshaw" <rearnsha@arm.com> wrote: 
> On 14/07/11 08:35, Xinyu Qi wrote:
> >>> Hi,
> >>>
> >>> It is the first part of iWMMXt maintenance.
> >>>
> >>> *config/arm/arm.c (arm_option_override):
> >>>   Enable iWMMXt with VFP. iWMMXt and NEON are incompatible.
> >> iWMMXt unsupported under Thumb-2 mode.
> >>>   (arm_expand_binop_builtin): Accept immediate op (with mode VOID)
> >>> *config/arm/arm.md:
> >>>   Resettle include location of iwmmxt.md so that *arm_movdi
> >> and *arm_movsi_insn could be used when iWMMXt is enabled.
> >>
> >> With the current work in trunk to handle enabled attributes
> >> and per-alternative predicable attributes (Thanks Bernd) we
> >> should be able to get rid of *cond_iwmmxt_movsi_insn"  in
> >> iwmmxt.md file. It's not a matter for this patch but for a
> >> follow-up patch.
> >>
> >> Actually we should probably do the same for the various insns
> >> that are dotted around all over the place with final
> >> conditions that prevent matching - atleast makes the backend
> >> description slightly smaller :).
> >>
> >>>   Add pipeline description file include.
> >>
> >> It is enough to say
> >>
> >>  (<filename>): Include.
> >>
> >> in the changelog entry.
> >>
> >> The include for the pipeline description file should be with
> >> the patch that you add this in i.e. patch #5. Please add this
> >> to MD_INCLUDES in t-arm as well.
> >>
> >> Also as a general note, please provide a correct Changelog entry.
> >>
> >> This is not the format that we expect Changelog entries to be in.
> >> Please look at the coding standards on the website for this
> >> or at other patches submitted with respect to Changelog
> >> entries. Please fix this for each patch in the patch stack.
> >>
> >>
> >> cheers
> >> Ramana
> >
> > Thanks for reviewing. I have updated the patches and the Changelog.
> >
> > *config/arm/arm.c (arm_option_override): Enable iWMMXt with VFP.
> >  (arm_expand_binop_builtin): Accept VOIDmode op.
> > *config/arm/arm.md (*arm_movdi, *arm_movsi_insn): Remove
> condition !TARGET_IWMMXT.
> >  (iwmmxt.md): Include location.
> >
> > Thanks,
> > Xinyu=
> >
> 
> +     VFP and iWMMXt however can coexist.  */
> +  if (TARGET_IWMMXT && TARGET_HARD_FLOAT && !TARGET_VFP)
> +    sorry ("iWMMXt and non-VFP floating point unit");
> +
> +  /* iWMMXt and NEON are incompatible.  */
> +  if (TARGET_IWMMXT && TARGET_NEON)
> +    sorry ("iWMMXt and NEON");
> 
> -  /* ??? iWMMXt insn patterns need auditing for Thumb-2.  */
> +  /* iWMMXt unsupported under Thumb-2 mode.  */
>    if (TARGET_THUMB2 && TARGET_IWMMXT)
>      sorry ("Thumb-2 iWMMXt");
> 
> Don't use sorry() when a feature is not supported by the hardware;
> sorry() is used when GCC is currently unable to support something that
> it should.  Use error() in these cases.
> 
> Secondly, iWMMXt is incompatible with the entire Thumb ISA, not just the
> Thumb-2 extensions to the Thumb ISA.

Done.

> 
> 
> +;; Load the Intel Wireless Multimedia Extension patterns
> +(include "iwmmxt.md")
> +
> 
> 
> No, the extension patterns need to come at the end of the main machine
> description.  The list at the top of the MD file is purely for pipeline
> descriptions.  Why do you think this is needed?

This modification is needless right now since *iwmmxt_movsi_insn and *iwmmxt_arm_movdi
have been corrected in the fourth part of the patch. Revert it.
The new modified patch is attached.

	* config/arm/arm.c (arm_option_override): Enable use of iWMMXt with VFP.
	Disable use of iWMMXt with NEON. Disable use of iWMMXt under Thumb mode.
	(arm_expand_binop_builtin): Accept VOIDmode op.

Thanks,
Xinyu

> 
> Other bits are ok.
> 
> R.

Attachment: 1_generic.diff
Description: 1_generic.diff