diff mbox

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

Message ID 53E36161.1080800@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov Aug. 7, 2014, 11:22 a.m. UTC
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.

Comments

Richard Earnshaw Aug. 7, 2014, 2:23 p.m. UTC | #1
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.
diff mbox

Patch

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")