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

[Richard Earnshaw <rearnsha at arm dot com>]

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.


+;; 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?

Other bits are ok.

R.