[PATCH] AArch64 Neon Misc Scalar Arithmetic Instructions

[mcrosier at codeaurora.org]

Hi James,
Thanks for the quick feedback.

> Hi Chad,
>
> Just after having a very quick once-over:
>
> +                                !strconcat(asmop, " $Rd, $Rn, $Rm"),
>
> Is it not normal to separate the opcode and operands with '\t' instead of
> '
> '?

That seems very reasonable, but I'm rather new to this territory so I
don't have a definitive answer.  However, I don't see any cases in the
AArchInstrNEON.td file that does this, so for consistency sake I assume it
would be fine to leave it as a space.

>
> -multiclass Neon_Scalar_D_size_patterns<SDPatternOperator opnode,
> -                                       Instruction INSTD> {
> +multiclass Neon_Scalar3Same_D_size_patterns<SDPatternOperator opnode,
> +                                            Instruction INSTD> {
>    def : Pat<(v1i64 (opnode (v1i64 FPR64:$Rn), (v1i64 FPR64:$Rm))),
>              (INSTD FPR64:$Rn, FPR64:$Rm)>;
>  }
>
> Is there any particular reason you're adding a multiclass with only one
> (anonymous) element? Is it just so the later definitions can all
> consistently be "defm" or are you going to be adding to this class?

My change was just to rename Neon_Scalar_D_size_patterns to
Neon_Scalar3Same_D_size_patterns for consistency sake.

To answer your questions, the Neon_Scalar3Same_BHSD_size_patterns
multiclass inherits from the Neon_Scalar3Same_D_size_patterns multiclass. 
Thus, Neon_Scalar3Same_D_size_patterns must be a multiclass.

> -defm : Neon_Scalar_D_size_patterns<int_arm_neon_vqadds, SQADDddd>;
> -defm : Neon_Scalar_D_size_patterns<int_arm_neon_vqaddu, UQADDddd>;
> -defm : Neon_Scalar_D_size_patterns<int_arm_neon_vqsubs, SQSUBddd>;
> -defm : Neon_Scalar_D_size_patterns<int_arm_neon_vqsubu, UQSUBddd>;
> +defm : Neon_Scalar3Same_D_size_patterns<int_arm_neon_vqadds, SQADDddd>;
> +defm : Neon_Scalar3Same_D_size_patterns<int_arm_neon_vqaddu, UQADDddd>;
> +defm : Neon_Scalar3Same_D_size_patterns<int_arm_neon_vqsubs, SQSUBddd>;
> +defm : Neon_Scalar3Same_D_size_patterns<int_arm_neon_vqsubu, UQSUBddd>;
>
> I'm sure this has been mentioned before, and you've obviously not changed
> this, but are we deliberately reusing all v7/AArch32 intrinsics
> ("int_arm_*") and only adding new ones under "int_aarch64_*"? or is this
> code an oversight?

Ana and I are reusing the int_arm_* intrinsics when possible.  If the
instruction is completely new with ARMv8, then we're creating a new
intrinsic.

> Apologies for any stupidity, I haven't been following along much with the
> v8 NEON implementation.
>

No worries.  Thanks for the feedback, James!!  Hopefully, Tim, Joey, Jim
or someone else can have a look.

 Chad

> Cheers,
>
> James
>
>
> On 3 October 2013 17:39, <mcrosier at codeaurora.org> wrote:
>
>> All,
>> The attached patches contain clang front-end and LLVM back-end code
>> changes to add support for the following NEON scalar arithmetic
>> instructions:
>>
>> -Signed saturating doubling multiply high half - SQDMULH
>> -Signed saturating rounding doubling multiply high half - SQRDMULH
>> -Floating-point multiply extended - FMULX
>> -Floating-point reciprocal step - FRECPS
>> -Floating-point reciprocal square root step - FRSQRTS
>>
>> Please have a look!
>>
>>  Chad
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>