Re: [PATCH][AArch64] Use new target pass registration framework for FMA steering pass

[James Greenhalgh]

On Thu, Oct 13, 2016 at 06:14:16PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> This patch moves the aarch64-specific FMA steering pass registration into the 
> new framework
> that Jakub introduced. With this patch the RTL dump for the steering pass is 
> now numbered properly
> so that it appears after the regrename pass, rather than getting a dump 
> number that puts it after
> all the other passes.
> 
> I've followed a similar approach to [1] and added an aarch64-passes.def file 
> and updated
> PASSES_EXTRA in t-aarch64. I deleted cortex-a57-fma-steering.h as I don't 
> think it adds any value.
> The prototype for the make_pass* function works just as well in 
> aarch64-protos.h I think.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Manually checked that the pass still runs when tuning for Cortex-A57 and 
> doesn't run otherwise.
> 
> Ok for trunk?

OK, a comment on git diffs below that doesn't change the patch content.

> [1] https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00615.html
> 
> 2016-10-13  Kyrylo Tkachov  
> 
> * config/aarch64/aarch64.c: Delete inclusion of
> cortex-a57-fma-steering.h.
> (aarch64_override_options): Delete call
> to aarch64_register_fma_steering.
> * config/aarch64/aarch64-protos.h (make_pass_fma_steering): Declare.
> * config/aarch64/cortex-a57-fma-steering.h: Delete.
> * config/aarch64/aarch64-passes.def: New file.
> * config/aarch64/cortex-a57-fma-steering.c
> (aarch64_register_fma_steering): Delete definition.
> (make_pass_fma_steering): Remove static qualifier.
> * config/aarch64/t-aarch64 (PASSES_EXTRA): New directive.
> (cortex-a57-fma-steering.o): Remove dependency on
> cortex-a57-fma-steering.h.


> diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.h 
> b/gcc/config/aarch64/aarch64-passes.def
> similarity index 78%
> rename from gcc/config/aarch64/cortex-a57-fma-steering.h
> rename to gcc/config/aarch64/aarch64-passes.def
> index 
> 65bf5acc132d2db645d1b00ef031dc33a195bb78..7fe80391a3fb0dc79715b9fb23fd4c08a9d26d74
>  100644
> --- a/gcc/config/aarch64/cortex-a57-fma-steering.h
> +++ b/gcc/config/aarch64/aarch64-passes.def
> @@ -1,6 +1,5 @@
> -/* This file contains declarations for the FMA steering optimization
> -   pass for Cortex-A57.
> -   Copyright (C) 2015-2016 Free Software Foundation, Inc.
> +/* AArch64-specific passes declarations.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> Contributed by ARM Ltd.
>  
> This file is part of GCC.
> @@ -19,4 +18,4 @@
> along with GCC; see the file COPYING3.  If not see
> .  */
>  
> -void aarch64_register_fma_steering (void);
> +INSERT_PASS_AFTER (pass_regrename, 1, pass_fma_steering);

A technicality on your git diff format, this should not be a rename.

Just make sure when you apply it to svn you accurately record a delete of
the old file, and creation of the new file.

Thanks,
James

[PATCH][AArch64] Use new target pass registration framework for FMA steering pass

[Kyrill Tkachov]

Hi all,

This patch moves the aarch64-specific FMA steering pass registration into the 
new framework
that Jakub introduced. With this patch the RTL dump for the steering pass is 
now numbered properly
so that it appears after the regrename pass, rather than getting a dump number 
that puts it after
all the other passes.

I've followed a similar approach to [1] and added an aarch64-passes.def file 
and updated
PASSES_EXTRA in t-aarch64. I deleted cortex-a57-fma-steering.h as I don't think 
it adds any value.
The prototype for the make_pass* function works just as well in 
aarch64-protos.h I think.

Bootstrapped and tested on aarch64-none-linux-gnu.
Manually checked that the pass still runs when tuning for Cortex-A57 and 
doesn't run otherwise.

Ok for trunk?

Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00615.html

2016-10-13  Kyrylo Tkachov  

* config/aarch64/aarch64.c: Delete inclusion of
cortex-a57-fma-steering.h.
(aarch64_override_options): Delete call
to aarch64_register_fma_steering.
* config/aarch64/aarch64-protos.h (make_pass_fma_steering): Declare.
* config/aarch64/cortex-a57-fma-steering.h: Delete.
* config/aarch64/aarch64-passes.def: New file.
* config/aarch64/cortex-a57-fma-steering.c
(aarch64_register_fma_steering): Delete definition.
(make_pass_fma_steering): Remove static qualifier.
* config/aarch64/t-aarch64 (PASSES_EXTRA): New directive.
(cortex-a57-fma-steering.o): Remove dependency on
cortex-a57-fma-steering.h.
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 4c551ef143d3b32e94bd58989c85ebd3352cdd9b..b6ca3dfacb0dc88e5d688905d9d013263d4e8d7f 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -464,4 +464,6 @@ enum aarch64_parse_opt_result aarch64_parse_extension (const char *,
 std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
 			unsigned long);
 
+rtl_opt_pass *make_pass_fma_steering (gcc::context *ctxt);
+
 #endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ef8b8a24388d2f8e21271e0285b8d9d48078e759..e7556632901177c04f9884be4f3ee40e5f677917 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -64,7 +64,6 @@
 #include "rtl-iter.h"
 #include "tm-constrs.h"
 #include "sched-int.h"
-#include "cortex-a57-fma-steering.h"
 #include "target-globals.h"
 #include "common/common-target.h"
 
@@ -8561,9 +8560,6 @@ aarch64_override_options (void)
  while processing functions with potential target attributes.  */
   target_option_default_node = target_option_current_node
   = build_target_option_node (&global_options);
-
-  aarch64_register_fma_steering ();
-
 }
 
 /* Implement targetm.override_options_after_change.  */
diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.h b/gcc/config/aarch64/aarch64-passes.def
similarity index 78%
rename from gcc/config/aarch64/cortex-a57-fma-steering.h
rename to gcc/config/aarch64/aarch64-passes.def
index 65bf5acc132d2db645d1b00ef031dc33a195bb78..7fe80391a3fb0dc79715b9fb23fd4c08a9d26d74 100644
--- a/gcc/config/aarch64/cortex-a57-fma-steering.h
+++ b/gcc/config/aarch64/aarch64-passes.def
@@ -1,6 +1,5 @@
-/* This file contains declarations for the FMA steering optimization
-   pass for Cortex-A57.
-   Copyright (C) 2015-2016 Free Software Foundation, Inc.
+/* AArch64-specific passes declarations.
+   Copyright (C) 2016 Free Software Foundation, Inc.
Contributed by ARM Ltd.
 
This file is part of GCC.
@@ -19,4 +18,4 @@
along with GCC; see the file COPYING3.  If not see
.  */
 
-void aarch64_register_fma_steering (void);
+INSERT_PASS_AFTER (pass_regrename, 1, pass_fma_steering);
diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
index 1bf804b4873c6b32e0eb3d640a74c2e52843e796..b5f329f75a6ccfcdf5a1d1dda6758bdd87ba 100644
--- a/gcc/config/aarch64/cortex-a57-fma-steering.c
+++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
@@ -35,7 +35,6 @@
 #include "context.h"
 #include "tree-pass.h"
 #include "regrename.h"
-#include "cortex-a57-fma-steering.h"
 #include "aarch64-protos.h"
 
 /* For better performance, the destination of FMADD/FMSUB instructions should
@@ -1068,21 +1067,8 @@ public:
 
 /* Create a new fma steering pass instance.  */
 
-static rtl_opt_pass *
+rtl_opt_pass *
 make_pass_fma_steering (gcc::context *ctxt)
 {
   return new pass_fma_steering (ctxt);
 }
-
-/* Register the FMA steering pass to the pass manager.  */
-
-void
-aarch64_register_fma_steering ()
-{
-  opt_pass *pass_fma_steering = make_pass_fma_steering (g);
-
-  struct register_pass_info fma_steering_info
-= { pass_fma_steering, "rnreg", 1, PASS_POS_INSERT_AFTER };
-
-  register_pass (&fma_steering_info);
-}
diff --git a/gcc/config/aarch64/t-aarch64 b/gcc/config/aarch64/t-aarch64
index 778e15c9652d298472926b5bb29b4