From patchwork Mon Sep 14 14:28:33 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 517448 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 454EB140779 for ; Tue, 15 Sep 2015 00:29:10 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=YzI59Q1a; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=wBcXXF6zxmtL/CbnB j07YubYMHdZjnQdv/nrEahRWBlB0+sPgrnz3XCyB6eTpeeDhY+8H+qwAz8JigdIt 6cfx1h5DlQhR7s13UbiXNVm0MPViovEnW9huMpNuuL2PMnSX0KW9NSKHaOUDWtPE WFSTq1k4G78usb6Qp5+kR/aYLI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=Z8kBeA0p7Mt8etzyHCWxBtv b5Cs=; b=YzI59Q1alTsb5yWAf49n0i+tVnuSRw8WCTrjsGDB0NOTGgYtbkEGfRU I6SzCiJ/XpYTdceGqk3qAy9uUqCJT2XLWxwfDG+fs0CTEReVjKcDxABa79h2l84T bzJBle7832HBWUH4+z2E3IIlopS6lYo1IIT0qmPxOdY8YEvPbgsk= Received: (qmail 19936 invoked by alias); 14 Sep 2015 14:28:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 19844 invoked by uid 89); 14 Sep 2015 14:28:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: fencepost.gnu.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (208.118.235.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 14 Sep 2015 14:28:56 +0000 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44571) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1ZbUkX-000660-Ho for gcc-patches@gnu.org; Mon, 14 Sep 2015 10:28:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbUkT-0002X0-L8 for gcc-patches@gnu.org; Mon, 14 Sep 2015 10:28:53 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:59265) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbUkT-0002Wb-CB for gcc-patches@gnu.org; Mon, 14 Sep 2015 10:28:49 -0400 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-03.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZbUkN-0007C3-Go from Tom_deVries@mentor.com ; Mon, 14 Sep 2015 07:28:44 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-03.mgc.mentorg.com (137.202.0.108) with Microsoft SMTP Server id 14.3.224.2; Mon, 14 Sep 2015 15:28:41 +0100 Subject: Re: [PATCH][PR67476] Add param parloops-schedule To: Jakub Jelinek References: <55F2B304.7060604@mentor.com> <20150911105749.GC1847@tucnak.redhat.com> <55F2D6E1.7030504@mentor.com> <20150914090902.GQ1847@tucnak.redhat.com> CC: Richard Biener , "gcc-patches@gnu.org" From: Tom de Vries Message-ID: <55F6D991.8080803@mentor.com> Date: Mon, 14 Sep 2015 16:28:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150914090902.GQ1847@tucnak.redhat.com> X-detected-operating-system: by eggs.gnu.org: Windows NT kernel [generic] [fuzzy] X-Received-From: 192.94.38.131 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 . >>>>> >>>>> 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 * 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 +. */ + +#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