diff mbox

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

Message ID 57FFC0E8.1090503@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov Oct. 13, 2016, 5:14 p.m. UTC
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  <kyrylo.tkachov@arm.com>

     * 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.

Comments

James Greenhalgh Oct. 17, 2016, 4:30 p.m. UTC | #1
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  <kyrylo.tkachov@arm.com>
> 
>     * 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
>     <http://www.gnu.org/licenses/>.  */
>  
> -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
diff mbox

Patch

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
    <http://www.gnu.org/licenses/>.  */
 
-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..b5f329f75a6ccfcdfcccc5a1d1dda6758bdd87ba 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 778e15c9652d298472926b5bb29b4b02166bc5f2..04eb63666e8db38367235fc82bb55f826eb8af9a 100644
--- a/gcc/config/aarch64/t-aarch64
+++ b/gcc/config/aarch64/t-aarch64
@@ -56,12 +56,13 @@  aarch64-c.o: $(srcdir)/config/aarch64/aarch64-c.c $(CONFIG_H) $(SYSTEM_H) \
 	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
 		$(srcdir)/config/aarch64/aarch64-c.c
 
+PASSES_EXTRA += $(srcdir)/config/aarch64/aarch64-passes.def
+
 cortex-a57-fma-steering.o: $(srcdir)/config/aarch64/cortex-a57-fma-steering.c \
     $(CONFIG_H) $(SYSTEM_H) $(TM_H) $(REGS_H) insn-config.h $(RTL_BASE_H) \
     dominance.h cfg.h cfganal.h $(BASIC_BLOCK_H) $(INSN_ATTR_H) $(RECOG_H) \
     output.h hash-map.h $(DF_H) $(OBSTACK_H) $(TARGET_H) $(RTL_H) \
     $(CONTEXT_H) $(TREE_PASS_H) regrename.h \
-    $(srcdir)/config/aarch64/cortex-a57-fma-steering.h \
     $(srcdir)/config/aarch64/aarch64-protos.h
 	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
 		$(srcdir)/config/aarch64/cortex-a57-fma-steering.c