diff mbox

[PR67476] Add param parloops-schedule

Message ID 55F6D991.8080803@mentor.com
State New
Headers show

Commit Message

Tom de Vries Sept. 14, 2015, 2:28 p.m. UTC
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
diff mbox

Patch

Support DEFPARAMENUM in params.def

2015-09-11  Tom de Vries  <tom@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