On Tue, Oct 20, 2015 at 4:26 PM, Marcus Shawcroft
<marcus.shawcr...@gmail.com> wrote:
> On 19 October 2015 at 14:57, Kyrill Tkachov <kyrylo.tkac...@arm.com> wrote:
>
>> 2015-10-19  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>
>>
>>     * config/aarch64/aarch64.md
>>  (*aarch64_fcvt<su_optab><GPF:mode><GPI:mode>2_mult): New pattern.
>>     * config/aarch64/aarch64-simd.md
>>  (*aarch64_fcvt<su_optab><VDQF:mode><fcvt_target>2_mult): Likewise.
>>     * config/aarch64/aarch64.c (aarch64_rtx_costs): Handle above patterns.
>>     (aarch64_fpconst_pow_of_2): New function.
>>     (aarch64_vec_fpconst_pow_of_2): Likewise.
>>     * config/aarch64/aarch64-protos.h (aarch64_fpconst_pow_of_2): Declare
>>     prototype.
>>     (aarch64_vec_fpconst_pow_of_2): Likewise.
>>     * config/aarch64/predicates.md (aarch64_fp_pow2): New predicate.
>>     (aarch64_fp_vec_pow2): Likewise.
>>
>> 2015-10-19  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>
>>
>>     * gcc.target/aarch64/fmul_fcvt_1.c: New test.
>>     * gcc.target/aarch64/fmul_fcvt_2.c: Likewise.
>
> +    char buf[64];
> +    sprintf (buf, "fcvtz<su>\\t%%0.<Vtype>, %%1.<Vtype>, #%d", fbits);
>
> Prefer snprintf please

Should we also update our coding standards for this ? i.e. use the non
`n' versions of the string functions only if you have a very good
reason. Even more so in the run time support libraries !

Maybe we can go to the extreme of poisoning sprintf too, but that's
probably an extreme...

regards
Ramana


>
> +  }
> +  [(set_attr "type" "neon_fp_to_int_<Vetype><q>")]
> +)
> +
> +
>
> Superflous blank line here ?
>
> +  *cost += rtx_cost (XEXP (x, 0), VOIDmode,
> +     (enum rtx_code) code, 0, speed);
>
> My understanding is the unnecessary use of enum  is now discouraged,
> (rtx_code) is sufficient in this case.
>
> +  int count = CONST_VECTOR_NUNITS (x);
> +  int i;
> +  for (i = 1; i < count; i++)
>
> Push the int into the for initializer.
> Push the rhs of the count assignment into the for condition and drop
> the definition of count.
>
> +/* { dg-final { scan-assembler "fcvtzs\tw\[0-9\], s\[0-9\]*.*#2" } } */
>
> I'd prefer scan-assembler-times or do you have a particular reason to
> avoid it in these tests?
>
> Cheers
> /Marcus

Reply via email to