From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67149 invoked by alias); 7 Feb 2020 17:20:49 -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 67141 invoked by uid 89); 7 Feb 2020 17:20:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.1 spammy=Individual X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Feb 2020 17:20:47 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 998521FB; Fri, 7 Feb 2020 09:20:45 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 261593F68E; Fri, 7 Feb 2020 09:20:45 -0800 (PST) From: Richard Sandiford To: Roman Zhuykov Mail-Followup-To: Roman Zhuykov ,"gcc-patches\@gcc.gnu.org" , richard.sandiford@arm.com Cc: "gcc-patches\@gcc.gnu.org" Subject: Re: [PATCH] issues with configure --enable-checking option References: <540c7900-96e4-d762-56be-6b453a0a38a7@ispras.ru> Date: Fri, 07 Feb 2020 17:20:00 -0000 In-Reply-To: <540c7900-96e4-d762-56be-6b453a0a38a7@ispras.ru> (Roman Zhuykov's message of "Wed, 29 Jan 2020 16:32:45 +0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00456.txt.bz2 Roman Zhuykov writes: > Hi! > I've investigated a bit, because some of the following confused me while= =20 > working with some local 9.2-based branch. > > Documentation issues: > (0) See patch for install.texi at the bottom, two possible values are=20 > not documented. Ok for master? Backports? > (1) For me it seems confusing to have 'tree' and 'gimple' values, but=20 > not sure how to solve this. > (2) Developer has to look into configure scripts to understand which=20 > macro will be defined. E.q. 'misc' means "CHECKING_P". > (3) Install.texi IMHO doesn't properly describe what happens when=20 > --enable-checking is used without "=3Dlist". Maybe we should explicitly=20 > tell this means same as "=3Dyes". > (4) Statement "This is =E2=80=98yes,extra=E2=80=99 by default when buildi= ng from the=20 > source repository or snapshots." is wrong, because 'snapshots' may=20 > relate to released branches (e.q. GCC 9-20200125 Snapshot), and=20 > gcc/DEV-PHASE is empty there. > (5) Statement "=E2=80=98extra=E2=80=99 adds for =E2=80=98misc=E2=80=99 ch= ecking extra checks ..." is=20 > also confusing, one can run 'configure --enable-checking=3Dextra' and wil= l=20 > have only ENABLE_EXTRA_CHECKING, but not CHECKING_P, and in common.opt=20 > flag_checking will have Init(0). > > Behavior issues: > (6) It is not obvious to have default --enable-checking=3Drelease on any= =20 > commit in releases/* branches (DEV-PHASE is empty there). IMHO it's=20 > enough 'experimental' when picking for example some commit between 9.1=20 > and 9.2. This also can confuse 'git bisect run' scenario when good=20 > revision is old enough and bad revision is on release branch. See also (4= ). > (7) Running "configure --enable-checking" means less (!) checks on=20 > master branch (where DEV-PHASE is experimental). Default is "yes+extra"=20 > and you get only "yes" with that option. > (8) Why we always start with "release" values ('assert'+'runtime') as=20 > default? If developer runs "configure --enable-checking=3Ddf,rtl,tree"=20 > probably it should mean only picked values are enabled. Why we silently=20 > add 'assert' and 'runtime' into that set? > > I haven't tried to find additional issues with related=20 > '--enable-stage1-checking' option. > > Roman > > PS. I see some lines have more than 80 chars in install.texi, few of=20 > them were added recently while cleaning references to SVN. Patch fixes=20 > this only forthe paragraph it touches. > -- > > gcc/ChangeLog: > > 2020-01-29=C2=A0 Roman Zhuykov > > =C2=A0=C2=A0=C2=A0 * doc/install.texi: Document 'types' and 'gimple' val= ues for > =C2=A0=C2=A0=C2=A0 '--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.= =C2=A0=20 > This does not change the > =C2=A0generated code, but adds error checking within the compiler.=C2=A0= This will > =C2=A0slow down the compiler and may only work properly if you are build= ing > =C2=A0the compiler with GCC@.=C2=A0 This is @samp{yes,extra} by default = when building > -from the source repository or snapshots, but @samp{release} for=20 > releases.=C2=A0 The default > -for building the stage1 compiler is @samp{yes}.=C2=A0 More control > +from the source repository or snapshots, but @samp{release} for releases. > +The default for building the stage1 compiler is @samp{yes}.=C2=A0 More c= ontrol Pre-existing problem, but: it looks like the current default is yes,types: [if test "x$enable_checking" =3D xno || test "x$enable_checking" =3D x; then # For --disable-checking or implicit --enable-checking=3Drelease, avoid # setting --enable-checking=3Dgc in the default stage1 checking for LTO # bootstraps. See PR62077. case $BUILD_CONFIG in *lto*) stage1_checking=3D--enable-checking=3Drelease,misc,gimple,rtlflag,tre= e,types;; *) stage1_checking=3D--enable-checking=3Dyes,types;; esac if test "x$enable_checking" =3D x && \ test -d ${srcdir}/gcc && \ test x"`cat ${srcdir}/gcc/DEV-PHASE`" =3D xexperimental; then stage1_checking=3D--enable-checking=3Dyes,types,extra fi else stage1_checking=3D--enable-checking=3D$enable_checking,types fi]) Could you fix that while you're there? > =C2=A0over the checks may be had by specifying @var{list}.=C2=A0 The cat= egories of > =C2=A0checks 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= =20 > checks > +at all), @samp{all} (all but @samp{valgrind}), @samp{release} (cheapest > =C2=A0checks @samp{assert,runtime}) or @samp{none} (same as @samp{no}). > =C2=A0Individual 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=20 > @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{mis= c}, > +@samp{rtl}, @samp{rtlflag}, @samp{runtime}, @samp{tree}, @samp{types}, > +@samp{extra} and @samp{valgrind}. @samp{extra} adds for @samp{misc}=20 > 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