This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Show valid options for -march and -mtune in --help=target for arm32 (PR driver/83193).


On 07/19/2018 12:31 PM, Richard Earnshaw (lists) wrote:
> On 19/07/18 11:22, Martin Liška wrote:
>> On 07/19/2018 12:01 PM, Richard Earnshaw (lists) wrote:
>>> On 19/07/18 10:56, Martin Liška wrote:
>>>> On 07/19/2018 11:28 AM, Richard Earnshaw (lists) wrote:
>>>>> On 19/07/18 08:30, Martin Liška wrote:
>>>>>> This is correct version of the patch. Anyway, I'm thinking about the ForceHelp
>>>>>> attribute. I may do it in a bit different version. Let me come up with one another
>>>>>> version of the patch.
>>>>>>
>>>>>> Martin
>>>>>>
>>>>>
>>>>> I don't understand how this is supposed to work.  -mcpu, -march and
>>>>> -mtune all take strings now and have to be parsed to identify various
>>>>> sub-components of the parameter.  So why do you talk about these being
>>>>> enum types?
>>>>
>>>> Yes, they are string types. But for purpose of --help output, it's nice
>>>> to present to a user a list of possible values. That's the enum type.
>>>>
>>>> Please take a look at attached patch.
>>>>
>>>
>>> But that isn't the list of possible values.  Please see the manual.  A
>>> valid CPU name can look something like
>>>
>>> 	cortex-a53+crypto
>>>
>>> and architectures names can be even more complex.
>>>
>>> You can't get this from that list of enum values.
>>
>> I'm fully aware of the limitation, it's questionable whether you want to get:
>>
>> @@ -56,6 +56,9 @@
>>    Known ARM ABIs (for use with the -mabi= option):
>>      aapcs aapcs-linux apcs-gnu atpcs iwmmxt
>>  
>> +  Known ARM architectures (for use with the -march= option):
>> +    armv4 armv4t armv5t armv5te armv5tej armv6 armv6-m armv6j armv6k armv6kz armv6s-m armv6t2 armv6z armv6zk armv7 armv7-a armv7-m armv7-r armv7e-m armv7ve armv8-a armv8-m.base armv8-m.main armv8-r armv8.1-a armv8.2-a armv8.3-a armv8.4-a iwmmxt iwmmxt2 native
>> +
>>    Known __fp16 formats (for use with the -mfp16-format= option):
>>      alternative ieee none
>>  
>> @@ -68,6 +71,12 @@
>>    Known floating-point ABIs (for use with the -mfloat-abi= option):
>>      hard soft softfp
>>  
>> +  Known ARM CPUs (for use with the -mcpu= and -mtune= options):
>> +    arm1020e arm1020t arm1022e arm1026ej-s arm10e arm10tdmi arm1136j-s arm1136jf-s arm1156t2-s arm1156t2f-s arm1176jz-s arm1176jzf-s arm710t arm720t arm740t arm7tdmi arm7tdmi-s arm8 arm810 arm9 arm920 arm920t arm922t arm926ej-s arm940t arm946e-s arm966e-s arm968e-s arm9e
>> +    arm9tdmi cortex-a12 cortex-a15 cortex-a15.cortex-a7 cortex-a17 cortex-a17.cortex-a7 cortex-a32 cortex-a35 cortex-a5 cortex-a53 cortex-a55 cortex-a57 cortex-a57.cortex-a53 cortex-a7 cortex-a72 cortex-a72.cortex-a53 cortex-a73 cortex-a73.cortex-a35 cortex-a73.cortex-a53
>> +    cortex-a75 cortex-a75.cortex-a55 cortex-a76 cortex-a76.cortex-a55 cortex-a8 cortex-a9 cortex-m0 cortex-m0.small-multiply cortex-m0plus cortex-m0plus.small-multiply cortex-m1 cortex-m1.small-multiply cortex-m23 cortex-m3 cortex-m33 cortex-m4 cortex-m7 cortex-r4
>> +    cortex-r4f cortex-r5 cortex-r52 cortex-r7 cortex-r8 ep9312 exynos-m1 fa526 fa606te fa626 fa626te fa726te fmp626 generic-armv7-a iwmmxt iwmmxt2 marvell-pj4 mpcore mpcorenovfp native strongarm strongarm110 strongarm1100 strongarm1110 xgene1 xscale
>> +
>>    TLS dialect to use:
>>      gnu gnu2
>>
>> I hope it's still beneficial for users.
> 
> Frankly, I find the list too long to be helpful.  I'd also prefer it if

One justification for this can be that with a very simple patch and
can have bash completion to finish a -march option value.

> we could come up with a more useful approach.  I've pondered if the
> following were possible:
> 
> In general target help, print
> 
> 	For list of supported CPUs [Architectures] use -mcpu=help [-march=help]
> 
> And then, invoking the compiler gives that list in a more user-friendly
> fashion.  Finally, at the end we could have:
> 
> For CPU [Architecture]-specific extensions use -mcpu=<cpuname>+help
> [-march=<archname>+help]
> 
> and then it would show the specific extensions for that architecture.
> 
> It's relatively straight forward to do the back-end plumbing for this,
> but the help driver would have to know how to call into the back-end or
> for the back-end to be able to report to the midend that this was a help
> invocation not a normal run.  I couldn't find a simple way of doing that
> when I tried before.

Would you be able to implement that as target_common hook? These are defined
in gcc/common/common-target.def. If so, I can then provide an API that
will use it.

Martin


> 
> R.
> 
>>
>> Martin
>>
>>>
>>> R.
>>>
>>>> Thanks,
>>>> Martin
>>>>
>>>>>
>>>>> R.
>>>>>
>>>>>>
>>>>>> 0001-Show-valid-options-for-march-and-mtune-in-help-targe-v3.patch
>>>>>>
>>>>>>
>>>>>> From 9bfc1400213911b4508e90198df7b2dd11efc85c Mon Sep 17 00:00:00 2001
>>>>>> From: marxin <mliska@suse.cz>
>>>>>> Date: Tue, 20 Feb 2018 10:39:09 +0100
>>>>>> Subject: [PATCH] Show valid options for -march and -mtune in --help=target for
>>>>>>  arm32 (PR driver/83193).
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2018-07-18  Martin Liska  <mliska@suse.cz>
>>>>>>
>>>>>>         PR driver/83193
>>>>>> 	* config/arm/arm-tables.opt: Add ForceHelp flag for
>>>>>>         processor_type and arch_name enum types.
>>>>>> 	* config/arm/parsecpu.awk: Likewise.
>>>>>> 	* doc/options.texi: Document new flag ForceHelp.
>>>>>> 	* opt-read.awk: Parse ForceHelp and set it in construction.
>>>>>> 	* optc-gen.awk: Likewise.
>>>>>> 	* opts.c (print_filtered_help): Handle force_help option.
>>>>>> 	* opts.h (struct cl_enum): New field force_help.
>>>>>> ---
>>>>>>  gcc/config/arm/arm-tables.opt | 4 ++--
>>>>>>  gcc/config/arm/parsecpu.awk   | 4 ++--
>>>>>>  gcc/doc/options.texi          | 4 ++++
>>>>>>  gcc/opt-read.awk              | 3 +++
>>>>>>  gcc/optc-gen.awk              | 3 ++-
>>>>>>  gcc/opts.c                    | 3 ++-
>>>>>>  gcc/opts.h                    | 3 +++
>>>>>>  7 files changed, 18 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/gcc/config/arm/arm-tables.opt b/gcc/config/arm/arm-tables.opt
>>>>>> index eacee746a39..c74229e27d7 100644
>>>>>> --- a/gcc/config/arm/arm-tables.opt
>>>>>> +++ b/gcc/config/arm/arm-tables.opt
>>>>>> @@ -21,7 +21,7 @@
>>>>>>  ; <http://www.gnu.org/licenses/>.
>>>>>>  
>>>>>>  Enum
>>>>>> -Name(processor_type) Type(enum processor_type)
>>>>>> +Name(processor_type) Type(enum processor_type) ForceHelp
>>>>>>  Known ARM CPUs (for use with the -mcpu= and -mtune= options):
>>>>>>  
>>>>>>  EnumValue
>>>>>> @@ -298,7 +298,7 @@ EnumValue
>>>>>>  Enum(processor_type) String(cortex-r52) Value( TARGET_CPU_cortexr52)
>>>>>>  
>>>>>>  Enum
>>>>>> -Name(arm_arch) Type(int)
>>>>>> +Name(arm_arch) Type(int) ForceHelp
>>>>>>  Known ARM architectures (for use with the -march= option):
>>>>>>  
>>>>>>  EnumValue
>>>>>> diff --git a/gcc/config/arm/parsecpu.awk b/gcc/config/arm/parsecpu.awk
>>>>>> index aabe1b0c64c..c499a5ed0ce 100644
>>>>>> --- a/gcc/config/arm/parsecpu.awk
>>>>>> +++ b/gcc/config/arm/parsecpu.awk
>>>>>> @@ -441,7 +441,7 @@ function gen_opt () {
>>>>>>      boilerplate("md")
>>>>>>  
>>>>>>      print "Enum"
>>>>>> -    print "Name(processor_type) Type(enum processor_type)"
>>>>>> +    print "Name(processor_type) Type(enum processor_type) ForceHelp"
>>>>>>      print "Known ARM CPUs (for use with the -mcpu= and -mtune= options):\n"
>>>>>>  
>>>>>>      ncpus = split (cpu_list, cpus)
>>>>>> @@ -454,7 +454,7 @@ function gen_opt () {
>>>>>>      }
>>>>>>  
>>>>>>      print "Enum"
>>>>>> -    print "Name(arm_arch) Type(int)"
>>>>>> +    print "Name(arm_arch) Type(int) ForceHelp"
>>>>>>      print "Known ARM architectures (for use with the -march= option):\n"
>>>>>>  
>>>>>>      narchs = split (arch_list, archs)
>>>>>> diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
>>>>>> index b3ca9f6fce6..af77ad78e8c 100644
>>>>>> --- a/gcc/doc/options.texi
>>>>>> +++ b/gcc/doc/options.texi
>>>>>> @@ -120,6 +120,10 @@ being described by this record.
>>>>>>  This property is required; it says what value (representable as
>>>>>>  @code{int}) should be used for the given string.
>>>>>>  
>>>>>> +@item ForceHelp
>>>>>> +This property is optional.  If present, enum values are printed
>>>>>> +in @option{--help} output.
>>>>>> +
>>>>>>  @item Canonical
>>>>>>  This property is optional.  If present, it says the present string is
>>>>>>  the canonical one among all those with the given value.  Other strings
>>>>>> diff --git a/gcc/opt-read.awk b/gcc/opt-read.awk
>>>>>> index 2072958e6ba..6d2be9e99d7 100644
>>>>>> --- a/gcc/opt-read.awk
>>>>>> +++ b/gcc/opt-read.awk
>>>>>> @@ -89,6 +89,9 @@ BEGIN {
>>>>>>  			enum_index[name] = n_enums
>>>>>>  			enum_unknown_error[name] = unknown_error
>>>>>>  			enum_help[name] = $3
>>>>>> +			enum_force_help[name] = test_flag("ForceHelp", props, "true")
>>>>>> +			if (enum_force_help[name] == "")
>>>>>> +			  enum_force_help[name] = "false"
>>>>>>  			n_enums++
>>>>>>  		}
>>>>>>  		else if ($1 == "EnumValue")  {
>>>>>> diff --git a/gcc/optc-gen.awk b/gcc/optc-gen.awk
>>>>>> index bf177e86330..5c4f4239db0 100644
>>>>>> --- a/gcc/optc-gen.awk
>>>>>> +++ b/gcc/optc-gen.awk
>>>>>> @@ -167,7 +167,8 @@ for (i = 0; i < n_enums; i++) {
>>>>>>  	print "    cl_enum_" name "_data,"
>>>>>>  	print "    sizeof (" enum_type[name] "),"
>>>>>>  	print "    cl_enum_" name "_set,"
>>>>>> -	print "    cl_enum_" name "_get"
>>>>>> +	print "    cl_enum_" name "_get,"
>>>>>> +	print "    " enum_force_help[name]
>>>>>>  	print "  },"
>>>>>>  }
>>>>>>  print "};"
>>>>>> diff --git a/gcc/opts.c b/gcc/opts.c
>>>>>> index b8ae8756b4f..214ef806cd5 100644
>>>>>> --- a/gcc/opts.c
>>>>>> +++ b/gcc/opts.c
>>>>>> @@ -1337,7 +1337,8 @@ print_filtered_help (unsigned int include_flags,
>>>>>>      {
>>>>>>        unsigned int j, pos;
>>>>>>  
>>>>>> -      if (opts->x_help_enum_printed[i] != 1)
>>>>>> +      if (opts->x_help_enum_printed[i] != 1
>>>>>> +	  && !cl_enums[i].force_help)
>>>>>>  	continue;
>>>>>>        if (cl_enums[i].help == NULL)
>>>>>>  	continue;
>>>>>> diff --git a/gcc/opts.h b/gcc/opts.h
>>>>>> index 3723bdbf95b..c8777b3cd6a 100644
>>>>>> --- a/gcc/opts.h
>>>>>> +++ b/gcc/opts.h
>>>>>> @@ -193,6 +193,9 @@ struct cl_enum
>>>>>>  
>>>>>>    /* Function to get the value of a variable of this type.  */
>>>>>>    int (*get) (const void *var);
>>>>>> +
>>>>>> +  /* Force enum to be printed in help.  */
>>>>>> +  bool force_help;
>>>>>>  };
>>>>>>  
>>>>>>  extern const struct cl_enum cl_enums[];
>>>>>>
>>>>>
>>>>
>>>>
>>>> 0001-Show-valid-options-for-march-and-mtune-in-help-targe.patch
>>>>
>>>>
>>>> From dcb80bbe7b82388f5c7147320d509d6e5a687033 Mon Sep 17 00:00:00 2001
>>>> From: marxin <mliska@suse.cz>
>>>> Date: Tue, 20 Feb 2018 10:39:09 +0100
>>>> Subject: [PATCH] Show valid options for -march and -mtune in --help=target for
>>>>  arm32 (PR driver/83193).
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2018-07-19  Martin Liska  <mliska@suse.cz>
>>>>
>>>> 	* config/arm/arm.opt: Use HelpEnum flag.
>>>> 	* doc/options.texi: Document it.
>>>> 	* opt-functions.awk: Parse HelpEnum and fill up
>>>>         proper index into enum list.
>>>> 	* opts.c (print_filtered_help): Mention also enums
>>>>         that are not directly, but mentioned in HelpEnum.
>>>> 	* opts.h (struct cl_option): Add new field var_enum_help
>>>>         and change var_enum to signed version.
>>>> ---
>>>>  gcc/config/arm/arm.opt |  6 +++---
>>>>  gcc/doc/options.texi   |  5 +++++
>>>>  gcc/opt-functions.awk  | 25 +++++++++++++++----------
>>>>  gcc/opts.c             |  3 +++
>>>>  gcc/opts.h             |  5 ++++-
>>>>  5 files changed, 30 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
>>>> index a1286a4a861..f182208edd1 100644
>>>> --- a/gcc/config/arm/arm.opt
>>>> +++ b/gcc/config/arm/arm.opt
>>>> @@ -82,7 +82,7 @@ mapcs-stack-check
>>>>  Target Report Mask(APCS_STACK) Undocumented
>>>>  
>>>>  march=
>>>> -Target RejectNegative ToLower Joined Var(arm_arch_string)
>>>> +Target RejectNegative ToLower Joined Var(arm_arch_string) HelpEnum(arm_arch)
>>>>  Specify the name of the target architecture.
>>>>  
>>>>  ; Other arm_arch values are loaded from arm-tables.opt
>>>> @@ -107,7 +107,7 @@ Target Report Mask(CALLER_INTERWORKING)
>>>>  Thumb: Assume function pointers may go to non-Thumb aware code.
>>>>  
>>>>  mcpu=
>>>> -Target RejectNegative ToLower Joined Var(arm_cpu_string)
>>>> +Target RejectNegative ToLower Joined Var(arm_cpu_string) HelpEnum(processor_type)
>>>>  Specify the name of the target CPU.
>>>>  
>>>>  mfloat-abi=
>>>> @@ -232,7 +232,7 @@ Target Report Mask(TPCS_LEAF_FRAME)
>>>>  Thumb: Generate (leaf) stack frames even if not needed.
>>>>  
>>>>  mtune=
>>>> -Target RejectNegative ToLower Joined Var(arm_tune_string)
>>>> +Target RejectNegative ToLower Joined Var(arm_tune_string) HelpEnum(processor_type)
>>>>  Tune code for the given processor.
>>>>  
>>>>  mprint-tune-info
>>>> diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
>>>> index b3ca9f6fce6..e234cc809d5 100644
>>>> --- a/gcc/doc/options.texi
>>>> +++ b/gcc/doc/options.texi
>>>> @@ -120,6 +120,11 @@ being described by this record.
>>>>  This property is required; it says what value (representable as
>>>>  @code{int}) should be used for the given string.
>>>>  
>>>> +@item EnumHelp(@var{name})
>>>> +Some string options accept enum values as arguments.
>>>> +When using @samp{EnumHelp}, list of possible values is listed
>>>> +in @option{--help} output.
>>>> +
>>>>  @item Canonical
>>>>  This property is optional.  If present, it says the present string is
>>>>  the canonical one among all those with the given value.  Other strings
>>>> diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
>>>> index 2c371e5a23a..9737b6b115b 100644
>>>> --- a/gcc/opt-functions.awk
>>>> +++ b/gcc/opt-functions.awk
>>>> @@ -232,37 +232,42 @@ function var_type_struct(flags)
>>>>  }
>>>>  
>>>>  # Given that an option has flags FLAGS, return an initializer for the
>>>> -# "var_enum", "var_type" and "var_value" fields of its cl_options[] entry.
>>>> +# "var_enum", "var_enum_help", "var_type" and "var_value" fields of its cl_options[] entry.
>>>>  function var_set(flags)
>>>>  {
>>>> +  help_enum_index = "-1"
>>>> +	if (flag_set_p("HelpEnum.*", flags)) {
>>>> +		help_enum_index = enum_index[opt_args("HelpEnum", flags)];
>>>> +	}
>>>> +
>>>>  	if (flag_set_p("Defer", flags))
>>>> -		return "0, CLVC_DEFER, 0"
>>>> +		return "-1, -1, CLVC_DEFER, 0"
>>>>  	s = nth_arg(1, opt_args("Var", flags))
>>>>  	if (s != "")
>>>> -		return "0, CLVC_EQUAL, " s
>>>> +		return "-1, -1, CLVC_EQUAL, " s
>>>>  	s = opt_args("Mask", flags);
>>>>  	if (s != "") {
>>>>  		vn = var_name(flags);
>>>>  		if (vn)
>>>> -			return "0, CLVC_BIT_SET, OPTION_MASK_" s
>>>> +			return "-1, -1, CLVC_BIT_SET, OPTION_MASK_" s
>>>>  		else
>>>> -			return "0, CLVC_BIT_SET, MASK_" s
>>>> +			return "-1, -1, CLVC_BIT_SET, MASK_" s
>>>>  	}
>>>>  	s = nth_arg(0, opt_args("InverseMask", flags));
>>>>  	if (s != "") {
>>>>  		vn = var_name(flags);
>>>>  		if (vn)
>>>> -			return "0, CLVC_BIT_CLEAR, OPTION_MASK_" s
>>>> +			return "-1, -1, CLVC_BIT_CLEAR, OPTION_MASK_" s
>>>>  		else
>>>> -			return "0, CLVC_BIT_CLEAR, MASK_" s
>>>> +			return "-1, -1, CLVC_BIT_CLEAR, MASK_" s
>>>>  	}
>>>>  	if (flag_set_p("Enum.*", flags)) {
>>>>  		en = opt_args("Enum", flags);
>>>> -		return enum_index[en] ", CLVC_ENUM, 0"
>>>> +		return enum_index[en] ", -1, CLVC_ENUM, 0"
>>>>  	}
>>>>  	if (var_type(flags) == "const char *")
>>>> -		return "0, CLVC_STRING, 0"
>>>> -	return "0, CLVC_BOOLEAN, 0"
>>>> +		return "-1, " help_enum_index ", CLVC_STRING, 0"
>>>> +	return "-1, -1, CLVC_BOOLEAN, 0"
>>>>  }
>>>>  
>>>>  # Given that an option called NAME has flags FLAGS, return an initializer
>>>> diff --git a/gcc/opts.c b/gcc/opts.c
>>>> index b8ae8756b4f..37e779e8732 100644
>>>> --- a/gcc/opts.c
>>>> +++ b/gcc/opts.c
>>>> @@ -1303,6 +1303,9 @@ print_filtered_help (unsigned int include_flags,
>>>>        if (option->var_type == CLVC_ENUM
>>>>  	  && opts->x_help_enum_printed[option->var_enum] != 2)
>>>>  	opts->x_help_enum_printed[option->var_enum] = 1;
>>>> +      else if (option->var_enum_help != -1
>>>> +	       && opts->x_help_enum_printed[option->var_enum_help] != 2)
>>>> +	opts->x_help_enum_printed[option->var_enum_help] = 1;
>>>>      }
>>>>  
>>>>    if (! found)
>>>> diff --git a/gcc/opts.h b/gcc/opts.h
>>>> index 3723bdbf95b..ba79a36d7ab 100644
>>>> --- a/gcc/opts.h
>>>> +++ b/gcc/opts.h
>>>> @@ -107,7 +107,10 @@ struct cl_option
>>>>    unsigned short flag_var_offset;
>>>>    /* Index in cl_enums of enum used for this option's arguments, for
>>>>       CLVC_ENUM options.  */
>>>> -  unsigned short var_enum;
>>>> +  short var_enum;
>>>> +  /* Index in cl_enums of enum used for string options which
>>>> +     have strings as possible values.  */
>>>> +  short var_enum_help;
>>>>    /* How this option's value is determined and sets a field.  */
>>>>    enum cl_var_type var_type;
>>>>    /* Value or bit-mask with which to set a field.  */
>>>>
>>>
>>
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]