diff mbox

[RFC] Patch candidate for other/39851

Message ID 2eb93d20-71eb-0a4f-e323-18465ca485f4@suse.cz
State New
Headers show

Commit Message

Martin Liška Aug. 8, 2017, 12:07 p.m. UTC 
Hi.

As mentioned in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39851#c0 we need
to call targetm.target_option.override () in order to properly report which
ISA options are enabled for a -march/-mtune. Currently, opts.c uses just
#include "common/common-target.h" we are unable to call the function direct.
One solution might be to put the hook to gcc/common/common-target.def, but
that would require huge refactoring of i386.c and i386-common.c files.

Thus I came with a small hook that lives in cl_option_handler_func.
With that I see proper results for test-case mentioned in the PR.

Thoughts?
Martin

Comments

Joseph Myers Aug. 28, 2017, 3:59 p.m. UTC | #1 
On Tue, 8 Aug 2017, Martin Liška wrote:

> Hi.
> 
> As mentioned in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39851#c0 we need
> to call targetm.target_option.override () in order to properly report which
> ISA options are enabled for a -march/-mtune. Currently, opts.c uses just
> #include "common/common-target.h" we are unable to call the function direct.
> One solution might be to put the hook to gcc/common/common-target.def, but
> that would require huge refactoring of i386.c and i386-common.c files.
> 
> Thus I came with a small hook that lives in cl_option_handler_func.
> With that I see proper results for test-case mentioned in the PR.

This patch is OK.  As you note, making the targetm.target_option.override 
hook into a common option would involve much refactoring, because many of 
those hooks mix things acting purely on the options structure (which could 
readily become common) and things relating to other back-end state (which 
would need to stay in a hook that's only used in the compiler proper, not 
the driver).
Martin Liška Aug. 29, 2017, 8:36 a.m. UTC | #2 
On 08/28/2017 05:59 PM, Joseph Myers wrote:
> On Tue, 8 Aug 2017, Martin Liška wrote:
> 
>> Hi.
>>
>> As mentioned in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39851#c0 we need
>> to call targetm.target_option.override () in order to properly report which
>> ISA options are enabled for a -march/-mtune. Currently, opts.c uses just
>> #include "common/common-target.h" we are unable to call the function direct.
>> One solution might be to put the hook to gcc/common/common-target.def, but
>> that would require huge refactoring of i386.c and i386-common.c files.
>>
>> Thus I came with a small hook that lives in cl_option_handler_func.
>> With that I see proper results for test-case mentioned in the PR.
> 
> This patch is OK.  As you note, making the targetm.target_option.override 
> hook into a common option would involve much refactoring, because many of 
> those hooks mix things acting purely on the options structure (which could 
> readily become common) and things relating to other back-end state (which 
> would need to stay in a hook that's only used in the compiler proper, not 
> the driver).
> 

Thanks for the review. I've just installed the patch and a small fallout for Ada.

Martin
From 891cac45682dddbecb343c3a7660ea0ab264373e Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 29 Aug 2017 10:34:32 +0200
Subject: [PATCH] Fix --help=target (Ada) (PR other/39851)

gcc/ada/ChangeLog:

2017-08-29  Martin Liska  <mliska@suse.cz>

	PR other/39851
	* gcc-interface/trans.c (Pragma_to_gnu): Set argument to NULL.
---
 gcc/ada/gcc-interface/trans.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 67044b7b574..c0c6fb30915 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -1486,7 +1486,7 @@ Pragma_to_gnu (Node_Id gnat_node)
 	else
 	  option_index = 0;
 
-	set_default_handlers (&handlers);
+	set_default_handlers (&handlers, NULL);
 	control_warning_option (option_index, (int) kind, arg, imply, location,
 				lang_mask, &handlers, &global_options,
 				&global_options_set, global_dc);
diff mbox

Patch

From 1bd4916c0c5594162d6a34c9eb4c7202931260df Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 8 Aug 2017 13:09:12 +0200
Subject: [PATCH] Patch candidate.

---
 gcc/c-family/c-common.c |  2 +-
 gcc/c-family/c-pragma.c |  2 +-
 gcc/gcc.c               |  3 ++-
 gcc/opts-common.c       |  3 ++-
 gcc/opts-global.c       | 12 ++++++++----
 gcc/opts.c              | 14 ++++++++++----
 gcc/opts.h              | 18 +++++++++++++-----
 gcc/toplev.c            |  3 ++-
 8 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index feb0904bcbf..a2469471fe5 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5496,7 +5496,7 @@  parse_optimize_options (tree args, bool attr_p)
   /* And apply them.  */
   decode_options (&global_options, &global_options_set,
 		  decoded_options, decoded_options_count,
-		  input_location, global_dc);
+		  input_location, global_dc, NULL);
 
   targetm.override_options_after_change();
 
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index 48b02b88bb5..3b49aefc6ff 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -815,7 +815,7 @@  handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy))
     }
 
   struct cl_option_handlers handlers;
-  set_default_handlers (&handlers);
+  set_default_handlers (&handlers, NULL);
   const char *arg = NULL;
   if (cl_options[option_index].flags & CL_JOINED)
     arg = option_string + 1 + cl_options[option_index].opt_len;
diff --git a/gcc/gcc.c b/gcc/gcc.c
index d8c5260e36b..fde68809d68 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3745,7 +3745,8 @@  driver_handle_option (struct gcc_options *opts,
 		      unsigned int lang_mask ATTRIBUTE_UNUSED, int kind,
 		      location_t loc,
 		      const struct cl_option_handlers *handlers ATTRIBUTE_UNUSED,
-		      diagnostic_context *dc)
+		      diagnostic_context *dc,
+		      void (*) (void))
 {
   size_t opt_index = decoded->opt_index;
   const char *arg = decoded->arg;
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 0cab42a021c..d7568145768 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -993,7 +993,8 @@  handle_option (struct gcc_options *opts,
       {
 	if (!handlers->handlers[i].handler (opts, opts_set, decoded,
 					    lang_mask, kind, loc,
-					    handlers, dc))
+					    handlers, dc,
+					    handlers->target_option_override_hook))
 	  return false;
       }
   
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index fc55512e554..343dbd3ac2c 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -169,7 +169,8 @@  lang_handle_option (struct gcc_options *opts,
 		    unsigned int lang_mask ATTRIBUTE_UNUSED, int kind,
 		    location_t loc,
 		    const struct cl_option_handlers *handlers,
-		    diagnostic_context *dc)
+		    diagnostic_context *dc,
+		    void (*) (void))
 {
   gcc_assert (opts == &global_options);
   gcc_assert (opts_set == &global_options_set);
@@ -269,10 +270,12 @@  decode_cmdline_options_to_array_default_mask (unsigned int argc,
 /* Set *HANDLERS to the default set of option handlers for use in the
    compilers proper (not the driver).  */
 void
-set_default_handlers (struct cl_option_handlers *handlers)
+set_default_handlers (struct cl_option_handlers *handlers,
+		      void (*target_option_override_hook) (void))
 {
   handlers->unknown_option_callback = unknown_option_callback;
   handlers->wrong_lang_callback = complain_wrong_lang;
+  handlers->target_option_override_hook = target_option_override_hook;
   handlers->num_handlers = 3;
   handlers->handlers[0].handler = lang_handle_option;
   handlers->handlers[0].mask = initial_lang_mask;
@@ -290,7 +293,8 @@  void
 decode_options (struct gcc_options *opts, struct gcc_options *opts_set,
 		struct cl_decoded_option *decoded_options,
 		unsigned int decoded_options_count,
-		location_t loc, diagnostic_context *dc)
+		location_t loc, diagnostic_context *dc,
+		void (*target_option_override_hook) (void))
 {
   struct cl_option_handlers handlers;
 
@@ -298,7 +302,7 @@  decode_options (struct gcc_options *opts, struct gcc_options *opts_set,
 
   lang_mask = initial_lang_mask;
 
-  set_default_handlers (&handlers);
+  set_default_handlers (&handlers, target_option_override_hook);
 
   default_options_optimization (opts, opts_set,
 				decoded_options, decoded_options_count,
diff --git a/gcc/opts.c b/gcc/opts.c
index 989cc6b6dec..5bc17be14b8 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -217,7 +217,7 @@  target_handle_option (struct gcc_options *opts,
 		      unsigned int lang_mask ATTRIBUTE_UNUSED, int kind,
 		      location_t loc,
 		      const struct cl_option_handlers *handlers ATTRIBUTE_UNUSED,
-		      diagnostic_context *dc)
+		      diagnostic_context *dc, void (*) (void))
 {
   gcc_assert (dc == global_dc);
   gcc_assert (kind == DK_UNSPECIFIED);
@@ -1715,7 +1715,8 @@  common_handle_option (struct gcc_options *opts,
 		      unsigned int lang_mask, int kind ATTRIBUTE_UNUSED,
 		      location_t loc,
 		      const struct cl_option_handlers *handlers,
-		      diagnostic_context *dc)
+		      diagnostic_context *dc,
+		      void (*target_option_override_hook) (void))
 {
   size_t scode = decoded->opt_index;
   const char *arg = decoded->arg;
@@ -1742,6 +1743,7 @@  common_handle_option (struct gcc_options *opts,
 	undoc_mask = ((opts->x_verbose_flag | opts->x_extra_warnings)
 		      ? 0
 		      : CL_UNDOCUMENTED);
+	target_option_override_hook ();
 	/* First display any single language specific options.  */
 	for (i = 0; i < cl_lang_count; i++)
 	  print_specific_help
@@ -1761,6 +1763,7 @@  common_handle_option (struct gcc_options *opts,
       if (lang_mask == CL_DRIVER)
 	break;
 
+      target_option_override_hook ();
       print_specific_help (CL_TARGET, CL_UNDOCUMENTED, 0, opts, lang_mask);
       opts->x_exit_after_options = true;
       break;
@@ -1887,8 +1890,11 @@  common_handle_option (struct gcc_options *opts,
 	  }
 
 	if (include_flags)
-	  print_specific_help (include_flags, exclude_flags, 0, opts,
-			       lang_mask);
+	  {
+	    target_option_override_hook ();
+	    print_specific_help (include_flags, exclude_flags, 0, opts,
+				 lang_mask);
+	  }
 	opts->x_exit_after_options = true;
 	break;
       }
diff --git a/gcc/opts.h b/gcc/opts.h
index 5599711cc76..2774e2c8b40 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -272,7 +272,8 @@  struct cl_option_handler_func
 		   const struct cl_decoded_option *decoded,
 		   unsigned int lang_mask, int kind, location_t loc,
 		   const struct cl_option_handlers *handlers,
-		   diagnostic_context *dc);
+		   diagnostic_context *dc,
+		   void (*target_option_override_hook) (void));
 
   /* The mask that must have some bit in common with the flags for the
      option for this particular handler to be used.  */
@@ -294,6 +295,9 @@  struct cl_option_handlers
   void (*wrong_lang_callback) (const struct cl_decoded_option *decoded,
 			       unsigned int lang_mask);
 
+  /* Target option override hook.  */
+  void (*target_option_override_hook) (void);
+
   /* The number of individual handlers.  */
   size_t num_handlers;
 
@@ -338,13 +342,15 @@  extern void decode_cmdline_options_to_array_default_mask (unsigned int argc,
 							  const char **argv, 
 							  struct cl_decoded_option **decoded_options,
 							  unsigned int *decoded_options_count);
-extern void set_default_handlers (struct cl_option_handlers *handlers);
+extern void set_default_handlers (struct cl_option_handlers *handlers,
+				  void (*target_option_override_hook) (void));
 extern void decode_options (struct gcc_options *opts,
 			    struct gcc_options *opts_set,
 			    struct cl_decoded_option *decoded_options,
 			    unsigned int decoded_options_count,
 			    location_t loc,
-			    diagnostic_context *dc);
+			    diagnostic_context *dc,
+			    void (*target_option_override_hook) (void));
 extern int option_enabled (int opt_idx, void *opts);
 extern bool get_option_state (struct gcc_options *, int,
 			      struct cl_option_state *);
@@ -391,14 +397,16 @@  extern bool common_handle_option (struct gcc_options *opts,
 				  unsigned int lang_mask, int kind,
 				  location_t loc,
 				  const struct cl_option_handlers *handlers,
-				  diagnostic_context *dc);
+				  diagnostic_context *dc,
+				  void (*target_option_override_hook) (void));
 extern bool target_handle_option (struct gcc_options *opts,
 				  struct gcc_options *opts_set,
 				  const struct cl_decoded_option *decoded,
 				  unsigned int lang_mask, int kind,
 				  location_t loc,
 				  const struct cl_option_handlers *handlers,
-				  diagnostic_context *dc);
+				  diagnostic_context *dc,
+				  void (*target_option_override_hook) (void));
 extern void finish_options (struct gcc_options *opts,
 			    struct gcc_options *opts_set,
 			    location_t loc);
diff --git a/gcc/toplev.c b/gcc/toplev.c
index d23714c4773..7d2b8fffa0b 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -2149,7 +2149,8 @@  toplev::main (int argc, char **argv)
      enough to default flags appropriately.  */
   decode_options (&global_options, &global_options_set,
 		  save_decoded_options, save_decoded_options_count,
-		  UNKNOWN_LOCATION, global_dc);
+		  UNKNOWN_LOCATION, global_dc,
+		  targetm.target_option.override);
 
   handle_common_deferred_options ();
 
-- 
2.13.3