diff mbox

[AArch64] Add aes and sha reservations for Thunderx2t99

Message ID CO2PR07MB26945BE5C89873CA5C309A27832C0@CO2PR07MB2694.namprd07.prod.outlook.com
State New
Headers show

Commit Message

Hurugalawadi, Naveen March 6, 2017, 5:09 a.m. UTC
Hi,

Please find attached the patch that adds aes and sha reservations for
Thunderx2t99.

Bootstrapped and Regression tested on aarch64-thunder-linux.
Please review the patch and let us know if its okay for Stage-1?

Thanks,
Naveen

2017-03-06  Julian Brown  <julian@codesourcery.com>
	    Naveen H.S  <Naveen.Hurugalawadi@cavium.com>

	* config/aarch64/thunderx2t99.md (thunderx2t99_crc): New Reservation.

Comments

James Greenhalgh March 8, 2017, 5:39 p.m. UTC | #1
On Mon, Mar 06, 2017 at 05:09:58AM +0000, Hurugalawadi, Naveen wrote:
> Hi,
> 
> Please find attached the patch that adds aes and sha reservations for
> Thunderx2t99.
> 
> Bootstrapped and Regression tested on aarch64-thunder-linux.
> Please review the patch and let us know if its okay for Stage-1?
> 
> Thanks,
> Naveen
> 
> 2017-03-06  Julian Brown  <julian@codesourcery.com>
> 	    Naveen H.S  <Naveen.Hurugalawadi@cavium.com>
> 
> 	* config/aarch64/thunderx2t99.md (thunderx2t99_crc): New Reservation.

> diff --git a/gcc/config/aarch64/thunderx2t99.md b/gcc/config/aarch64/thunderx2t99.md
> index f807547..2eb136b 100644
> --- a/gcc/config/aarch64/thunderx2t99.md
> +++ b/gcc/config/aarch64/thunderx2t99.md
> @@ -443,7 +443,22 @@
>         (eq_attr "type" "neon_store2_one_lane,neon_store2_one_lane_q"))
>    "thunderx2t99_ls01,thunderx2t99_f01")
>  
> +;; Crypto extensions.
> +
> +; FIXME: Forwarding path for aese/aesmc or aesd/aesimc pairs?

Do you need these if you've enabled instruction fusion?

I'm not sure on the exact mechanics of how instruction fusion works, and
whether you'd get further scheduling benefits from modeling a forwarding
path if one exists.

Regardless, this patch is OK for Stage 1.

Thanks,
James

> +(define_insn_reservation "thunderx2t99_aes" 5
> +  (and (eq_attr "tune" "thunderx2t99")
> +       (eq_attr "type" "crypto_aese,crypto_aesmc"))
> +  "thunderx2t99_f1")
> +
>  (define_insn_reservation "thunderx2t99_pmull" 5
>    (and (eq_attr "tune" "thunderx2t99")
>         (eq_attr "type" "crypto_pmull"))
>    "thunderx2t99_f1")
> +
> +(define_insn_reservation "thunderx2t99_sha" 7
> +  (and (eq_attr "tune" "thunderx2t99")
> +       (eq_attr "type" "crypto_sha1_fast,crypto_sha1_xor,crypto_sha1_slow,\
> +			crypto_sha256_fast,crypto_sha256_slow"))
> +  "thunderx2t99_f1")
Kyrill Tkachov March 9, 2017, 10:01 a.m. UTC | #2
On 08/03/17 17:39, James Greenhalgh wrote:
> On Mon, Mar 06, 2017 at 05:09:58AM +0000, Hurugalawadi, Naveen wrote:
>> Hi,
>>
>> Please find attached the patch that adds aes and sha reservations for
>> Thunderx2t99.
>>
>> Bootstrapped and Regression tested on aarch64-thunder-linux.
>> Please review the patch and let us know if its okay for Stage-1?
>>
>> Thanks,
>> Naveen
>>
>> 2017-03-06  Julian Brown  <julian@codesourcery.com>
>> 	    Naveen H.S  <Naveen.Hurugalawadi@cavium.com>
>>
>> 	* config/aarch64/thunderx2t99.md (thunderx2t99_crc): New Reservation.
>> diff --git a/gcc/config/aarch64/thunderx2t99.md b/gcc/config/aarch64/thunderx2t99.md
>> index f807547..2eb136b 100644
>> --- a/gcc/config/aarch64/thunderx2t99.md
>> +++ b/gcc/config/aarch64/thunderx2t99.md
>> @@ -443,7 +443,22 @@
>>          (eq_attr "type" "neon_store2_one_lane,neon_store2_one_lane_q"))
>>     "thunderx2t99_ls01,thunderx2t99_f01")
>>   
>> +;; Crypto extensions.
>> +
>> +; FIXME: Forwarding path for aese/aesmc or aesd/aesimc pairs?
> Do you need these if you've enabled instruction fusion?

I'd recommend it if one wants to bring the pairs together.

> I'm not sure on the exact mechanics of how instruction fusion works, and
> whether you'd get further scheduling benefits from modeling a forwarding
> path if one exists.

The (macro) fusion mechanism in the scheduler makes sure to keep
the pairs together if they appear together before scheduling, but doesn't
do anything to actually bring them together if they are far apart.
So having bypasses would encourage the scheduler to form pairs more aggressively.

(These are not objections to the patch, just some background)

Kyrill

>
> Regardless, this patch is OK for Stage 1.
>
> Thanks,
> James
>
>> +(define_insn_reservation "thunderx2t99_aes" 5
>> +  (and (eq_attr "tune" "thunderx2t99")
>> +       (eq_attr "type" "crypto_aese,crypto_aesmc"))
>> +  "thunderx2t99_f1")
>> +
>>   (define_insn_reservation "thunderx2t99_pmull" 5
>>     (and (eq_attr "tune" "thunderx2t99")
>>          (eq_attr "type" "crypto_pmull"))
>>     "thunderx2t99_f1")
>> +
>> +(define_insn_reservation "thunderx2t99_sha" 7
>> +  (and (eq_attr "tune" "thunderx2t99")
>> +       (eq_attr "type" "crypto_sha1_fast,crypto_sha1_xor,crypto_sha1_slow,\
>> +			crypto_sha256_fast,crypto_sha256_slow"))
>> +  "thunderx2t99_f1")
James Greenhalgh March 9, 2017, 10:30 a.m. UTC | #3
On Thu, Mar 09, 2017 at 10:01:57AM +0000, Kyrill Tkachov wrote:
> 
> On 08/03/17 17:39, James Greenhalgh wrote:
> >On Mon, Mar 06, 2017 at 05:09:58AM +0000, Hurugalawadi, Naveen wrote:
> >>Hi,
> >>
> >>Please find attached the patch that adds aes and sha reservations for
> >>Thunderx2t99.
> >>
> >>Bootstrapped and Regression tested on aarch64-thunder-linux.
> >>Please review the patch and let us know if its okay for Stage-1?
> >>
> >>Thanks,
> >>Naveen
> >>
> >>2017-03-06  Julian Brown  <julian@codesourcery.com>
> >>	    Naveen H.S  <Naveen.Hurugalawadi@cavium.com>
> >>
> >>	* config/aarch64/thunderx2t99.md (thunderx2t99_crc): New Reservation.
> >>diff --git a/gcc/config/aarch64/thunderx2t99.md b/gcc/config/aarch64/thunderx2t99.md
> >>index f807547..2eb136b 100644
> >>--- a/gcc/config/aarch64/thunderx2t99.md
> >>+++ b/gcc/config/aarch64/thunderx2t99.md
> >>@@ -443,7 +443,22 @@
> >>         (eq_attr "type" "neon_store2_one_lane,neon_store2_one_lane_q"))
> >>    "thunderx2t99_ls01,thunderx2t99_f01")
> >>+;; Crypto extensions.
> >>+
> >>+; FIXME: Forwarding path for aese/aesmc or aesd/aesimc pairs?
> >Do you need these if you've enabled instruction fusion?
> 
> I'd recommend it if one wants to bring the pairs together.
> 
> >I'm not sure on the exact mechanics of how instruction fusion works, and
> >whether you'd get further scheduling benefits from modeling a forwarding
> >path if one exists.
> 
> The (macro) fusion mechanism in the scheduler makes sure to keep
> the pairs together if they appear together before scheduling, but doesn't
> do anything to actually bring them together if they are far apart.
> So having bypasses would encourage the scheduler to form pairs more
> aggressively.

Thanks for the explanation, that is useful to know.

James
diff mbox

Patch

diff --git a/gcc/config/aarch64/thunderx2t99.md b/gcc/config/aarch64/thunderx2t99.md
index f807547..2eb136b 100644
--- a/gcc/config/aarch64/thunderx2t99.md
+++ b/gcc/config/aarch64/thunderx2t99.md
@@ -443,7 +443,22 @@ 
        (eq_attr "type" "neon_store2_one_lane,neon_store2_one_lane_q"))
   "thunderx2t99_ls01,thunderx2t99_f01")
 
+;; Crypto extensions.
+
+; FIXME: Forwarding path for aese/aesmc or aesd/aesimc pairs?
+
+(define_insn_reservation "thunderx2t99_aes" 5
+  (and (eq_attr "tune" "thunderx2t99")
+       (eq_attr "type" "crypto_aese,crypto_aesmc"))
+  "thunderx2t99_f1")
+
 (define_insn_reservation "thunderx2t99_pmull" 5
   (and (eq_attr "tune" "thunderx2t99")
        (eq_attr "type" "crypto_pmull"))
   "thunderx2t99_f1")
+
+(define_insn_reservation "thunderx2t99_sha" 7
+  (and (eq_attr "tune" "thunderx2t99")
+       (eq_attr "type" "crypto_sha1_fast,crypto_sha1_xor,crypto_sha1_slow,\
+			crypto_sha256_fast,crypto_sha256_slow"))
+  "thunderx2t99_f1")