From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117683 invoked by alias); 17 Feb 2020 11:01:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 117593 invoked by uid 89); 17 Feb 2020 11:01:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=Individual, touches, categories X-HELO: smtp.ispras.ru Received: from winnie.ispras.ru (HELO smtp.ispras.ru) (83.149.199.91) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Feb 2020 11:01:43 +0000 Received: from [10.10.3.54] (utre4ko.intra.ispras.ru [10.10.3.54]) by smtp.ispras.ru (Postfix) with ESMTP id E4D0B203C9; Mon, 17 Feb 2020 14:01:38 +0300 (MSK) Subject: Re: [PATCH] issues with configure --enable-checking option From: Roman Zhuykov To: "gcc-patches@gcc.gnu.org" , Richard Sandiford Cc: Richard Biener , Jakub Jelinek , Sandra Loosemore , Alexander Monakov References: <540c7900-96e4-d762-56be-6b453a0a38a7@ispras.ru> <2f34ffaf-5038-01bd-4881-262a964e1820@ispras.ru> Message-ID: <0bc43bfc-00df-8a19-0561-f859c35b4f93@ispras.ru> Date: Mon, 17 Feb 2020 11:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <2f34ffaf-5038-01bd-4881-262a964e1820@ispras.ru> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00941.txt.bz2 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 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 >>> >>>     * 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  > >     * 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[,]} > +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 >