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 >