On 31 July 2015 at 15:04, Ramana Radhakrishnan
<ramana.radhakrish...@foss.arm.com> wrote:
>
>
> On 29/07/15 11:09, Prathamesh Kulkarni wrote:
>> Hi,
>> This patch tries to implement division with multiplication by
>> reciprocal using vrecpe/vrecps
>> with -funsafe-math-optimizations and -freciprocal-math enabled.
>> Tested on arm-none-linux-gnueabihf using qemu.
>> OK for trunk ?
>>
>> Thank you,
>> Prathamesh
>>
>
> I've tried this in the past and never been convinced that 2 iterations are 
> enough to get to stability with this given that the results are only precise 
> for 8 bits / iteration. Thus I've always believed you need 3 iterations 
> rather than 2 at which point I've never been sure that it's worth it. So the 
> testing that you've done with this currently is not enough for this to go 
> into the tree.
>
> I'd like this to be tested on a couple of different AArch32 implementations 
> with a wider range of inputs to verify that the results are acceptable as 
> well as running something like SPEC2k(6) with atleast one iteration to ensure 
> correctness.
Hi,
I got results of SPEC2k6 fp benchmarks:
a15: +0.64% overall, 481.wrf: +6.46%
a53: +0.21% overall, 416.gamess: -1.39%, 481.wrf: +6.76%
a57: +0.35% overall, 481.wrf: +3.84%
The other benchmarks had (almost) identical results.

Thanks,
Prathamesh
>
>
> moving on to the patches.
>
>> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
>> index 654d9d5..28c2e2a 100644
>> --- a/gcc/config/arm/neon.md
>> +++ b/gcc/config/arm/neon.md
>> @@ -548,6 +548,32 @@
>>                      (const_string "neon_mul_<V_elem_ch><q>")))]
>>  )
>>
>
> Please add a comment here.
>
>> +(define_expand "div<mode>3"
>> +  [(set (match_operand:VCVTF 0 "s_register_operand" "=w")
>> +        (div:VCVTF (match_operand:VCVTF 1 "s_register_operand" "w")
>> +               (match_operand:VCVTF 2 "s_register_operand" "w")))]
>
> I want to double check that this doesn't collide with Alan's patches for FP16 
> especially if he reuses the VCVTF iterator for all the vcvt f16 cases.
>
>> +  "TARGET_NEON && flag_unsafe_math_optimizations && flag_reciprocal_math"
>> +  {
>> +    rtx rec = gen_reg_rtx (<MODE>mode);
>> +    rtx vrecps_temp = gen_reg_rtx (<MODE>mode);
>> +
>> +    /* Reciprocal estimate */
>> +    emit_insn (gen_neon_vrecpe<mode> (rec, operands[2]));
>> +
>> +    /* Perform 2 iterations of Newton-Raphson method for better accuracy */
>> +    for (int i = 0; i < 2; i++)
>> +      {
>> +     emit_insn (gen_neon_vrecps<mode> (vrecps_temp, rec, operands[2]));
>> +     emit_insn (gen_mul<mode>3 (rec, rec, vrecps_temp));
>> +      }
>> +
>> +    /* We now have reciprocal in rec, perform operands[0] = operands[1] * 
>> rec */
>> +    emit_insn (gen_mul<mode>3 (operands[0], operands[1], rec));
>> +    DONE;
>> +  }
>> +)
>> +
>> +
>>  (define_insn "mul<mode>3add<mode>_neon"
>>    [(set (match_operand:VDQW 0 "s_register_operand" "=w")
>>          (plus:VDQW (mult:VDQW (match_operand:VDQW 2 "s_register_operand" 
>> "w")
>> diff --git a/gcc/testsuite/gcc.target/arm/vect-div-1.c 
>> b/gcc/testsuite/gcc.target/arm/vect-div-1.c
>> new file mode 100644
>> index 0000000..e562ef3
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/vect-div-1.c
>> @@ -0,0 +1,14 @@
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target arm_v8_neon_ok } */
>> +/* { dg-options "-O2 -funsafe-math-optimizations -ftree-vectorize 
>> -fdump-tree-vect-all" } */
>> +/* { dg-add-options arm_v8_neon } */
>
> No this is wrong.
>
> What is armv8 specific about this test ? This is just like another test that 
> is for Neon. vrecpe / vrecps are not instructions that were introduced in the 
> v8 version of the architecture. They've existed in the base Neon instruction 
> set. The code generation above in the patterns will be enabled when 
> TARGET_NEON is true which can happen when -mfpu=neon 
> -mfloat-abi={softfp/hard} is true.
>
>> +
>> +void
>> +foo (int len, float * __restrict p, float *__restrict x)
>> +{
>> +  len = len & ~31;
>> +  for (int i = 0; i < len; i++)
>> +    p[i] = p[i] / x[i];
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
>> diff --git a/gcc/testsuite/gcc.target/arm/vect-div-2.c 
>> b/gcc/testsuite/gcc.target/arm/vect-div-2.c
>> new file mode 100644
>> index 0000000..8e15d0a
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/vect-div-2.c
>> @@ -0,0 +1,14 @@
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target arm_v8_neon_ok } */
>
> And likewise.
>
>> +/* { dg-options "-O2 -funsafe-math-optimizations -fno-reciprocal-math 
>> -ftree-vectorize -fdump-tree-vect-all" } */
>> +/* { dg-add-options arm_v8_neon } */
>> +
>> +void
>> +foo (int len, float * __restrict p, float *__restrict x)
>> +{
>> +  len = len & ~31;
>> +  for (int i = 0; i < len; i++)
>> +    p[i] = p[i] / x[i];
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-times "vectorized 0 loops" 1 "vect" } } */
>
>
> regards
> Ramana

Reply via email to