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?
> 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/. Would also
be good to put @samp{extra} in alphabetical order wrt the other options.
OK with those changes, and thanks for doing this.
Richard
