Re: [PATCH v2] PR85678: Change default to -fno-common
On Mon, Nov 4, 2019 at 3:39 PM Wilco Dijkstra wrote:
>
> Hi Richard,
>
> >> > Please don't add -fcommon in lto.exp.
> >>
> >> So what is the best way to add an extra option to lto.exp?
> >> Note dg-lto-options completely overrides the options from lto.exp, so I
> >> can't
> >> use that except in tests which already use it.
> >
> > On what testcases do you need it at all?
>
> These need it in order to run over the original set of LTO options. A
> possibility
> would be to select one of the set of options and just run that using
> dg-lto-options
> (assuming it's safe to use -flto-partition and/or -flinker-plugin on all
> targets).
>
> PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_0.C line 3)
> PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_0.C line 3)
> PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_1.c line 1)
> PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_1.c line 1)
Please investigate those - C++ has -fno-common already so it might be a mix
of C/C++ required here. Note that secondary files can use dg-options
with the same behavior as dg-additional-options (they append to dg-lto-options),
so here in _1.c add { dg-options "-fcommon" }
> PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O0
> -flto -flto-partition=1to1 -fno-use-linker-plugin
> PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O0
> -flto -flto-partition=none -fuse-linker-plugin
> PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O0
> -flto -fuse-linker-plugin -fno-fat-lto-objects
> PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O2
> -flto -flto-partition=1to1 -fno-use-linker-plugin
> PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O2
> -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
> PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O2
> -flto -fuse-linker-plugin
>
>
> PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O0
> -flto -flto-partition=1to1 -fno-use-linker-plugin
> PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O0
> -flto -flto-partition=none -fuse-linker-plugin
> PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O0
> -flto -fuse-linker-plugin -fno-fat-lto-objects
> PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O2
> -flto -flto-partition=1to1 -fno-use-linker-plugin
> PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O2
> -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
> PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O2
> -flto -fuse-linker-plugin
This is a testcase relying on -fcommon to link (it has two
definitions), I belive you can
add dg-options "-fcommon" to the secondary file here as well, one
COMMON is enough
to make the link work.
Richard.
>
> Wilco
Re: [PATCH v2] PR85678: Change default to -fno-common
Hi Richard, >> > Please don't add -fcommon in lto.exp. >> >> So what is the best way to add an extra option to lto.exp? >> Note dg-lto-options completely overrides the options from lto.exp, so I can't >> use that except in tests which already use it. > > On what testcases do you need it at all? These need it in order to run over the original set of LTO options. A possibility would be to select one of the set of options and just run that using dg-lto-options (assuming it's safe to use -flto-partition and/or -flinker-plugin on all targets). PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_0.C line 3) PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_0.C line 3) PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_1.c line 1) PASS->FAIL: g++.dg/lto/odr-6 2 (test for LTO warnings, odr-6_1.c line 1) PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O0 -flto -flto-partition=1to1 -fno-use-linker-plugin PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O0 -flto -flto-partition=none -fuse-linker-plugin PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O0 -flto -fuse-linker-plugin -fno-fat-lto-objects PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O2 -flto -flto-partition=1to1 -fno-use-linker-plugin PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects PASS->FAIL: g++.dg/lto/odr-6 cp_lto_odr-6_0.o-cp_lto_odr-6_1.o link, -O2 -flto -fuse-linker-plugin PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O0 -flto -flto-partition=1to1 -fno-use-linker-plugin PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O0 -flto -flto-partition=none -fuse-linker-plugin PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O0 -flto -fuse-linker-plugin -fno-fat-lto-objects PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O2 -flto -flto-partition=1to1 -fno-use-linker-plugin PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects PASS->FAIL: gcc.dg/lto/pr88077 c_lto_pr88077_0.o-c_lto_pr88077_1.o link, -O2 -flto -fuse-linker-plugin Wilco
Re: [PATCH v2] PR85678: Change default to -fno-common
On Wed, Oct 30, 2019 at 3:33 PM Wilco Dijkstra wrote: > > Hi Richard, > > > Please don't add -fcommon in lto.exp. > > So what is the best way to add an extra option to lto.exp? > Note dg-lto-options completely overrides the options from lto.exp, so I can't > use that except in tests which already use it. On what testcases do you need it at all? Richard. > Cheers, > Wilco
Re: [PATCH v2] PR85678: Change default to -fno-common
Hi Richard, > Please don't add -fcommon in lto.exp. So what is the best way to add an extra option to lto.exp? Note dg-lto-options completely overrides the options from lto.exp, so I can't use that except in tests which already use it. Cheers, Wilco
Re: [PATCH v2] PR85678: Change default to -fno-common
On Tue, Oct 29, 2019 at 1:33 PM Wilco Dijkstra wrote:
>
> v2: Tweak testsuite options to avoid failures
>
> GCC currently defaults to -fcommon. As discussed in the PR, this is an
> ancient
> C feature which is not conforming with the latest C standards. On many
> targets
> this means global variable accesses have a codesize and performance penalty.
> This applies to C code only, C++ code is not affected by -fcommon. It is
> about
> time to change the default.
>
> Bootstrap OK, passes testsuite on AArch64. OK for commit?
Please don't add -fcommon in lto.exp.
> ChangeLog
> 2019-10-29 Wilco Dijkstra
>
> PR85678
> * common.opt (fcommon): Change init to 1.
>
> doc/
> * invoke.texi (-fcommon): Update documentation.
>
> testsuite/
>
> * gcc.dg/alias-15.c: Add -fcommon.
> * gcc.dg/fdata-sections-1.c: Likewise.
> * gcc.dg/ipa/pr77653.c: Likewise.
> * gcc.dg/lto/20090729_0.c: Likewise.
> * gcc.dg/lto/20111207-1_0.c: Likewise.
> * gcc.dg/lto/c-compatible-types-1_0.c: Likewise.
> * gcc.dg/lto/pr55525_0.c: Likewise.
> * gcc.target/aarch64/sve/peel_ind_1.c: Allow ANCHOR0.
> * gcc.target/aarch64/sve/peel_ind_2.c: Likewise
> * gcc.target/aarch64/sve/peel_ind_3.c: Likewise
> * lib/lto.exp (lto_init): Add -fcommon.
> ---
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index
> f74b10aafc223e4961915b009c092f4876eddba4..798b6aeff3536e21c95752b5dd085f8ffef04643
> 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1131,7 +1131,7 @@ Common Report Var(flag_combine_stack_adjustments)
> Optimization
> Looks for opportunities to reduce stack adjustments and stack references.
>
> fcommon
> -Common Report Var(flag_no_common,0)
> +Common Report Var(flag_no_common,0) Init(1)
> Put uninitialized globals in the common section.
>
> fcompare-debug
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index
> 92fb316a368a4a36218fac6de2744c7ab6446ef5..18cfd07d4bbb4b866808db0701faf88bddbd9a94
> 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -568,7 +568,7 @@ Objective-C and Objective-C++ Dialects}.
> -fnon-call-exceptions -fdelete-dead-exceptions -funwind-tables @gol
> -fasynchronous-unwind-tables @gol
> -fno-gnu-unique @gol
> --finhibit-size-directive -fno-common -fno-ident @gol
> +-finhibit-size-directive -fcommon -fno-ident @gol
> -fpcc-struct-return -fpic -fPIC -fpie -fPIE -fno-plt @gol
> -fno-jump-tables @gol
> -frecord-gcc-switches @gol
> @@ -14049,35 +14049,27 @@ useful for building programs to run under WINE@.
> code that is not binary compatible with code generated without that switch.
> Use it to conform to a non-default application binary interface.
>
> -@item -fno-common
> -@opindex fno-common
> +@item -fcommon
> @opindex fcommon
> +@opindex fno-common
> @cindex tentative definitions
> -In C code, this option controls the placement of global variables
> -defined without an initializer, known as @dfn{tentative definitions}
> -in the C standard. Tentative definitions are distinct from declarations
> +In C code, this option controls the placement of global variables
> +defined without an initializer, known as @dfn{tentative definitions}
> +in the C standard. Tentative definitions are distinct from declarations
> of a variable with the @code{extern} keyword, which do not allocate storage.
>
> -Unix C compilers have traditionally allocated storage for
> -uninitialized global variables in a common block. This allows the
> -linker to resolve all tentative definitions of the same variable
> +The default is @option{-fno-common}, which specifies that the compiler places
> +uninitialized global variables in the BSS section of the object file.
> +This inhibits the merging of tentative definitions by the linker so you get a
> +multiple-definition error if the same variable is accidentally defined in
> more
> +than one compilation unit.
> +
> +The @option{-fcommon} places uninitialized global variables in a common
> block.
> +This allows the linker to resolve all tentative definitions of the same
> variable
> in different compilation units to the same object, or to a non-tentative
> -definition.
> -This is the behavior specified by @option{-fcommon}, and is the default for
> -GCC on most targets.
> -On the other hand, this behavior is not required by ISO
> -C, and on some targets may carry a speed or code size penalty on
> -variable references.
> -
> -The @option{-fno-common} option specifies that the compiler should instead
> -place uninitialized global variables in the BSS section of the object file.
> -This inhibits the merging of tentative definitions by the linker so
> -you get a multiple-definition error if the same
> -variable is defined in more than one compilation unit.
> -Compiling with @option{-fno-common} is useful on targets for which
> -it provides better performance, or if you wish to verify that the
> -program will work on other
[PATCH v2] PR85678: Change default to -fno-common
v2: Tweak testsuite options to avoid failures
GCC currently defaults to -fcommon. As discussed in the PR, this is an ancient
C feature which is not conforming with the latest C standards. On many targets
this means global variable accesses have a codesize and performance penalty.
This applies to C code only, C++ code is not affected by -fcommon. It is about
time to change the default.
Bootstrap OK, passes testsuite on AArch64. OK for commit?
ChangeLog
2019-10-29 Wilco Dijkstra
PR85678
* common.opt (fcommon): Change init to 1.
doc/
* invoke.texi (-fcommon): Update documentation.
testsuite/
* gcc.dg/alias-15.c: Add -fcommon.
* gcc.dg/fdata-sections-1.c: Likewise.
* gcc.dg/ipa/pr77653.c: Likewise.
* gcc.dg/lto/20090729_0.c: Likewise.
* gcc.dg/lto/20111207-1_0.c: Likewise.
* gcc.dg/lto/c-compatible-types-1_0.c: Likewise.
* gcc.dg/lto/pr55525_0.c: Likewise.
* gcc.target/aarch64/sve/peel_ind_1.c: Allow ANCHOR0.
* gcc.target/aarch64/sve/peel_ind_2.c: Likewise
* gcc.target/aarch64/sve/peel_ind_3.c: Likewise
* lib/lto.exp (lto_init): Add -fcommon.
---
diff --git a/gcc/common.opt b/gcc/common.opt
index
f74b10aafc223e4961915b009c092f4876eddba4..798b6aeff3536e21c95752b5dd085f8ffef04643
100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1131,7 +1131,7 @@ Common Report Var(flag_combine_stack_adjustments)
Optimization
Looks for opportunities to reduce stack adjustments and stack references.
fcommon
-Common Report Var(flag_no_common,0)
+Common Report Var(flag_no_common,0) Init(1)
Put uninitialized globals in the common section.
fcompare-debug
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index
92fb316a368a4a36218fac6de2744c7ab6446ef5..18cfd07d4bbb4b866808db0701faf88bddbd9a94
100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -568,7 +568,7 @@ Objective-C and Objective-C++ Dialects}.
-fnon-call-exceptions -fdelete-dead-exceptions -funwind-tables @gol
-fasynchronous-unwind-tables @gol
-fno-gnu-unique @gol
--finhibit-size-directive -fno-common -fno-ident @gol
+-finhibit-size-directive -fcommon -fno-ident @gol
-fpcc-struct-return -fpic -fPIC -fpie -fPIE -fno-plt @gol
-fno-jump-tables @gol
-frecord-gcc-switches @gol
@@ -14049,35 +14049,27 @@ useful for building programs to run under WINE@.
code that is not binary compatible with code generated without that switch.
Use it to conform to a non-default application binary interface.
-@item -fno-common
-@opindex fno-common
+@item -fcommon
@opindex fcommon
+@opindex fno-common
@cindex tentative definitions
-In C code, this option controls the placement of global variables
-defined without an initializer, known as @dfn{tentative definitions}
-in the C standard. Tentative definitions are distinct from declarations
+In C code, this option controls the placement of global variables
+defined without an initializer, known as @dfn{tentative definitions}
+in the C standard. Tentative definitions are distinct from declarations
of a variable with the @code{extern} keyword, which do not allocate storage.
-Unix C compilers have traditionally allocated storage for
-uninitialized global variables in a common block. This allows the
-linker to resolve all tentative definitions of the same variable
+The default is @option{-fno-common}, which specifies that the compiler places
+uninitialized global variables in the BSS section of the object file.
+This inhibits the merging of tentative definitions by the linker so you get a
+multiple-definition error if the same variable is accidentally defined in more
+than one compilation unit.
+
+The @option{-fcommon} places uninitialized global variables in a common block.
+This allows the linker to resolve all tentative definitions of the same
variable
in different compilation units to the same object, or to a non-tentative
-definition.
-This is the behavior specified by @option{-fcommon}, and is the default for
-GCC on most targets.
-On the other hand, this behavior is not required by ISO
-C, and on some targets may carry a speed or code size penalty on
-variable references.
-
-The @option{-fno-common} option specifies that the compiler should instead
-place uninitialized global variables in the BSS section of the object file.
-This inhibits the merging of tentative definitions by the linker so
-you get a multiple-definition error if the same
-variable is defined in more than one compilation unit.
-Compiling with @option{-fno-common} is useful on targets for which
-it provides better performance, or if you wish to verify that the
-program will work on other systems that always treat uninitialized
-variable definitions this way.
+definition. This behavior does not conform to ISO C, is inconsistent with C++,
+and on many targets implies a speed and code size penalty on global variable
+references. It is mainly useful to enable legacy code to link without errors.
