On 4 February 2016 at 16:31, Ramana Radhakrishnan
<ramana....@googlemail.com> wrote:
> On Sun, Jan 17, 2016 at 9:06 AM, Prathamesh Kulkarni
> <prathamesh.kulka...@linaro.org> wrote:
>> 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 for the benchmarking results -  Please repost the patch with
> the changes that I had requested in my previous review - given it is
> now stage4 , I would rather queue changes like this for stage1 now.
Hi,
Please find the updated patch attached.
It passes testsuite for arm-none-linux-gnueabi, arm-none-linux-gnueabihf and
arm-none-eabi.
However the test-case added in the patch (neon-vect-div-1.c) fails to
get vectorized at -O2
for armeb-none-linux-gnueabihf.
Charles suggested me to try with -O3, which worked.
It appears the test-case fails to get vectorized with
-fvect-cost-model=cheap (which is default enabled at -O2)
and passes for -fno-vect-cost-model / -fvect-cost-model=dynamic

I can't figure out why it fails -fvect-cost-model=cheap.
From the vect dump (attached):
neon-vect-div-1.c:12:3: note: Setting misalignment to -1.
neon-vect-div-1.c:12:3: note: not vectorized: unsupported unaligned load.*_9

Thanks,
Prathamesh
>
> Thanks,
> Ramana
>
>>
>> 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
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 55b61eb..7eafee4 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -578,6 +578,38 @@
                     (const_string "neon_mul_<V_elem_ch><q>")))]
 )
 
+/* Perform division using multiply-by-reciprocal. 
+   Reciprocal is calculated using Newton-Raphson method.
+   Enabled with -funsafe-math-optimizations -freciprocal-math
+   and disabled for -Os since it increases code size .  */
+
+(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")))]
+  "TARGET_NEON && !optimize_size
+   && 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 (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/neon-vect-div-1.c 
b/gcc/testsuite/gcc.target/arm/neon-vect-div-1.c
new file mode 100644
index 0000000..dae724a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/neon-vect-div-1.c
@@ -0,0 +1,16 @@
+/* Test pattern div<mode>3.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O3 -funsafe-math-optimizations -fdump-tree-vect-all" } */
+/* { dg-add-options arm_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 1 loops" 1 "vect" } } */
diff --git a/gcc/testsuite/gcc.target/arm/neon-vect-div-2.c 
b/gcc/testsuite/gcc.target/arm/neon-vect-div-2.c
new file mode 100644
index 0000000..0450b70
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/neon-vect-div-2.c
@@ -0,0 +1,16 @@
+/* Test pattern div<mode>3.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O3 -funsafe-math-optimizations -fno-reciprocal-math 
-fdump-tree-vect-all" } */
+/* { dg-add-options arm_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" } } */

Attachment: ChangeLog
Description: Binary data

Attachment: neon-vect-div-1.c.148t.vect
Description: Binary data

Reply via email to