Message ID | Zhjd3RG3cqKsKteb@localhost.localdomain |
---|---|
State | New |
Headers | show |
Series | contrib/check-params-in-docs.py: Ignore target-specific params | expand |
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
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