On 14/09/15 11:09, Jakub Jelinek wrote:
On Fri, Sep 11, 2015 at 03:28:01PM +0200, Tom de Vries wrote:
On 11/09/15 12:57, Jakub Jelinek wrote:
On Fri, Sep 11, 2015 at 12:55:00PM +0200, Tom de Vries wrote:
Hi,

this patch adds a param parloops-schedule=<0-4>, which sets the omp schedule
for loops paralellized by parloops.

The <0-4> maps onto <static|dynamic|guided|auto|runtime>.

Bootstrapped and reg-tested on x86_64.

OK for trunk?
I don't really like it, the mapping of the integers to the enum values
is non-obvious and hard to remember.
Perhaps add support for enumeration params if you want this instead?


This patch adds handling of a DEFPARAMENUM macro, which is similar to the
DEFPARAM macro, but allows the values to be named.

So the definition of param parloop-schedule becomes:
...
DEFPARAMENUM PARAM_PARLOOPS_SCHEDULE,
              "parloops-schedule",
              "Schedule type of omp schedule for loops parallelized by "
              "parloops (static, dynamic, guided, auto, runtime)",
              0, 0, 4, "static", "dynamic", "guided", "auto", "runtime")
...
[ I'll repost the original patch containing this update. ]

OK for trunk if x86_64 bootstrap and reg-test succeeds?

That still allows numeric arguments for the param, which is IMHO
undesirable.  If it is enum kind, only the enum values should be accepted.

Fixed.

Also, it would be nice if params.h in that case would define an enum with
the values like
PARAM_PARLOOPS_SCHEDULE_KIND_{static,dynamic,guided,auto,runtime}, so use
values not wrapped in ""s and only in a macro or generator make both
enums and string array out of that.

Done, there's now a file params-enum.h containing these enums.

There is also the question if we can use __VA_ARGS__, isn't that C99 or
C++11 and later feature?  I see gengtype.h and ipa-icf-gimple.h use
that too, so maybe yes, but am not sure.


I've removed the use of variadic macros, meaning we now use DEFPARAMENUM5 instead of DEFPARAMENUM.

Also, I've remove the min/max arguments in DEFPARAMENUM5.

And I've ensured that the default is now specified as a string rather than an integer.

So the new definition of PARAM_PARLOOPS_SCHEDULE looks like:

DEFPARAMENUM5 (PARAM_PARLOOPS_SCHEDULE,
              "parloops-schedule",
              "Schedule type of omp schedule for loops parallelized by "
              "parloops (static, dynamic, guided, auto, runtime)",
              static,
              static, dynamic, guided, auto, runtime)

[ Again, I'll repost the original patch containing this update. ]

This patch adds support for DEFPARAMENUM5.

OK for trunk, if bootstrap and reg-test on x86_64 succeeds?

Thanks,
- Tom

Support DEFPARAMENUM in params.def

2015-09-11  Tom de Vries  <t...@codesourcery.com>

	* Makefile.in (PARAMS_H, PLUGIN_HEADERS): Add params-enum.h.
	* params-enum.h: New file.
	* opts.c (handle_param): Handle case that param arg is a string.
	* params-list.h: Handle DEFPARAMENUM5 in params.def.
	* params.c (find_param): New function, factored out of ...
	(set_param_value): ... here.
	(param_string_value_p): New function.
	* params.h (struct param_info): Add value_names field.
	(find_param, param_string_value_p): Declare.
---
 gcc/Makefile.in   |  6 ++--
 gcc/opts.c        | 19 +++++++----
 gcc/params-enum.h | 39 ++++++++++++++++++++++
 gcc/params-list.h |  3 ++
 gcc/params.c      | 97 ++++++++++++++++++++++++++++++++++++++++++-------------
 gcc/params.h      |  6 ++++
 6 files changed, 138 insertions(+), 32 deletions(-)
 create mode 100644 gcc/params-enum.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b495bd2..b825785 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -890,7 +890,7 @@ RTL_BASE_H = coretypes.h rtl.h rtl.def $(MACHMODE_H) reg-notes.def \
 FIXED_VALUE_H = fixed-value.h $(MACHMODE_H) double-int.h
 RTL_H = $(RTL_BASE_H) $(FLAGS_H) genrtl.h
 READ_MD_H = $(OBSTACK_H) $(HASHTAB_H) read-md.h
-PARAMS_H = params.h params.def
+PARAMS_H = params.h params-enum.h params.def
 BUILTINS_DEF = builtins.def sync-builtins.def omp-builtins.def \
 	gtm-builtins.def sanitizer.def cilkplus.def cilk-builtins.def
 INTERNAL_FN_DEF = internal-fn.def
@@ -3254,8 +3254,8 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
   tree-iterator.h $(PLUGIN_H) $(TREE_SSA_H) langhooks.h incpath.h debug.h \
   $(EXCEPT_H) tree-ssa-sccvn.h real.h output.h $(IPA_UTILS_H) \
   $(C_PRAGMA_H)  $(CPPLIB_H)  $(FUNCTION_H) \
-  cppdefault.h flags.h $(MD5_H) params.def params.h prefix.h tree-inline.h \
-  $(GIMPLE_PRETTY_PRINT_H) realmpfr.h \
+  cppdefault.h flags.h $(MD5_H) params.def params.h params-enum.h \
+  prefix.h tree-inline.h $(GIMPLE_PRETTY_PRINT_H) realmpfr.h \
   $(IPA_PROP_H) $(TARGET_H) $(RTL_H) $(TM_P_H) $(CFGLOOP_H) $(EMIT_RTL_H) \
   version.h stringpool.h gimplify.h gimple-iterator.h gimple-ssa.h \
   fold-const.h tree-cfg.h tree-into-ssa.h tree-ssanames.h print-tree.h \
diff --git a/gcc/opts.c b/gcc/opts.c
index f1a9acd..3349aaf 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2116,14 +2116,21 @@ handle_param (struct gcc_options *opts, struct gcc_options *opts_set,
 	      arg);
   else
     {
-      value = integral_argument (equal + 1);
-      if (value == -1)
-	error_at (loc, "invalid --param value %qs", equal + 1);
+      *equal = '\0';
+
+      enum compiler_param index;
+      if (!find_param (arg, &index))
+	error_at (loc, "invalid --param name %qs", arg);
       else
 	{
-	  *equal = '\0';
-	  set_param_value (arg, value,
-			   opts->x_param_values, opts_set->x_param_values);
+	  if (!param_string_value_p (index, equal + 1, &value))
+	    value = integral_argument (equal + 1);
+
+	  if (value == -1)
+	    error_at (loc, "invalid --param value %qs", equal + 1);
+	  else
+	    set_param_value (arg, value,
+			     opts->x_param_values, opts_set->x_param_values);
 	}
     }
 
diff --git a/gcc/params-enum.h b/gcc/params-enum.h
new file mode 100644
index 0000000..a95e16c
--- /dev/null
+++ b/gcc/params-enum.h
@@ -0,0 +1,39 @@
+/* params-enums.h - Run-time parameter enums.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#define DEFPARAM(ENUM, OPTION, HELP, DEFAULT, MIN, MAX)
+#define DEFPARAMENUMNAME(ENUM) ENUM ## _KIND
+#define DEFPARAMENUMVAL(ENUM, V) ENUM ## _KIND_ ## V
+#define DEFPARAMENUMTERM(ENUM) ENUM ## _KIND_ ## LAST
+#define DEFPARAMENUM5(ENUM, OPTION, HELP, DEFAULT, V0, V1, V2, V3, V4)	\
+  enum DEFPARAMENUMNAME (ENUM)					\
+  {								\
+    DEFPARAMENUMVAL (ENUM, V0),					\
+    DEFPARAMENUMVAL (ENUM, V1),					\
+    DEFPARAMENUMVAL (ENUM, V2),					\
+    DEFPARAMENUMVAL (ENUM, V3),					\
+    DEFPARAMENUMVAL (ENUM, V4),					\
+    DEFPARAMENUMTERM (ENUM)					\
+  };
+#include "params.def"
+#undef DEFPARAMENUM5
+#undef DEFPARAMENUMTERM
+#undef DEFPARAMENUMVAL
+#undef DEFPARAMENUMNAME
+#undef DEFPARAM
diff --git a/gcc/params-list.h b/gcc/params-list.h
index ee33ef5..fa76856 100644
--- a/gcc/params-list.h
+++ b/gcc/params-list.h
@@ -19,5 +19,8 @@ along with GCC; see the file COPYING3.  If not see
 
 #define DEFPARAM(enumerator, option, nocmsgid, default, min, max) \
   enumerator,
+#define DEFPARAMENUM5(enumerator, option, nocmsgid, default, \
+		      v0, v1, v2, v3, v4) enumerator,
 #include "params.def"
 #undef DEFPARAM
+#undef DEFPARAMENUM5
diff --git a/gcc/params.c b/gcc/params.c
index b0bc80b..d58d81e 100644
--- a/gcc/params.c
+++ b/gcc/params.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "common/common-target.h"
 #include "params.h"
+#include "params-enum.h"
 #include "diagnostic-core.h"
 
 /* An array containing the compiler parameters and their current
@@ -37,12 +38,23 @@ static size_t num_compiler_params;
    default values determined.  */
 static bool params_finished;
 
+#define DEFPARAM(ENUM, OPTION, HELP, DEFAULT, MIN, MAX)
+#define DEFPARAMENUM5(ENUM, OPTION, HELP, DEFAULT, V0, V1, V2, V3, V4)	\
+  static const char *values_ ## ENUM [] = { #V0, #V1, #V2, #V3, #V4, NULL };
+#include "params.def"
+#undef DEFPARAMENUM5
+#undef DEFPARAM
+
 static const param_info lang_independent_params[] = {
 #define DEFPARAM(ENUM, OPTION, HELP, DEFAULT, MIN, MAX) \
-  { OPTION, DEFAULT, MIN, MAX, HELP },
+  { OPTION, DEFAULT, MIN, MAX, HELP, NULL },
+#define DEFPARAMENUM5(ENUM, OPTION, HELP, DEFAULT,	     \
+		      V0, V1, V2, V3, V4)		     \
+  { OPTION, (int)ENUM ## _KIND_ ## DEFAULT, 0, 4, HELP, values_ ## ENUM },
 #include "params.def"
 #undef DEFPARAM
-  { NULL, 0, 0, 0, NULL }
+#undef DEFPARAMENUM5
+  { NULL, 0, 0, 0, NULL, NULL }
 };
 
 /* Add the N PARAMS to the current list of compiler parameters.  */
@@ -114,6 +126,45 @@ set_param_value_internal (compiler_param num, int value,
     params_set[i] = true;
 }
 
+/* Return true if it can find the matching entry for NAME in the parameter
+   table, and assign the entry index to INDEX.  Return false otherwise.  */
+
+bool
+find_param (const char *name, enum compiler_param *index)
+{
+  for (size_t i = 0; i < num_compiler_params; ++i)
+    if (strcmp (compiler_params[i].option, name) == 0)
+      {
+	*index = (enum compiler_param) i;
+	return true;
+      }
+
+  return false;
+}
+
+/* Return true if param with entry index INDEX should be defined using strings.
+   If so, return the value corresponding to VALUE_NAME in *VALUE_P.  */
+
+bool
+param_string_value_p (enum compiler_param index, const char *value_name,
+		      int *value_p)
+{
+  param_info *entry = &compiler_params[(int) index];
+  if (entry->value_names == NULL)
+    return false;
+
+  *value_p = -1;
+
+  for (int i = 0; entry->value_names[i] != NULL; ++i)
+    if (strcmp (entry->value_names[i], value_name) == 0)
+      {
+	*value_p = i;
+	return true;
+      }
+
+  return true;
+}
+
 /* Set the VALUE associated with the parameter given by NAME in PARAMS
    and PARAMS_SET.  */
 
@@ -126,27 +177,27 @@ set_param_value (const char *name, int value,
   /* Make sure nobody tries to set a parameter to an invalid value.  */
   gcc_assert (value != INVALID_PARAM_VAL);
 
-  /* Scan the parameter table to find a matching entry.  */
-  for (i = 0; i < num_compiler_params; ++i)
-    if (strcmp (compiler_params[i].option, name) == 0)
-      {
-	if (value < compiler_params[i].min_value)
-	  error ("minimum value of parameter %qs is %u",
-		 compiler_params[i].option,
-		 compiler_params[i].min_value);
-	else if (compiler_params[i].max_value > compiler_params[i].min_value
-		 && value > compiler_params[i].max_value)
-	  error ("maximum value of parameter %qs is %u",
-		 compiler_params[i].option,
-		 compiler_params[i].max_value);
-	else
-	  set_param_value_internal ((compiler_param) i, value,
-				    params, params_set, true);
-	return;
-      }
-
-  /* If we didn't find this parameter, issue an error message.  */
-  error ("invalid parameter %qs", name);
+  enum compiler_param index;
+  if (!find_param (name, &index))
+    {
+      /* If we didn't find this parameter, issue an error message.  */
+      error ("invalid parameter %qs", name);
+      return;
+    }
+  i = (size_t)index;
+
+  if (value < compiler_params[i].min_value)
+    error ("minimum value of parameter %qs is %u",
+	   compiler_params[i].option,
+	   compiler_params[i].min_value);
+  else if (compiler_params[i].max_value > compiler_params[i].min_value
+	   && value > compiler_params[i].max_value)
+    error ("maximum value of parameter %qs is %u",
+	   compiler_params[i].option,
+	   compiler_params[i].max_value);
+  else
+    set_param_value_internal ((compiler_param) i, value,
+			      params, params_set, true);
 }
 
 /* Set the value of the parameter given by NUM to VALUE in PARAMS and
diff --git a/gcc/params.h b/gcc/params.h
index 9f7618a..1090d00 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -55,6 +55,9 @@ struct param_info
 
   /* A short description of the option.  */
   const char *const help;
+
+  /* The optional names corresponding to the values.  */
+  const char **value_names;
 };
 
 /* An array containing the compiler parameters and their current
@@ -85,6 +88,9 @@ enum compiler_param
   LAST_PARAM
 };
 
+extern bool find_param (const char *, enum compiler_param *);
+extern bool param_string_value_p (enum compiler_param, const char *, int *);
+
 /* The value of the parameter given by ENUM.  Not an lvalue.  */
 #define PARAM_VALUE(ENUM) \
   ((int) global_options.x_param_values[(int) ENUM])
-- 
1.9.1

Reply via email to