On 26/05/16 11:17, James Greenhalgh wrote:
On Fri, May 20, 2016 at 11:04:32AM +0100, Kyrill Tkachov wrote:
Hi all,
The recent -frename-registers change exposed a deficiency in the way we fuse
AESE/AESMC instruction pairs in aarch64.
Basically we want to enforce:
AESE Vn, _
AESMC Vn, Vn
to enable the fusion, but regrename comes along and renames the output Vn
register in AESMC to something else, killing the fusion in the hardware.
The solution in this patch is to add an alternative that ties the input and
output registers in the AESMC pattern and enable that alternative when the
fusion is enabled.
With this patch I've confirmed that the above preferred register sequence is
kept even with -frename-registers when tuning for a cpu that enables the
fusion and that the chain is broken by regrename otherwise and have seen the
appropriate improvement in a proprietary benchmark (that I cannot name) that
exercises this sequence.
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?
Thanks,
Kyrill
2016-05-20 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
* config/aarch64/aarch64.c (aarch64_fusion_enabled_p): New function.
* config/aarch64/aarch64-protos.h (aarch64_fusion_enabled_p): Declare
prototype.
* config/aarch64/aarch64-simd.md (aarch64_crypto_aes<aesmc_op>v16qi):
Add "=w,0" alternative. Enable it when AES/AESMC fusion is enabled.
diff --git a/gcc/config/aarch64/aarch64-protos.h
b/gcc/config/aarch64/aarch64-protos.h
index
21cf55b60f86024429ea36ead0d2d8ae4c94b579..f6da854fbaeeab34239a1f874edaedf8a01bf9c2
100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -290,6 +290,7 @@ bool aarch64_constant_address_p (rtx);
bool aarch64_expand_movmem (rtx *);
bool aarch64_float_const_zero_rtx_p (rtx);
bool aarch64_function_arg_regno_p (unsigned);
+bool aarch64_fusion_enabled_p (unsigned int);
This argument type should be "enum aarch64_fusion_pairs".
bool aarch64_gen_movmemqi (rtx *);
bool aarch64_gimple_fold_builtin (gimple_stmt_iterator *);
bool aarch64_is_extend_from_extract (machine_mode, rtx, rtx);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
b93f961fc4ebd9eb3f50b0580741c80ab6eca427..815973ca6e764121f2669ad160918561450e6c50
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13359,6 +13359,14 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn
*curr)
return false;
}
+/* Return true iff the instruction fusion described by OP is enabled. */
+
+bool
+aarch64_fusion_enabled_p (unsigned int op)
+{
+ return (aarch64_tune_params.fusible_ops & op) != 0;
+}
+
A follow-up patch fixing the uses in aarch_macro_fusion_pair_p to use your
new function would be nice.
OK with the change to argument type.
Thanks, here is what I'm committing.
The patch to use the new function in aarch_macro_fusion_pair_p is in testing.
Kyrill
2016-05-27 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
* config/aarch64/aarch64.c (aarch64_fusion_enabled_p): New function.
* config/aarch64/aarch64-protos.h (aarch64_fusion_enabled_p): Declare
prototype.
* config/aarch64/aarch64-simd.md (aarch64_crypto_aes<aesmc_op>v16qi):
Add "=w,0" alternative. Enable it when AES/AESMC fusion is enabled.
Thanks,
James
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 98c81baad827f3c0b0c7054dd97d3d0989af3796..6e97c01cf768ec5ca2f18e795a8085b8a247f5b6 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -290,6 +290,7 @@ bool aarch64_constant_address_p (rtx);
bool aarch64_expand_movmem (rtx *);
bool aarch64_float_const_zero_rtx_p (rtx);
bool aarch64_function_arg_regno_p (unsigned);
+bool aarch64_fusion_enabled_p (enum aarch64_fusion_pairs);
bool aarch64_gen_movmemqi (rtx *);
bool aarch64_gimple_fold_builtin (gimple_stmt_iterator *);
bool aarch64_is_extend_from_extract (machine_mode, rtx, rtx);
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 59a578f5937a240b325af22021bbd662230ed404..8be69357eb086e288ae838dc536be8a2ebe0463b 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -5401,13 +5401,25 @@ (define_insn "aarch64_crypto_aes<aes_op>v16qi"
[(set_attr "type" "crypto_aese")]
)
+;; When AES/AESMC fusion is enabled we want the register allocation to
+;; look like:
+;; AESE Vn, _
+;; AESMC Vn, Vn
+;; So prefer to tie operand 1 to operand 0 when fusing.
+
(define_insn "aarch64_crypto_aes<aesmc_op>v16qi"
- [(set (match_operand:V16QI 0 "register_operand" "=w")
- (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "w")]
+ [(set (match_operand:V16QI 0 "register_operand" "=w,w")
+ (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0,w")]
CRYPTO_AESMC))]
"TARGET_SIMD && TARGET_CRYPTO"
"aes<aesmc_op>\\t%0.16b, %1.16b"
- [(set_attr "type" "crypto_aesmc")]
+ [(set_attr "type" "crypto_aesmc")
+ (set_attr_alternative "enabled"
+ [(if_then_else (match_test
+ "aarch64_fusion_enabled_p (AARCH64_FUSE_AES_AESMC)")
+ (const_string "yes" )
+ (const_string "no"))
+ (const_string "yes")])]
)
;; sha1
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1784b9215b3ada0dd944838b7ca9f9dbfaf750ec..a19855e605d77cfa2221f99759486432963b6f98 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13235,6 +13235,14 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
return false;
}
+/* Return true iff the instruction fusion described by OP is enabled. */
+
+bool
+aarch64_fusion_enabled_p (enum aarch64_fusion_pairs op)
+{
+ return (aarch64_tune_params.fusible_ops & op) != 0;
+}
+
/* If MEM is in the form of [base+offset], extract the two parts
of address and set to BASE and OFFSET, otherwise return false
after clearing BASE and OFFSET. */
