[ARM] Enable DF only when TARGET_VFP_DOUBLE

Message ID CAELXzTPCwn7Vj3i8T_ASrQr9k5ODsDYBE=s=yTZHMMoOFO2A0A@mail.gmail.com
State New
Headers show
Series
  • [ARM] Enable DF only when TARGET_VFP_DOUBLE
Related show

Commit Message

Kugan Vivekanandarajah Oct. 10, 2019, 1:55 a.m.
As reported in Linaro bug report
(https://bugs.linaro.org/show_bug.cgi?id=4636 ; there is no
reproducible testcase provided), for some applications, we see

(insn 126 125 127 9 (set (reg:DF 189)
        (fma:DF (reg:DF 126 [ _74 ])
            (reg:DF 190)
            (reg:DF 191))) "ops.c":30 -1
     (nil))

This looks like due to a typo in the md patterns. Attached patch fixes
this. Bootsrapped and regression tested on arm-linux-gnueabihf without
any regressions.  Is this OK for trunk?

Thanks,
Kugan

gcc/ChangeLog:

2019-10-10  kugan.vivekanandarajah  <kugan.vivekanandarajah@linaro.org>

    * config/arm/vfp.md (fma<SDF:mode>4): Enable DF only when
    TARGET_VFP_DOUBLE.
    (*fmsub<SDF:mode>4): Likewise.
    (*fnmsub<SDF:mode>4): Likewise.
    (*fnmadd<SDF:mode>4): Likewise.

Comments

Kyrill Tkachov Oct. 10, 2019, 9:53 a.m. | #1
Hi Kugan,

On 10/10/19 2:55 AM, Kugan Vivekanandarajah wrote:
> As reported in Linaro bug report

> (https://bugs.linaro.org/show_bug.cgi?id=4636 ; there is no

> reproducible testcase provided), for some applications, we see

>

> (insn 126 125 127 9 (set (reg:DF 189)

>          (fma:DF (reg:DF 126 [ _74 ])

>              (reg:DF 190)

>              (reg:DF 191))) "ops.c":30 -1

>       (nil))

>

> This looks like due to a typo in the md patterns. Attached patch fixes

> this. Bootsrapped and regression tested on arm-linux-gnueabihf without

> any regressions.  Is this OK for trunk?

>

> Thanks,

> Kugan

>

> gcc/ChangeLog:

>

> 2019-10-10  kugan.vivekanandarajah  <kugan.vivekanandarajah@linaro.org>


I think the ChangeLog rules require to put your name with a space and 
capitalised first letters.


>      * config/arm/vfp.md (fma<SDF:mode>4): Enable DF only when

>      TARGET_VFP_DOUBLE.

>      (*fmsub<SDF:mode>4): Likewise.

>      (*fnmsub<SDF:mode>4): Likewise.

>      (*fnmadd<SDF:mode>4): Likewise.



diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 661919e2357..1979aa6fdb4 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -1321,7 +1321,7 @@
          (fma:SDF (match_operand:SDF 1 "register_operand" "<F_constraint>")
  		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
  		 (match_operand:SDF 3 "register_operand" "0")))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
    "vfma%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
    [(set_attr "predicable" "yes")
     (set_attr "type" "ffma<vfp_type>")]


I'm a bit surprised that TARGET_FMA (which just checks isa_bit_vfpv4) doesn't imply TARGET_VFP_DOUBLE.
Can one really have a VFPV4 single-precision-only configuration? Richard?

Thanks,
Kyrill


  @@ -1357,7 +1357,7 @@
  					     "<F_constraint>"))
  		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
  		 (match_operand:SDF 3 "register_operand" "0")))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
    "vfms%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
    [(set_attr "predicable" "yes")
     (set_attr "type" "ffma<vfp_type>")]
@@ -1379,7 +1379,7 @@
  	(fma:SDF (match_operand:SDF 1 "register_operand" "<F_constraint>")
  		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
  		 (neg:SDF (match_operand:SDF 3 "register_operand" "0"))))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
    "vfnms%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
    [(set_attr "predicable" "yes")
     (set_attr "type" "ffma<vfp_type>")]
@@ -1402,7 +1402,7 @@
  					       "<F_constraint>"))
  		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
  		 (neg:SDF (match_operand:SDF 3 "register_operand" "0"))))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
    "vfnma%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
    [(set_attr "predicable" "yes")
     (set_attr "type" "ffma<vfp_type>")]
Andre Vieira (lists) Oct. 10, 2019, 10:28 a.m. | #2
Hi Kyrill,

On 10/10/2019 10:53, Kyrill Tkachov wrote:

> 

> 

> I'm a bit surprised that TARGET_FMA (which just checks isa_bit_vfpv4) 

> doesn't imply TARGET_VFP_DOUBLE.

> Can one really have a VFPV4 single-precision-only configuration? Richard?

> 


Armv7e-M supports single precision only FPv4, which also includes a 
single precision only VFMA instruction. So I think this is correct.

See Section A.7.7.233 VFMA, VFMS in 
https://static.docs.arm.com/ddi0403/ed/DDI0403E_d_armv7m_arm.pdf

You can target this variant using one of the following three(six):
'-march=armv7e-m+fp -mfpu=auto -mfloat-abi={softfp,hard}'
'-march=cortex-m4 -mfpu=auto -mfloat-abi={softfp,hard}'
'-march=cortex-m7+nofp.dp -mfpu=auto -mfloat-abi={softfp,hard}'

Cheers,
Andre
Kyrill Tkachov Oct. 10, 2019, 10:29 a.m. | #3
Hi Andre,

On 10/10/19 11:28 AM, Andre Vieira (lists) wrote:
> Hi Kyrill,

>

> On 10/10/2019 10:53, Kyrill Tkachov wrote:

>

> >

> >

> > I'm a bit surprised that TARGET_FMA (which just checks isa_bit_vfpv4)

> > doesn't imply TARGET_VFP_DOUBLE.

> > Can one really have a VFPV4 single-precision-only configuration? 

> Richard?

> >

>

> Armv7e-M supports single precision only FPv4, which also includes a

> single precision only VFMA instruction. So I think this is correct.

>

> See Section A.7.7.233 VFMA, VFMS in

> https://static.docs.arm.com/ddi0403/ed/DDI0403E_d_armv7m_arm.pdf

>

> You can target this variant using one of the following three(six):

> '-march=armv7e-m+fp -mfpu=auto -mfloat-abi={softfp,hard}'

> '-march=cortex-m4 -mfpu=auto -mfloat-abi={softfp,hard}'

> '-march=cortex-m7+nofp.dp -mfpu=auto -mfloat-abi={softfp,hard}'

>

Ah, thanks.

Kugan, your patch is ok then with the ChangeLog fixed.

Thanks,

Kyrill

> Cheers,

> Andre

Patch

diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 661919e2357..1979aa6fdb4 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -1321,7 +1321,7 @@ 
         (fma:SDF (match_operand:SDF 1 "register_operand" "<F_constraint>")
 		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
 		 (match_operand:SDF 3 "register_operand" "0")))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
   "vfma%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
   [(set_attr "predicable" "yes")
    (set_attr "type" "ffma<vfp_type>")]
@@ -1357,7 +1357,7 @@ 
 					     "<F_constraint>"))
 		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
 		 (match_operand:SDF 3 "register_operand" "0")))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
   "vfms%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
   [(set_attr "predicable" "yes")
    (set_attr "type" "ffma<vfp_type>")]
@@ -1379,7 +1379,7 @@ 
 	(fma:SDF (match_operand:SDF 1 "register_operand" "<F_constraint>")
 		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
 		 (neg:SDF (match_operand:SDF 3 "register_operand" "0"))))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
   "vfnms%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
   [(set_attr "predicable" "yes")
    (set_attr "type" "ffma<vfp_type>")]
@@ -1402,7 +1402,7 @@ 
 					       "<F_constraint>"))
 		 (match_operand:SDF 2 "register_operand" "<F_constraint>")
 		 (neg:SDF (match_operand:SDF 3 "register_operand" "0"))))]
-  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA"
+  "TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_FMA <vfp_double_cond>"
   "vfnma%?.<V_if_elem>\\t%<V_reg>0, %<V_reg>1, %<V_reg>2"
   [(set_attr "predicable" "yes")
    (set_attr "type" "ffma<vfp_type>")]