Ping.  Proposed patch is docs-only (install.texi), and IMHO it's better
to push it into 8.4 and 9.3.

11.02.2020 17:46, Roman Zhuykov wrote:
> 07.02.2020 20:20, Richard Sandiford writes:
>> Roman Zhuykov <zhr...@ispras.ru> writes:
>>> Hi!
>>> I've investigated a bit, because some of the following confused me while 
>>> working with some local 9.2-based branch.
>>>
>>> Documentation issues:
>>> (0) See patch for install.texi at the bottom, two possible values are 
>>> not documented. Ok for master? Backports?
>>> (1) For me it seems confusing to have 'tree' and 'gimple' values, but 
>>> not sure how to solve this.
>>> (2) Developer has to look into configure scripts to understand which 
>>> macro will be defined. E.q. 'misc' means "CHECKING_P".
>>> (3) Install.texi IMHO doesn't properly describe what happens when 
>>> --enable-checking is used without "=list". Maybe we should explicitly 
>>> tell this means same as "=yes".
>>> (4) Statement "This is ‘yes,extra’ by default when building from the 
>>> source repository or snapshots." is wrong, because 'snapshots' may 
>>> relate to released branches (e.q. GCC 9-20200125 Snapshot), and 
>>> gcc/DEV-PHASE is empty there.
>>> (5) Statement "‘extra’ adds for ‘misc’ checking extra checks ..." is 
>>> also confusing, one can run 'configure --enable-checking=extra' and will 
>>> have only ENABLE_EXTRA_CHECKING, but not CHECKING_P, and in common.opt 
>>> flag_checking will have Init(0).
>>>
>>> Behavior issues:
>>> (6) It is not obvious to have default --enable-checking=release on any 
>>> commit in releases/* branches (DEV-PHASE is empty there). IMHO it's 
>>> enough 'experimental' when picking for example some commit between 9.1 
>>> and 9.2. This also can confuse 'git bisect run' scenario when good 
>>> revision is old enough and bad revision is on release branch. See also (4).
>>> (7) Running "configure --enable-checking" means less (!) checks on 
>>> master branch (where DEV-PHASE is experimental). Default is "yes+extra" 
>>> and you get only "yes" with that option.
>>> (8) Why we always start with "release" values ('assert'+'runtime') as 
>>> default? If developer runs "configure --enable-checking=df,rtl,tree" 
>>> probably it should mean only picked values are enabled. Why we silently 
>>> add 'assert' and 'runtime' into that set?
>>>
>>> I haven't tried to find additional issues with related 
>>> '--enable-stage1-checking' option.
>>>
>>> Roman
>>>
>>> PS. I see some lines have more than 80 chars in install.texi, few of 
>>> them were added recently while cleaning references to SVN. Patch fixes 
>>> this only forthe paragraph it touches.
>>> --
>>>
>>> gcc/ChangeLog:
>>>
>>> 2020-01-29  Roman Zhuykov <zhr...@ispras.ru>
>>>
>>>      * doc/install.texi: Document 'types' and 'gimple' values for
>>>      '--enable-checking' configure option.
>>>
>>> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
>>> --- a/gcc/doc/install.texi
>>> +++ b/gcc/doc/install.texi
>>> @@ -1845,19 +1845,19 @@ consistency checks of the requested complexity.  
>>> This does not change the
>>>   generated code, but adds error checking within the compiler.  This will
>>>   slow down the compiler and may only work properly if you are building
>>>   the compiler with GCC@.  This is @samp{yes,extra} by default when building
>>> -from the source repository or snapshots, but @samp{release} for 
>>> releases.  The default
>>> -for building the stage1 compiler is @samp{yes}.  More control
>>> +from the source repository or snapshots, but @samp{release} for releases.
>>> +The default for building the stage1 compiler is @samp{yes}.  More control
>> Pre-existing problem, but: it looks like the current default is yes,types:
>>
>> [if test "x$enable_checking" = xno || test "x$enable_checking" = x; then
>>   # For --disable-checking or implicit --enable-checking=release, avoid
>>   # setting --enable-checking=gc in the default stage1 checking for LTO
>>   # bootstraps.  See PR62077.
>>   case $BUILD_CONFIG in
>>     *lto*)
>>       
>> stage1_checking=--enable-checking=release,misc,gimple,rtlflag,tree,types;;
>>     *)
>>       stage1_checking=--enable-checking=yes,types;;
>>   esac
>>   if test "x$enable_checking" = x && \
>>      test -d ${srcdir}/gcc && \
>>      test x"`cat ${srcdir}/gcc/DEV-PHASE`" = xexperimental; then
>>     stage1_checking=--enable-checking=yes,types,extra
>>   fi
>> else
>>   stage1_checking=--enable-checking=$enable_checking,types
>> fi])
>>
>> Could you fix that while you're there?
> No change needed, documentation is correct here, that global
> configure.ac yes+types change was r126951 and a bit later in r133479 we
> have 'types' included into 'yes' set in gcc/configure.ac. In patch v1
> I've already included 'types' into 'yes' set description few lines below.
>
> I hope somebody will have a look at those behavior issues (6), (7), (8),
> probably on stage1 later. So, lets add
> (9) Remove unneeded 'types' item in stage1_checking in global configure.ac.
>
>>>   over the checks may be had by specifying @var{list}.  The categories of
>>>   checks available are @samp{yes} (most common checks
>>> -@samp{assert,misc,tree,gc,rtlflag,runtime}), @samp{no} (no checks at
>>> -all), @samp{all} (all but @samp{valgrind}), @samp{release} (cheapest
>>> +@samp{assert,misc,tree,gc,gimple,rtlflag,runtime,types}), @samp{no} (no 
>>> checks
>>> +at all), @samp{all} (all but @samp{valgrind}), @samp{release} (cheapest
>>>   checks @samp{assert,runtime}) or @samp{none} (same as @samp{no}).
>>>   Individual checks can be enabled with these flags @samp{assert},
>>> -@samp{df}, @samp{fold}, @samp{gc}, @samp{gcac}, @samp{misc}, @samp{rtl},
>>> -@samp{rtlflag}, @samp{runtime}, @samp{tree}, @samp{extra} and 
>>> @samp{valgrind}.
>>> -@samp{extra} adds for @samp{misc} checking extra checks that might affect
>>> -code generation and should therefore not differ between stage1 and later
>>> -stages.
>>> +@samp{df}, @samp{fold}, @samp{gc}, @samp{gcac}, @samp{gimple}, @samp{misc},
>>> +@samp{rtl}, @samp{rtlflag}, @samp{runtime}, @samp{tree}, @samp{types},
>>> +@samp{extra} and @samp{valgrind}. @samp{extra} adds for @samp{misc} 
>>> checking
>> Both of these are again pre-existing, but s/adds for/adds/.
> Fixed that in patch v2 with another wording.
>
>> Would also be good to put @samp{extra} in alphabetical order wrt the other 
>> options.
> Done.
>
>> OK with those changes,
> Since I have to ask again about backports, I've decided to make few more
> steps and with Alexander's help created new patch which rewords the
> whole option description and covers items (3), (4) and (8).  CCing Jakub
> and Richard as release managers, also ask Sandra to take a quick look if
> new wording is alright.  New patch suits all active branches.  OK for
> 10-9-8 ?
>
> Roman
> --
>
> gcc/ChangeLog:
>
> 2020-02-11  Roman Zhuykov  <zhr...@ispras.ru>
>
>     * doc/install.texi: Properly document current behavior of
>     '--enable-checking' and '--enable-stage1-checking' configure
>     options.
>
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -1839,42 +1839,49 @@ final releases.  The specific files which get
> @option{-Werror} are
>  controlled by the Makefiles.
>  
>  @item --enable-checking
> +@itemx --disable-checking
>  @itemx --enable-checking=@var{list}
> -When you specify this option, the compiler is built to perform internal
> -consistency checks of the requested complexity.  This does not change the
> -generated code, but adds error checking within the compiler.  This will
> -slow down the compiler and may only work properly if you are building
> -the compiler with GCC@.  This is @samp{yes,extra} by default when building
> -from the source repository or snapshots, but @samp{release} for
> releases.  The default
> -for building the stage1 compiler is @samp{yes}.  More control
> -over the checks may be had by specifying @var{list}.  The categories of
> -checks available are @samp{yes} (most common checks
> -@samp{assert,misc,tree,gc,rtlflag,runtime}), @samp{no} (no checks at
> -all), @samp{all} (all but @samp{valgrind}), @samp{release} (cheapest
> -checks @samp{assert,runtime}) or @samp{none} (same as @samp{no}).
> -Individual checks can be enabled with these flags @samp{assert},
> -@samp{df}, @samp{fold}, @samp{gc}, @samp{gcac}, @samp{misc}, @samp{rtl},
> -@samp{rtlflag}, @samp{runtime}, @samp{tree}, @samp{extra} and
> @samp{valgrind}.
> -@samp{extra} adds for @samp{misc} checking extra checks that might affect
> -code generation and should therefore not differ between stage1 and later
> -stages.
> -
> -The @samp{valgrind} check requires the external @command{valgrind}
> -simulator, available from @uref{http://valgrind.org/}.  The
> -@samp{df}, @samp{rtl}, @samp{gcac} and @samp{valgrind} checks are very
> expensive.
> -To disable all checking, @samp{--disable-checking} or
> -@samp{--enable-checking=none} must be explicitly requested.  Disabling
> -assertions will make the compiler and runtime slightly faster but
> -increase the risk of undetected internal errors causing wrong code to be
> -generated.
> +This option controls performing internal consistency checks in the
> compiler.
> +It does not change the generated code, but adds error checking of the
> +requested complexity.  This will slow down the compiler and may only work
> +properly if you are building the compiler with GCC@.
> +
> +When the option is not specified, the active set of checks depends on
> context.
> +Namely, bootstrap stage 1 defaults to @samp{--enable-checking=yes}, builds
> +from release archives default to @samp{--enable-checking=release}, and
> +otherwise @samp{--enable-checking=yes,extra} is used.  When the option is
> +specified without a @var{list}, the result is the same as
> +@samp{--enable-checking=yes}.  Likewise, @samp{--disable-checking} is
> +equivalent to @samp{--enable-checking=no}.
> +
> +The categories of checks available in @var{list} are @samp{yes} (most
> common
> +checks @samp{assert,misc,gc,gimple,rtlflag,runtime,tree,types}), @samp{no}
> +(no checks at all), @samp{all} (all but @samp{valgrind}), @samp{release}
> +(cheapest checks @samp{assert,runtime}) or @samp{none} (same as @samp{no}).
> +@samp{release} checks are always on and to disable them
> +@samp{--disable-checking} or @samp{--enable-checking=no[,<other checks>]}
> +must be explicitly requested.  Disabling assertions will make the compiler
> +and runtime slightly faster but increase the risk of undetected internal
> +errors causing wrong code to be generated.
> +
> +Individual checks can be enabled with these flags: @samp{assert},
> @samp{df},
> +@samp{extra}, @samp{fold}, @samp{gc}, @samp{gcac}, @samp{gimple},
> +@samp{misc}, @samp{rtl}, @samp{rtlflag}, @samp{runtime}, @samp{tree},
> +@samp{types} and @samp{valgrind}.  @samp{extra} extends @samp{misc}
> +checking with extra checks that might affect code generation and should
> +therefore not differ between stage1 and later stages in bootstrap.
> +
> +The @samp{valgrind} check requires the external @command{valgrind}
> simulator,
> +available from @uref{http://valgrind.org/}.  The @samp{df}, @samp{rtl},
> +@samp{gcac} and @samp{valgrind} checks are very expensive.
>  
>  @item --disable-stage1-checking
>  @itemx --enable-stage1-checking
>  @itemx --enable-stage1-checking=@var{list}
> -If no @option{--enable-checking} option is specified the stage1
> -compiler will be built with @samp{yes} checking enabled, otherwise
> -the stage1 checking flags are the same as specified by
> -@option{--enable-checking}.  To build the stage1 compiler with
> +This option affects only bootstrap build.  If no @option{--enable-checking}
> +option is specified the stage1 compiler will be built with @samp{yes}
> +checking enabled, otherwise the stage1 checking flags are the same as
> +specified by @option{--enable-checking}.  To build the stage1 compiler with
>  different checking options use @option{--enable-stage1-checking}.
>  The list of checking options is the same as for @option{--enable-checking}.
>  If your system is too slow or too small to bootstrap a released compiler
>

Reply via email to