[PATCH][AArch64] Restrict usage of FP/SIMD registers for TImode reload and absdi2 patterns for non-float/simd targets

[PATCH][AArch64] Restrict usage of FP/SIMD registers for TImode reload and absdi2 patterns for non-float/simd targets

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

Hi all,

This patch arises from PR 62014 where apparently gcc generates usage of 
FP registers with -mgeneral-regs-only. The PR turned out to be bogus in 
the end that but an inspection of aarch64.md shows that there are some 
patterns that don't have their usage of FP/SIMD registers properly 
guarded by the simd attribute or by the TARGET_FLOAT predicate. This 
patch addresses that, although I could not come up with a testcase that 
demonstrated wrong behaviour.

I built the linux kernel with this patch and looked for fmov 
instructions in the disassembly. They appeared only in the crypto code 
that uses the new AES instructions and therefore allows usage of vector 
registers.
But even without this patch the kernel compiled to an identical binary 
as with this patch (phew!)

I've added a comment that hopefully clarifies the usage of the fp and 
simd attributes.

Bootstrapped on aarch64-linux and tested on aarch64-none-elf as well.

Ok for trunk?

Thanks,
Kyrill

2014-08-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.md (absdi2): Set simd attribute.
     (aarch64_reload_mov<mode>): Predicate on TARGET_FLOAT.
     (aarch64_movdi_<mode>high): Likewise.
     (aarch64_mov<mode>high_di): Likewise.
     (aarch64_movdi_<mode>low): Likewise.
     (aarch64_mov<mode>low_di): Likewise.
     (aarch64_movtilow_tilow): Likewise.
     Add comment explaining usage of fp,simd attributes and of
     TARGET_FLOAT and TARGET_SIMD.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-restrict-simd.patch --]
[-- Type: text/x-patch; name=aarch64-restrict-simd.patch, Size: 3996 bytes --]

commit 4bc3f5d3da9450bd0748ab4a61a48c739586fd3c
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Aug 5 12:20:23 2014 +0100

    [AArch64] Restrict FP/SIMD reg usage on non-fp/simd targets

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 59c4ba4..e9758a8 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -141,12 +141,22 @@
 ; to share pipeline descriptions.
 (include "../arm/types.md")
 
+;; It is important to set the fp or simd attributes to yes when a pattern
+;; alternative uses the FP or SIMD register files, usually signified by use of
+;; the 'w' constraint.  This will ensure that the alternative will be
+;; disabled when compiling with -mgeneral-regs-only or with the +nofp/+nosimd
+;; architecture extensions.  If all the alternatives in a pattern use the
+;; FP or SIMD registers then the pattern predicate should include TARGET_FLOAT
+;; or TARGET_SIMD.
+
 ;; Attribute that specifies whether or not the instruction touches fp
-;; registers.
+;; registers.  When this is set to yes for an alternative, that alternative
+;; will be disabled when !TARGET_FLOAT.
 (define_attr "fp" "no,yes" (const_string "no"))
 
 ;; Attribute that specifies whether or not the instruction touches simd
-;; registers.
+;; registers.  When this is set to yes for an alternative, that alternative
+;; will be disabled when !TARGET_SIMD.
 (define_attr "simd" "no,yes" (const_string "no"))
 
 (define_attr "length" ""
@@ -1954,7 +1964,8 @@
 							     GEN_INT (63)))));
     DONE;
   }
-  [(set_attr "type" "alu_sreg")]
+  [(set_attr "type" "alu_sreg")
+   (set_attr "simd" "no,yes")]
 )
 
 (define_insn "neg<mode>2"
@@ -3728,7 +3739,7 @@
         (match_operand:TX 1 "register_operand" "w"))
    (clobber (match_operand:DI 2 "register_operand" "=&r"))
   ]
-  ""
+  "TARGET_FLOAT"
   {
     rtx op0 = simplify_gen_subreg (TImode, operands[0], <MODE>mode, 0);
     rtx op1 = simplify_gen_subreg (TImode, operands[1], <MODE>mode, 0);
@@ -3746,7 +3757,7 @@
 (define_insn "aarch64_movdi_<mode>low"
   [(set (match_operand:DI 0 "register_operand" "=r")
         (truncate:DI (match_operand:TX 1 "register_operand" "w")))]
-  "reload_completed || reload_in_progress"
+  "TARGET_FLOAT && (reload_completed || reload_in_progress)"
   "fmov\\t%x0, %d1"
   [(set_attr "type" "f_mrc")
    (set_attr "length" "4")
@@ -3757,7 +3768,7 @@
         (truncate:DI
 	  (lshiftrt:TX (match_operand:TX 1 "register_operand" "w")
 		       (const_int 64))))]
-  "reload_completed || reload_in_progress"
+  "TARGET_FLOAT && (reload_completed || reload_in_progress)"
   "fmov\\t%x0, %1.d[1]"
   [(set_attr "type" "f_mrc")
    (set_attr "length" "4")
@@ -3767,7 +3778,7 @@
   [(set (zero_extract:TX (match_operand:TX 0 "register_operand" "+w")
                          (const_int 64) (const_int 64))
         (zero_extend:TX (match_operand:DI 1 "register_operand" "r")))]
-  "reload_completed || reload_in_progress"
+  "TARGET_FLOAT && (reload_completed || reload_in_progress)"
   "fmov\\t%0.d[1], %x1"
   [(set_attr "type" "f_mcr")
    (set_attr "length" "4")
@@ -3776,7 +3787,7 @@
 (define_insn "aarch64_mov<mode>low_di"
   [(set (match_operand:TX 0 "register_operand" "=w")
         (zero_extend:TX (match_operand:DI 1 "register_operand" "r")))]
-  "reload_completed || reload_in_progress"
+  "TARGET_FLOAT && (reload_completed || reload_in_progress)"
   "fmov\\t%d0, %x1"
   [(set_attr "type" "f_mcr")
    (set_attr "length" "4")
@@ -3784,9 +3795,9 @@
 
 (define_insn "aarch64_movtilow_tilow"
   [(set (match_operand:TI 0 "register_operand" "=w")
-        (zero_extend:TI 
+        (zero_extend:TI
 	  (truncate:DI (match_operand:TI 1 "register_operand" "w"))))]
-  "reload_completed || reload_in_progress"
+  "TARGET_FLOAT && (reload_completed || reload_in_progress)"
   "fmov\\t%d0, %d1"
   [(set_attr "type" "fmov")
    (set_attr "length" "4")

Re: [PATCH][AArch64] Restrict usage of FP/SIMD registers for TImode reload and absdi2 patterns for non-float/simd targets

[Richard Earnshaw]

On 07/08/14 12:22, Kyrill Tkachov wrote:
> Hi all,
> 
> This patch arises from PR 62014 where apparently gcc generates usage of 
> FP registers with -mgeneral-regs-only. The PR turned out to be bogus in 
> the end that but an inspection of aarch64.md shows that there are some 
> patterns that don't have their usage of FP/SIMD registers properly 
> guarded by the simd attribute or by the TARGET_FLOAT predicate. This 
> patch addresses that, although I could not come up with a testcase that 
> demonstrated wrong behaviour.
> 
> I built the linux kernel with this patch and looked for fmov 
> instructions in the disassembly. They appeared only in the crypto code 
> that uses the new AES instructions and therefore allows usage of vector 
> registers.
> But even without this patch the kernel compiled to an identical binary 
> as with this patch (phew!)
> 
> I've added a comment that hopefully clarifies the usage of the fp and 
> simd attributes.
> 
> Bootstrapped on aarch64-linux and tested on aarch64-none-elf as well.
> 
> Ok for trunk?
> 
> Thanks,
> Kyrill
> 
> 2014-08-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      * config/aarch64/aarch64.md (absdi2): Set simd attribute.
>      (aarch64_reload_mov<mode>): Predicate on TARGET_FLOAT.
>      (aarch64_movdi_<mode>high): Likewise.
>      (aarch64_mov<mode>high_di): Likewise.
>      (aarch64_movdi_<mode>low): Likewise.
>      (aarch64_mov<mode>low_di): Likewise.
>      (aarch64_movtilow_tilow): Likewise.
>      Add comment explaining usage of fp,simd attributes and of
>      TARGET_FLOAT and TARGET_SIMD.
> 

OK.

R.