diff mbox series

issues with configure --enable-checking option

Message ID 540c7900-96e4-d762-56be-6b453a0a38a7@ispras.ru
State New
Headers show
Series issues with configure --enable-checking option | expand

Commit Message

Roman Zhuykov Jan. 29, 2020, 1:32 p.m. UTC
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 <zhroma@ispras.ru>

     * doc/install.texi: Document 'types' and 'gimple' values for
     '--enable-checking' configure option.

+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

Comments

Roman Zhuykov Feb. 4, 2020, 7:16 a.m. UTC | #1
Ping with CCing Jeff and Joseph.

I think patch is rather obvious, but hopefully other items will also get
some attention.

29.01.2020 16:32, Roman Zhuykov wrote:
> 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  <zhroma@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
>  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
> +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
>
>
>
Richard Sandiford Feb. 7, 2020, 5:20 p.m. UTC | #2
Roman Zhuykov <zhroma@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 <zhroma@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
Roman Zhuykov Feb. 11, 2020, 2:46 p.m. UTC | #3
07.02.2020 20:20, Richard Sandiford writes:
> Roman Zhuykov <zhroma@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 <zhroma@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  <zhroma@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
Roman Zhuykov Feb. 17, 2020, 11:01 a.m. UTC | #4
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 <zhroma@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 <zhroma@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  <zhroma@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
>
Sandra Loosemore Feb. 22, 2020, 3:25 a.m. UTC | #5
On 2/11/20 7:46 AM, Roman Zhuykov wrote:
> [snip]
> 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 ?

I'm not an expert on the content, but the new text reads OK except for 
using future tense to describe current behavior.  Namely:

> +requested complexity.  This will slow down the compiler and may only work

s/will slow/slows/

> +must be explicitly requested.  Disabling assertions will make the compiler
> +and runtime slightly faster but increase the risk of undetected internal

s/will make/makes/
s/increase/increases/

> +option is specified the stage1 compiler will be built with @samp{yes}

s/will be/is/

OK with those fixes applied.

-Sandra
Roman Zhuykov Feb. 25, 2020, 8:20 a.m. UTC | #6
Hi all!

22.02.2020 6:25, Sandra Loosemore wrote:
> On 2/11/20 7:46 AM, Roman Zhuykov wrote:
>> 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 ?
>
> I'm not an expert on the content, but the new text reads OK except for
> using future tense to describe current behavior.  Namely:
>
>> +requested complexity.  This will slow down the compiler and may only
>> work
>
> s/will slow/slows/
>
>> +must be explicitly requested.  Disabling assertions will make the
>> compiler
>> +and runtime slightly faster but increase the risk of undetected
>> internal
>
> s/will make/makes/
> s/increase/increases/
>
>> +option is specified the stage1 compiler will be built with @samp{yes}
>
> s/will be/is/
>
> OK with those fixes applied.

07.02.2020 20:20, Richard Sandiford wrote:
> OK with those changes, and thanks for doing this.

Sandra and Richard, thank you for review!

Since 'types' checks are included into 'yes' and I addressed all other
hints, I have pushed updated patch as r10-6832.

Jakub, Richard B, can I apply it to 8 and 9 ?

Roman


doc: properly describe --enable-checking behavior

This patch rewords the whole description to fix minor issues:
 - documents 'gimple' and 'types' checks,
 - clarifies what happens when option is used without '=list',
 - fixes inaccurate wrong wording about release snapshots,
 - describes that release checks can only be disabled explicitly.

gcc/ChangeLog:

2020-02-25  Roman Zhuykov  <zhroma@ispras.ru>

    * doc/install.texi (--enable-checking): Properly document current
    behavior.
    (--enable-stage1-checking): Minor clarification about bootstrap.

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1839,41 +1839,48 @@ 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 slows 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 makes the compiler and
+runtime slightly faster but increases 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
+This option affects only bootstrap build.  If no @option{--enable-checking}
+option is specified the stage1 compiler is 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}.
Jakub Jelinek Feb. 25, 2020, 8:30 a.m. UTC | #7
On Tue, Feb 25, 2020 at 11:20:58AM +0300, Roman Zhuykov wrote:
> Sandra and Richard, thank you for review!
> 
> Since 'types' checks are included into 'yes' and I addressed all other
> hints, I have pushed updated patch as r10-6832.
> 
> Jakub, Richard B, can I apply it to 8 and 9 ?

Yes.

> +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

Is archives the right word?  The --enable-checking=release by default is
turned on whenever it is a release branch, checked by whether gcc/DEV-PHASE
is not experimental, and generally applies to everything but the GCC trunk.

> +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.

I would certainly not say that rtl checking is very expensive, it is
somewhat expensive, but I'm using it in all my bootstraps and others are as
well.  On the other side, fold checking is extremely expensive.

So perhaps

The @samp{rtl} checks are expensive and the @samp{df},
@samp{fold}, @samp{gcac} and @samp{valgrind} checks are very expensive.

?

	Jakub
Roman Zhuykov Feb. 25, 2020, 10:36 a.m. UTC | #8
Hi, Jakub!

Jakub Jelinek writes:
>> +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
> Is archives the right word?  The --enable-checking=release by default is
> turned on whenever it is a release branch, checked by whether gcc/DEV-PHASE
> is not experimental, and generally applies to everything but the GCC trunk.

Not sure how to formulate it better, just "builds from release branch or
release archives default to ..." ?

Quoting myself:

> Documentation issues (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.
> ...
> 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.

I've started this thread with telling about a bit strange behavior and
wrong documentation wording for release snapshots.  I was planing to ask
again about behavior issues later when stage1 starts, certainly it can't
be changed without slightly modifying some branching/release
procedures.  AFAIR, after moving to git you have already posted some
patch offering solution to drop DATESTAMP bumps using some git tricks. 
We may also consider using other git tricks to check whether current
commit is tagged and working copy is clean.  E.g. in qemu Makefile they
have:

# Create QEMU_PKGVERSION and FULL_VERSION strings
# If PKGVERSION is set, use that; otherwise get version and -dirty
status from git
QEMU_PKGVERSION := $(if $(PKGVERSION),$(PKGVERSION),$(shell \
  cd $(SRC_PATH); \
  if test -e .git; then \
    git describe --match 'v*' 2>/dev/null | tr -d '\n'; \
    if ! git diff-index --quiet HEAD &>/dev/null; then \
      echo "-dirty"; \
    fi; \
  fi))

If GCC would use an approach like this to extract version string, it can
be extended for DEV-PHASE guessing.

So, IMHO the best next step is to improve the behavior rather then docs :)

>> +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.
> I would certainly not say that rtl checking is very expensive

Actually, that sentence wasn't changed in my patch, 'rtl' and 'gcac' are
in that list since 2001.

> it is somewhat expensive, but I'm using it in all my bootstraps and others are as
> well.

I forget all the time that I have to make the same practice.

> So perhaps
>
> The @samp{rtl} checks are expensive and the @samp{df},
> @samp{fold}, @samp{gcac} and @samp{valgrind} checks are very expensive.
>
> ?

Sure, I'm ready to update it in all active branches if we decide how to
deal with "release branches/archives" above.


Roman
Jakub Jelinek Feb. 25, 2020, 10:41 a.m. UTC | #9
On Tue, Feb 25, 2020 at 01:36:13PM +0300, Roman Zhuykov wrote:
> Not sure how to formulate it better, just "builds from release branch or
> release archives default to ..." ?

"from a release branch or" or "from release branches or", otherwise LGTM.

	Jakub
Roman Zhuykov March 10, 2020, 12:20 p.m. UTC | #10
Hi!

25.02.2020 13:36, Roman Zhuykov wrote:

> So, IMHO the best next step is to improve the behavior rather then docs :)

I want to add one more point into this discussion.  I have recently
decided to test some stuff on old branches, e.q gcc-4.9, 5 and 6.
On modern systems there are some issues with building old branches, at
least I met "struct ucontext -> ucontext_t",
"sys/ustat.h - no such file" and few others.  But in this particular
experiment, I was using pretty old Ubuntu 16.04,
and there were no issues with building unpatched frozen branches.

But, since the stuff was really experimental, at some moment I've
decided to apply the following to enable more checking:

diff --git a/gcc/DEV-PHASE b/gcc/DEV-PHASE
--- a/gcc/DEV-PHASE
+++ b/gcc/DEV-PHASE
@@ -0,0 +1 @@
+experimental
diff --git a/gcc/configure b/gcc/configure
--- a/gcc/configure
+++ b/gcc/configure
@@ -6727,7 +6727,7 @@ do
     # these set all the flags to specific states
     yes)        ac_assert_checking=1 ; ac_checking=1 ; ac_df_checking= ;
             ac_fold_checking= ; ac_gc_checking=1 ;
-            ac_gc_always_collect= ; ac_gimple_checking=1 ;
ac_rtl_checking= ;
+            ac_gc_always_collect= ; ac_gimple_checking=1 ;
ac_rtl_checking=1 ;
             ac_rtlflag_checking=1 ; ac_runtime_checking=1 ;
             ac_tree_checking=1 ; ac_valgrind_checking= ;
             ac_types_checking=1 ;;
diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -427,7 +427,7 @@ do
     # these set all the flags to specific states
     yes)        ac_assert_checking=1 ; ac_checking=1 ; ac_df_checking= ;
             ac_fold_checking= ; ac_gc_checking=1 ;
-            ac_gc_always_collect= ; ac_gimple_checking=1 ;
ac_rtl_checking= ;
+            ac_gc_always_collect= ; ac_gimple_checking=1 ;
ac_rtl_checking=1 ;
             ac_rtlflag_checking=1 ; ac_runtime_checking=1 ;
             ac_tree_checking=1 ; ac_valgrind_checking= ;
             ac_types_checking=1 ;;

And that gives broken basic x86_64 bootstrap on gcc-5 and gcc-6 branches!

First, about gcc-4.9 branch, it works fine.  There was another story
that it failed with rtl checks on ppc64.  Alex told me there was some
moment when all folks forgot to test that.  So, everything went fine
after backporting r243144 (r5-10072) and r212829 (r5-1977).

But the gcc-5 branch case was much more tricky.  The following 3 hunks
are needed to fix x86_64 bootstrap.
And that one with UNKNOWN_LOCATION is really a null pointer dereference
we put into a released compiler!

diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
--- a/gcc/fortran/frontend-passes.c
+++ b/gcc/fortran/frontend-passes.c
@@ -138,7 +138,7 @@ gfc_run_passes (gfc_namespace *ns)
  */
 
 static int
-realloc_string_callback (gfc_code **c, int *walk_subtrees,
+realloc_string_callback (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED,
              void *data ATTRIBUTE_UNUSED)
 {
   gfc_expr *expr1, *expr2;
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -38,7 +38,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"    /* For create_tmp_var_raw.  */
 #include "stringpool.h"
 #include "gfortran.h"
-#include "diagnostic-core.h"    /* For internal_error.  */
 #include "trans.h"
 #include "trans-stmt.h"
 #include "trans-types.h"
diff --git a/gcc/toplev.c b/gcc/toplev.c
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1386,8 +1386,7 @@ process_options (void)
 
       if (flag_sanitize & SANITIZE_THREAD)
     {
-      error (UNKNOWN_LOCATION,
-         "%<-fcheck-pointer-bounds%> is not supported with "
+      error ("%<-fcheck-pointer-bounds%> is not supported with "
          "Thread Sanitizer");
 
       flag_check_pointer_bounds = 0;


In gcc-6 branch the solution was simple - I have to revert r262046
(r6-10168), haven't investigate that deeply.

Overall, IMHO this is one more point to review current
branching/checking approach.
So, I understand that main purpose of empty DEV-PHASE is to test "almost
released" compiler in the same way it will be compiled from release
archives.
But those issues show it's also necessary to run checking-bootstrap when
backporting any patch.

PS. Everything seems fine in gcc-7 branch.

Roman
diff mbox series

Patch

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
  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