contrib/check-params-in-docs.py: Ignore target-specific params

On Thu 2024-04-11 20:51:55, Thomas Schwinge wrote:
> Hi!
> 
> On 2024-04-11T19:52:51+0200, Martin Jambor <mjambor@suse.cz> wrote:
> > contrib/check-params-in-docs.py is a script that checks that all
> > options reported with ./gcc/xgcc -Bgcc --help=param are in
> > gcc/doc/invoke.texi and vice versa.
> 
> Eh, first time I'm hearing about this one!
> 
> (a) Shouldn't this be running as part of the GCC build process?
> 
> > gcn-preferred-vectorization-factor is in the manual but normally not
> > reported by --help, probably because I do not have gcn offload
> > configured.
> 
> No, because you've not been building GCC for GCN target.  ;-P
> 
> > This patch makes the script silently about this particular
> > fact.
> 
> (b) Shouldn't we instead ignore any '--param's with "gcn" prefix, similar
> to how that's done for "skip aarch64 params"?
> 
> (c) ..., and shouldn't we likewise skip any "x86" ones?
> 
> (d) ..., or in fact any target specific ones, following after the generic
> section?  (Easily achieved with a special marker in
> 'gcc/doc/invoke.texi', just before:
> 
>     The following choices of @var{name} are available on AArch64 targets:
> 
> ..., and adjusting the 'takewhile' in 'contrib/check-params-in-docs.py'
> accordingly?

Hi,

I've made a patch to address (b), (c), (d).  I didn't adjust takewhile.  I
chose to do it differently since target-specific params in both invoke.texi and
--help=params have to be ignored.

The downside of this patch is that the script won't complain if someone adds a
target-specific param and doesn't document it.

What do you think?

Cheers,
Filip

-- 8< --

contrib/check-params-in-docs.py is a script that checks that all options
reported with gcc --help=params are in gcc/doc/invoke.texi and vice
versa.
gcc/doc/invoke.texi lists target-specific params but gcc --help=params
doesn't.  This meant that the script would mistakenly complain about
parms missing from --help=params.  Previously, the script was just set
to ignore aarch64 and gcn params which solved this issue only for x86.
This patch sets the script to ignore all target-specific params.

contrib/ChangeLog:

	* check-params-in-docs.py: Ignore target specific params.

Signed-off-by: Filip Kastl <fkastl@suse.cz>
---
 contrib/check-params-in-docs.py | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Hi!

On 2024-04-12T09:08:13+0200, Filip Kastl <fkastl@suse.cz> wrote:
> On Thu 2024-04-11 20:51:55, Thomas Schwinge wrote:
>> On 2024-04-11T19:52:51+0200, Martin Jambor <mjambor@suse.cz> wrote:
>> > contrib/check-params-in-docs.py is a script that checks that all
>> > options reported with ./gcc/xgcc -Bgcc --help=param are in
>> > gcc/doc/invoke.texi and vice versa.
>> 
>> Eh, first time I'm hearing about this one!
>> 
>> (a) Shouldn't this be running as part of the GCC build process?
>> 
>> > gcn-preferred-vectorization-factor is in the manual but normally not
>> > reported by --help, probably because I do not have gcn offload
>> > configured.
>> 
>> No, because you've not been building GCC for GCN target.  ;-P
>> 
>> > This patch makes the script silently about this particular
>> > fact.
>> 
>> (b) Shouldn't we instead ignore any '--param's with "gcn" prefix, similar
>> to how that's done for "skip aarch64 params"?
>> 
>> (c) ..., and shouldn't we likewise skip any "x86" ones?
>> 
>> (d) ..., or in fact any target specific ones, following after the generic
>> section?  (Easily achieved with a special marker in
>> 'gcc/doc/invoke.texi', just before:
>> 
>>     The following choices of @var{name} are available on AArch64 targets:
>> 
>> ..., and adjusting the 'takewhile' in 'contrib/check-params-in-docs.py'
>> accordingly?

> I've made a patch to address (b), (c), (d).  I didn't adjust takewhile.  I
> chose to do it differently since target-specific params in both invoke.texi and
> --help=params have to be ignored.

Right, I realized that after I had sent my email...

> The downside of this patch is that the script won't complain if someone adds a
> target-specific param and doesn't document it.

Yes, but that's a pre-existing problem -- unless you happened to be
targeting some x86 variant.  The target-specific '--param's will have to
be handled differently.

> What do you think?

Looks like a good incremental improvement to me, thanks!


Grüße
 Thomas


> contrib/check-params-in-docs.py is a script that checks that all options
> reported with gcc --help=params are in gcc/doc/invoke.texi and vice
> versa.
> gcc/doc/invoke.texi lists target-specific params but gcc --help=params
> doesn't.  This meant that the script would mistakenly complain about
> parms missing from --help=params.  Previously, the script was just set
> to ignore aarch64 and gcn params which solved this issue only for x86.
> This patch sets the script to ignore all target-specific params.
>
> contrib/ChangeLog:
>
> 	* check-params-in-docs.py: Ignore target specific params.
>
> Signed-off-by: Filip Kastl <fkastl@suse.cz>
> ---
>  contrib/check-params-in-docs.py | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/check-params-in-docs.py b/contrib/check-params-in-docs.py
> index f7879dd8e08..ccdb8d72169 100755
> --- a/contrib/check-params-in-docs.py
> +++ b/contrib/check-params-in-docs.py
> @@ -38,6 +38,9 @@ def get_param_tuple(line):
>      description = line[i:].strip()
>      return (name, description)
>  
> +def target_specific(param):
> +    return param.split('-')[0] in ('aarch64', 'gcn', 'x86')
> +
>  
>  parser = argparse.ArgumentParser()
>  parser.add_argument('texi_file')
> @@ -45,13 +48,16 @@ parser.add_argument('params_output')
>  
>  args = parser.parse_args()
>  
> -ignored = {'logical-op-non-short-circuit', 'gcn-preferred-vectorization-factor'}
> -params = {}
> +ignored = {'logical-op-non-short-circuit'}
> +help_params = {}
>  
>  for line in open(args.params_output).readlines():
>      if line.startswith(' ' * 2) and not line.startswith(' ' * 8):
>          r = get_param_tuple(line)
> -        params[r[0]] = r[1]
> +        help_params[r[0]] = r[1]
> +
> +# Skip target-specific params
> +help_params = [x for x in help_params.keys() if not target_specific(x)]
>  
>  # Find section in .texi manual with parameters
>  texi = ([x.strip() for x in open(args.texi_file).readlines()])
> @@ -66,14 +72,13 @@ for line in texi:
>              texi_params.append(line[len(token):])
>              break
>  
> -# skip digits
> +# Skip digits
>  texi_params = [x for x in texi_params if not x[0].isdigit()]
> -# skip aarch64 params
> -texi_params = [x for x in texi_params if not x.startswith('aarch64')]
> -sorted_params = sorted(texi_params)
> +# Skip target-specific params
> +texi_params = [x for x in texi_params if not target_specific(x)]
>  
>  texi_set = set(texi_params) - ignored
> -params_set = set(params.keys()) - ignored
> +params_set = set(help_params) - ignored
>  
>  success = True
>  extra = texi_set - params_set
> -- 
> 2.43.1

Hi,

On Fri, Apr 12 2024, Filip Kastl wrote:
> On Thu 2024-04-11 20:51:55, Thomas Schwinge wrote:
>> Hi!
>> 
>> On 2024-04-11T19:52:51+0200, Martin Jambor <mjambor@suse.cz> wrote:
>> > contrib/check-params-in-docs.py is a script that checks that all
>> > options reported with ./gcc/xgcc -Bgcc --help=param are in
>> > gcc/doc/invoke.texi and vice versa.
>> 
>> Eh, first time I'm hearing about this one!

It's running as part of our internal buildbot that Martin Liška set up.

I must admit I did want to spend the minimum time necessary to fix the
failure and did not realize Filip was looking at it too until I commited
my simple fix...

>> 
>> (a) Shouldn't this be running as part of the GCC build process?
>> 
>> > gcn-preferred-vectorization-factor is in the manual but normally not
>> > reported by --help, probably because I do not have gcn offload
>> > configured.
>> 
>> No, because you've not been building GCC for GCN target.  ;-P
>> 
>> > This patch makes the script silently about this particular
>> > fact.
>> 
>> (b) Shouldn't we instead ignore any '--param's with "gcn" prefix, similar
>> to how that's done for "skip aarch64 params"?
>> 
>> (c) ..., and shouldn't we likewise skip any "x86" ones?
>> 
>> (d) ..., or in fact any target specific ones, following after the generic
>> section?  (Easily achieved with a special marker in
>> 'gcc/doc/invoke.texi', just before:
>> 
>>     The following choices of @var{name} are available on AArch64 targets:
>> 
>> ..., and adjusting the 'takewhile' in 'contrib/check-params-in-docs.py'
>> accordingly?
>
> Hi,
>
> I've made a patch to address (b), (c), (d).  I didn't adjust takewhile.  I
> chose to do it differently since target-specific params in both invoke.texi and
> --help=params have to be ignored.
>
> The downside of this patch is that the script won't complain if someone adds a
> target-specific param and doesn't document it.
>
> What do you think?

...and this is clearly much better.  Thanks!

Martin

>
> Cheers,
> Filip
>
> -- 8< --
>
> contrib/check-params-in-docs.py is a script that checks that all options
> reported with gcc --help=params are in gcc/doc/invoke.texi and vice
> versa.
> gcc/doc/invoke.texi lists target-specific params but gcc --help=params
> doesn't.  This meant that the script would mistakenly complain about
> parms missing from --help=params.  Previously, the script was just set
> to ignore aarch64 and gcn params which solved this issue only for x86.
> This patch sets the script to ignore all target-specific params.
>
> contrib/ChangeLog:
>
> 	* check-params-in-docs.py: Ignore target specific params.
>
> Signed-off-by: Filip Kastl <fkastl@suse.cz>
> ---
>  contrib/check-params-in-docs.py | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/check-params-in-docs.py b/contrib/check-params-in-docs.py
> index f7879dd8e08..ccdb8d72169 100755
> --- a/contrib/check-params-in-docs.py
> +++ b/contrib/check-params-in-docs.py
> @@ -38,6 +38,9 @@ def get_param_tuple(line):
>      description = line[i:].strip()
>      return (name, description)
>  
> +def target_specific(param):
> +    return param.split('-')[0] in ('aarch64', 'gcn', 'x86')
> +
>  
>  parser = argparse.ArgumentParser()
>  parser.add_argument('texi_file')
> @@ -45,13 +48,16 @@ parser.add_argument('params_output')
>  
>  args = parser.parse_args()
>  
> -ignored = {'logical-op-non-short-circuit', 'gcn-preferred-vectorization-factor'}
> -params = {}
> +ignored = {'logical-op-non-short-circuit'}
> +help_params = {}
>  
>  for line in open(args.params_output).readlines():
>      if line.startswith(' ' * 2) and not line.startswith(' ' * 8):
>          r = get_param_tuple(line)
> -        params[r[0]] = r[1]
> +        help_params[r[0]] = r[1]
> +
> +# Skip target-specific params
> +help_params = [x for x in help_params.keys() if not target_specific(x)]
>  
>  # Find section in .texi manual with parameters
>  texi = ([x.strip() for x in open(args.texi_file).readlines()])
> @@ -66,14 +72,13 @@ for line in texi:
>              texi_params.append(line[len(token):])
>              break
>  
> -# skip digits
> +# Skip digits
>  texi_params = [x for x in texi_params if not x[0].isdigit()]
> -# skip aarch64 params
> -texi_params = [x for x in texi_params if not x.startswith('aarch64')]
> -sorted_params = sorted(texi_params)
> +# Skip target-specific params
> +texi_params = [x for x in texi_params if not target_specific(x)]
>  
>  texi_set = set(texi_params) - ignored
> -params_set = set(params.keys()) - ignored
> +params_set = set(help_params) - ignored
>  
>  success = True
>  extra = texi_set - params_set
> -- 
> 2.43.1