[PATCH][arm/AArch64] Assume unhandled NEON types are neon_arith_basic types when scheduling for Cortex-A5

[PATCH][arm/AArch64] Assume unhandled NEON types are neon_arith_basic types when scheduling for Cortex-A5

[Kyrill Tkachov]

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

Hi all,

Some scheduling descriptions, like the Cortex-A57 one, are reused for 
multiple -mcpu options.
Sometimes those other -mcpu cores support more architecture features 
than the Armv8-A Cortex-A57.
For example, the Cortex-A75 and Cortex-A76 support Armv8.2-A as well as 
the Dot Product instructions.
These Dot Product instructions have the neon_dot and neon_dot_q 
scheduling type, but that type is not
handled in cortex-a57.md, since the Cortex-A57 itself doesn't need to 
care about these instructions.

But if we just ignore the neon_dot(_q) type at scheduling we get really 
terrible codegen when compiling
for -mcpu=cortex-a76, for example, because the scheduler just pools all 
the UDOT instructions at the end
of the basic block, since it doesn't assume anything about their behaviour.

This patch ameliorates the situation somewhat by telling the Cortex-A57 
scheduling model to treat any
insn that doesn't get assigned a cortex_a57_neon_type but is actually a 
is_neon_type instruction as
a simple neon_arith_basic instruction. This allows us to treat 
post-Armv8-A SIMD instructions more sanely
without having to model each of them explicitly in cortex-a57.md.

Bootstrapped and tested on arm-none-linux-gnueabihf and 
aarch64-none-linux-gnu.

Ok for trunk from an aarch64 perspective?

Thanks,
Kyrill

2019-01-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/cortex-a57.md (cortex_a57_neon_type): Use neon_arith_basic
     for is_neon_type instructions that have not already been categorized.


[-- Attachment #2: neon-types.patch --]
[-- Type: text/x-patch, Size: 947 bytes --]

diff --git a/gcc/config/arm/cortex-a57.md b/gcc/config/arm/cortex-a57.md
index 6ba4f5711cf4b1127ff6cc2e59f1fa9dbd25a9b1..d1ea2aa1edbec1981c68d2e54c5ae6dfe0fec944 100644
--- a/gcc/config/arm/cortex-a57.md
+++ b/gcc/config/arm/cortex-a57.md
@@ -236,7 +236,12 @@ (define_attr "cortex_a57_neon_type"
 			   neon_store1_4reg, neon_store1_4reg_q,\
 			   neon_store1_one_lane, neon_store1_one_lane_q,\
 			   neon_store2_one_lane, neon_store2_one_lane_q")
-	    (const_string "neon_store_complex")]
+	    (const_string "neon_store_complex")
+;; If it doesn't match any of the above that we want to treat specially but is
+;; still a NEON type, treat it as a basic NEON type.  This is better than
+;; dropping it on the floor and making no assumptions about it whatsoever.
+	  (eq_attr "is_neon_type" "yes")
+	    (const_string "neon_arith_basic")]
 	  (const_string "unknown")))
 
 ;; The Cortex-A57 core is modelled as a triple issue pipeline that has

Re: [PATCH][arm/AArch64] Assume unhandled NEON types are neon_arith_basic types when scheduling for Cortex-A57

[Kyrill Tkachov]

Something somewhere cut off the subject line: it should say Cortex-A57.

Sorry about that.

Kyrill

On 7/1/19 4:13 PM, Kyrill Tkachov wrote:
> Hi all,
>
> Some scheduling descriptions, like the Cortex-A57 one, are reused for
> multiple -mcpu options.
> Sometimes those other -mcpu cores support more architecture features
> than the Armv8-A Cortex-A57.
> For example, the Cortex-A75 and Cortex-A76 support Armv8.2-A as well as
> the Dot Product instructions.
> These Dot Product instructions have the neon_dot and neon_dot_q
> scheduling type, but that type is not
> handled in cortex-a57.md, since the Cortex-A57 itself doesn't need to
> care about these instructions.
>
> But if we just ignore the neon_dot(_q) type at scheduling we get really
> terrible codegen when compiling
> for -mcpu=cortex-a76, for example, because the scheduler just pools all
> the UDOT instructions at the end
> of the basic block, since it doesn't assume anything about their 
> behaviour.
>
> This patch ameliorates the situation somewhat by telling the Cortex-A57
> scheduling model to treat any
> insn that doesn't get assigned a cortex_a57_neon_type but is actually a
> is_neon_type instruction as
> a simple neon_arith_basic instruction. This allows us to treat
> post-Armv8-A SIMD instructions more sanely
> without having to model each of them explicitly in cortex-a57.md.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf and
> aarch64-none-linux-gnu.
>
> Ok for trunk from an aarch64 perspective?
>
> Thanks,
> Kyrill
>
> 2019-01-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>      * config/arm/cortex-a57.md (cortex_a57_neon_type): Use 
> neon_arith_basic
>      for is_neon_type instructions that have not already been categorized.
>

Re: [PATCH][arm/AArch64] Assume unhandled NEON types are neon_arith_basic types when scheduling for Cortex-A5

[James Greenhalgh]

On Mon, Jul 01, 2019 at 04:13:40PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> Some scheduling descriptions, like the Cortex-A57 one, are reused for 
> multiple -mcpu options.
> Sometimes those other -mcpu cores support more architecture features 
> than the Armv8-A Cortex-A57.
> For example, the Cortex-A75 and Cortex-A76 support Armv8.2-A as well as 
> the Dot Product instructions.
> These Dot Product instructions have the neon_dot and neon_dot_q 
> scheduling type, but that type is not
> handled in cortex-a57.md, since the Cortex-A57 itself doesn't need to 
> care about these instructions.
> 
> But if we just ignore the neon_dot(_q) type at scheduling we get really 
> terrible codegen when compiling
> for -mcpu=cortex-a76, for example, because the scheduler just pools all 
> the UDOT instructions at the end
> of the basic block, since it doesn't assume anything about their behaviour.
> 
> This patch ameliorates the situation somewhat by telling the Cortex-A57 
> scheduling model to treat any
> insn that doesn't get assigned a cortex_a57_neon_type but is actually a 
> is_neon_type instruction as
> a simple neon_arith_basic instruction. This allows us to treat 
> post-Armv8-A SIMD instructions more sanely
> without having to model each of them explicitly in cortex-a57.md.
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf and 
> aarch64-none-linux-gnu.
> 
> Ok for trunk from an aarch64 perspective?

OK.

Thansk,
James

> 
> Thanks,
> Kyrill
>