[PATCH] PR85678: Change default to -fno-common

[PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

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.

OK for commit?

ChangeLog
2019-10-25  Wilco Dijkstra  <wdijkstr@arm.com>

	PR85678
	* common.opt (fcommon): Change init to 1.

doc/
	* invoke.texi (-fcommon): Update documentation.
---

diff --git a/gcc/common.opt b/gcc/common.opt
index 0195b0cb85a06dd043fd0412b42dfffddfa2495b..b0840f41a5e480f4428bd62724b0dc3d54c68c0b 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 857d9692729e503657d0d0f44f1f6252ec90d49a..5b4ff66015f5f94a5bd89e4dc3d2d53553cc091e 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
@@ -14050,35 +14050,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.
 
 @item -fno-ident
 @opindex fno-ident

Re: [PATCH] PR85678: Change default to -fno-common

[Georg-Johann Lay]

Wilco Dijkstra schrieb:
> 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.
> 
> OK for commit?

IIRC using -fno-common might lead to some testsuit fallout because
some optimizations / test cases are sensitive to -f[no-]common.
So I wonder that no adjustments to test cases are needed?

> ChangeLog
> 2019-10-25  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	PR85678
> 	* common.opt (fcommon): Change init to 1.
> 
> doc/
> 	* invoke.texi (-fcommon): Update documentation.
> ---
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 0195b0cb85a06dd043fd0412b42dfffddfa2495b..b0840f41a5e480f4428bd62724b0dc3d54c68c0b 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 857d9692729e503657d0d0f44f1f6252ec90d49a..5b4ff66015f5f94a5bd89e4dc3d2d53553cc091e 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
> @@ -14050,35 +14050,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.

IMO "uninitialized" is confusing because the variables actually
*are* initialized: with zero.  It's just that the variables don't have
explicit initializers.  Dito for "uninitialized" in the --help message.

Johann


> +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.
>  
>  @item -fno-ident
>  @opindex fno-ident
> 

Re: [PATCH] PR85678: Change default to -fno-common

[Segher Boessenkool]

On Fri, Oct 25, 2019 at 03:47:10PM +0000, Wilco Dijkstra wrote:
> 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.

Does this actually work on all older OSes (etc.) we support?  Only one
way to find out, of course :-)


Segher

Re: [PATCH] PR85678: Change default to -fno-common

[Iain Sandoe]

Georg-Johann Lay <gjl@gcc.gnu.org> wrote:

> Wilco Dijkstra schrieb:
>> 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.
>> OK for commit?
> 
> IIRC using -fno-common might lead to some testsuit fallout because
> some optimizations / test cases are sensitive to -f[no-]common.
> So I wonder that no adjustments to test cases are needed?

Two points here:

(a) I actually agree with the idea to change the default

(b) there will surely be testsuite fallout on Darwin, where common accesses are
     mandated to be indirect in the ABI, where non-weak .data accesses are not
    Thus code generated for Darwin will change for any test using variables that
    are “common” and where the test does not already force -fno-common

    I’d expect fallout to be quite large - since there are plenty of testcases with uninit
   gobals.  We might want to make a test-run and see the size of the problem, but
   preferably once the stage#1 submisison and gcc-7 release cycles are done.

thanks
Iain

>> ChangeLog
>> 2019-10-25  Wilco Dijkstra  <wdijkstr@arm.com>
>> 	PR85678
>> 	* common.opt (fcommon): Change init to 1.
>> doc/
>> 	* invoke.texi (-fcommon): Update documentation.
>> ---
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index 0195b0cb85a06dd043fd0412b42dfffddfa2495b..b0840f41a5e480f4428bd62724b0dc3d54c68c0b 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 857d9692729e503657d0d0f44f1f6252ec90d49a..5b4ff66015f5f94a5bd89e4dc3d2d53553cc091e 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
>> @@ -14050,35 +14050,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.
> 
> IMO "uninitialized" is confusing because the variables actually
> *are* initialized: with zero.  It's just that the variables don't have
> explicit initializers.  Dito for "uninitialized" in the --help message.
> 
> Johann
> 
> 
>> +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.
>>  @item -fno-ident
>> @opindex fno-ident

Re: [PATCH] PR85678: Change default to -fno-common

[Segher Boessenkool]

On Sat, Oct 26, 2019 at 12:21:15PM +0100, Iain Sandoe wrote:
> Georg-Johann Lay <gjl@gcc.gnu.org> wrote:
> > Wilco Dijkstra schrieb:
> >> 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.
> >> OK for commit?
> > 
> > IIRC using -fno-common might lead to some testsuit fallout because
> > some optimizations / test cases are sensitive to -f[no-]common.
> > So I wonder that no adjustments to test cases are needed?
> 
> Two points here:
> 
> (a) I actually agree with the idea to change the default

I think everyone does, this has been long overdue.

> (b) there will surely be testsuite fallout on Darwin, where common accesses are
>      mandated to be indirect in the ABI, where non-weak .data accesses are not
>     Thus code generated for Darwin will change for any test using variables that
>     are “common” and where the test does not already force -fno-common
> 
>     I’d expect fallout to be quite large - since there are plenty of testcases with uninit
>    gobals.  We might want to make a test-run and see the size of the problem, but
>    preferably once the stage#1 submisison and gcc-7 release cycles are done.

Yeah -- and if fallout is more than just testsuite, should it be postponed
to GCC 11?  And announced now, etc.

But to find out we probably should flip the switch soon, see what happens,
only flip it back if necessary.


Segher

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 10/25/19 9:47 AM, Wilco Dijkstra wrote:
> 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.
> 
> OK for commit?
> 
> ChangeLog
> 2019-10-25  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	PR85678
> 	* common.opt (fcommon): Change init to 1.
> 
> doc/
> 	* invoke.texi (-fcommon): Update documentation.
Has this been bootstrapped and regression tested?

jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Harald van Dijk]

On 25/10/2019 16:47, Wilco Dijkstra wrote:
> 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.

The PR references C99/C11 6.9p5, but that is not a constraint. Any 
violation merely renders the behaviour of a program undefined, so no 
diagnostic is required. To the best of my knowledge, both -fcommon and 
-fno-common are compatible with the C standard.

This is no reason not to change the default, but...

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

...can this inaccurate characterization be left out of the documentation?

Cheers,
Harald van Dijk

Re: [PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi Jeff,

> Has this been bootstrapped and regression tested?

Yes, it bootstraps OK of course. I ran regression over the weekend, there
are a few minor regressions in lto due to relying on tentative definitions
and a few latent bugs. I'd expect there will be a few similar failures on
other targets but nothing major since few testcases rely on -fcommon.
The big question is how it affects the distros.

Wilco

Re: [PATCH] PR85678: Change default to -fno-common

[Segher Boessenkool]

On Mon, Oct 28, 2019 at 03:05:58PM +0000, Wilco Dijkstra wrote:
> > Has this been bootstrapped and regression tested?
> 
> Yes, it bootstraps OK of course. I ran regression over the weekend, there
> are a few minor regressions in lto due to relying on tentative definitions
> and a few latent bugs. I'd expect there will be a few similar failures on
> other targets but nothing major since few testcases rely on -fcommon.
> The big question is how it affects the distros.

On what targets has it been bootstrapped (and tested)?


Segher

[PATCH v2] PR85678: Change default to -fno-common

[Wilco Dijkstra]

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  <wdijkstr@arm.com>

	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.
 
 @item -fno-ident
 @opindex fno-ident
diff --git a/gcc/testsuite/gcc.dg/alias-15.c b/gcc/testsuite/gcc.dg/alias-15.c
index 0a8e69b61eceef95e61f16d7874b5a4204e0594f..304ad1fbaeb7fe68ef26efd31c2ee545db6a0ffc 100644
--- a/gcc/testsuite/gcc.dg/alias-15.c
+++ b/gcc/testsuite/gcc.dg/alias-15.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-additional-options  "-O2 -fdump-ipa-cgraph" } */
+/* { dg-additional-options  "-O2 -fcommon -fdump-ipa-cgraph" } */
 
 /* RTL-level CSE shouldn't introduce LCO (for the string) into varpool */
 char *p;
diff --git a/gcc/testsuite/gcc.dg/fdata-sections-1.c b/gcc/testsuite/gcc.dg/fdata-sections-1.c
index e8a6639f32e38fdb539232288c92000f000ca79e..de5ddfc0179a588ce1bdb3d5f39a14438823c071 100644
--- a/gcc/testsuite/gcc.dg/fdata-sections-1.c
+++ b/gcc/testsuite/gcc.dg/fdata-sections-1.c
@@ -2,7 +2,7 @@
 /* Origin: Jonathan Larmour <jifl-bugzilla@jifvik.org> */
 
 /* { dg-do compile { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
-/* { dg-options "-fdata-sections" } */
+/* { dg-options "-fcommon -fdata-sections" } */
 
 int x;
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr77653.c b/gcc/testsuite/gcc.dg/ipa/pr77653.c
index f0b2b224091583bcf51d8d7444033c2042de3b7b..2fddb7eab548690a7a9d2edb3172c2b17ff9ea63 100644
--- a/gcc/testsuite/gcc.dg/ipa/pr77653.c
+++ b/gcc/testsuite/gcc.dg/ipa/pr77653.c
@@ -1,5 +1,5 @@
 /* { dg-require-alias "" } */
-/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
+/* { dg-options "-O2 -fcommon -fdump-ipa-icf-details"  } */
 
 int a, b, c, d, e, h, i, j, k, l;
 const int f;
diff --git a/gcc/testsuite/gcc.dg/lto/20090729_0.c b/gcc/testsuite/gcc.dg/lto/20090729_0.c
index 05ae74f872e28c417a8032688b3c75cd38eb6f6f..13fe62b5923b7c868373d8ca269730aa588d4992 100644
--- a/gcc/testsuite/gcc.dg/lto/20090729_0.c
+++ b/gcc/testsuite/gcc.dg/lto/20090729_0.c
@@ -1,4 +1,4 @@
-/* { dg-lto-options "-w" } */
+/* { dg-lto-options { {-fcommon -w} {-fcommon} } } */
 
 double i;
 int j;
diff --git a/gcc/testsuite/gcc.dg/lto/20111207-1_0.c b/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
index 486264066f6fcab4a248fd6293af2b6cae65caf5..5f11264af17a5a50c6d27e6eb8667bbdfce131f1 100644
--- a/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
+++ b/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
@@ -1,4 +1,4 @@
 /* { dg-lto-do run } */
-/* { dg-lto-options { { -flto } } } */
+/* { dg-lto-options { { -flto -fcommon } {-fcommon} {-fcommon} {-fcommon} } } */
 /* { dg-require-linker-plugin "" } */
 /* { dg-extra-ld-options "-fuse-linker-plugin" } */
diff --git a/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c b/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
index 376da00599d04a738b8b5d6c6d3625d915cae83b..45b03735a6befd47b5b24cab5c62ecc8792aed4c 100644
--- a/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
+++ b/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
@@ -1,5 +1,5 @@
 /* { dg-lto-do run } */
-/* { dg-lto-options "-O3" } */
+/* { dg-lto-options { {-O3 -fcommon} {-fcommon} } } */
 
 /* By C standard Each enumerated type shall be compatible with char, a  signed
    integer, type, or an unsigned integer type. The choice of type is
diff --git a/gcc/testsuite/gcc.dg/lto/pr55525_0.c b/gcc/testsuite/gcc.dg/lto/pr55525_0.c
index 7faaf806a75836be4eb9a3ede7559dd4d4151185..d8d16d11d32d3918bc47f4b41c7e7dabe255fd39 100644
--- a/gcc/testsuite/gcc.dg/lto/pr55525_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr55525_0.c
@@ -1,5 +1,5 @@
 /* { dg-lto-do link } */
-/* { dg-lto-options { { -flto -w } } } */
+/* { dg-lto-options { { -fcommon -flto -w } } } */
 
 char s[sizeof (char *)];
 int main(void)
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
index 156d04ae5ca222ddea3e12a3b785050c6113a548..e9afc2047e49e4382fd2cc5b150de51be89e2e3d 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
@@ -21,7 +21,7 @@ foo (void)
 }
 
 /* We should operate on aligned vectors.  */
-/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
+/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
 /* We should use an induction that starts at -5, with only the last
    7 elements of the first iteration being active.  */
 /* { dg-final { scan-assembler {\tindex\tz[0-9]+\.s, #-5, #5\n} } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
index e792cdf2cad297e7044fdecd576343c9ac212078..6bd7fc73fb03d339b17c62d1ad818c9bdb351e1b 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
@@ -17,7 +17,7 @@ foo (void)
 }
 
 /* We should operate on aligned vectors.  */
-/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
+/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
 /* We should unroll the loop three times.  */
 /* { dg-final { scan-assembler-times "\tst1w\t" 3 } } */
 /* { dg-final { scan-assembler {\tptrue\t(p[0-9]+)\.s, vl7\n.*\teor\tp[0-7]\.b, (p[0-7])/z, (\1\.b, \2\.b|\2\.b, \1\.b)\n} } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
index 441589eef600df0d1b264780774a9bdc4deb975e..3adddf3f4049a73bae99ab3468f83c535ef55170 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
@@ -17,5 +17,5 @@ foo (int start)
 }
 
 /* We should operate on aligned vectors.  */
-/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
+/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
 /* { dg-final { scan-assembler {\tubfx\t} } } */
diff --git a/gcc/testsuite/lib/lto.exp b/gcc/testsuite/lib/lto.exp
index 25c934731df0bb94f2e61561142f75438c4294fa..e931f946ad2537a53a45e1855513fb4d1682351c 100644
--- a/gcc/testsuite/lib/lto.exp
+++ b/gcc/testsuite/lib/lto.exp
@@ -191,21 +191,21 @@ proc lto_init { args } {
     if ![info exists LTO_OPTIONS] {
         if [check_linker_plugin_available] {
 	  set LTO_OPTIONS [list	\
-	      {-O0 -flto -flto-partition=none -fuse-linker-plugin} \
-	      {-O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects } \
-	      {-O0 -flto -flto-partition=1to1 -fno-use-linker-plugin } \
-	      {-O2 -flto -flto-partition=1to1 -fno-use-linker-plugin } \
-	      {-O0 -flto -fuse-linker-plugin -fno-fat-lto-objects }	\
-	      {-O2 -flto -fuse-linker-plugin}	\
+	      {-O0 -flto -fcommon -flto-partition=none -fuse-linker-plugin} \
+	      {-O2 -flto -fcommon -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects } \
+	      {-O0 -flto -fcommon -flto-partition=1to1 -fno-use-linker-plugin } \
+	      {-O2 -flto -fcommon -flto-partition=1to1 -fno-use-linker-plugin } \
+	      {-O0 -flto -fcommon -fuse-linker-plugin -fno-fat-lto-objects }	\
+	      {-O2 -flto -fcommon -fuse-linker-plugin}	\
 	  ]
  	} else {
 	  set LTO_OPTIONS [list	\
-	      {-O0 -flto -flto-partition=none } \
-	      {-O2 -flto -flto-partition=none } \
-	      {-O0 -flto -flto-partition=1to1 } \
-	      {-O2 -flto -flto-partition=1to1 } \
-	      {-O0 -flto }	\
-	      {-O2 -flto}		\
+	      {-O0 -flto -fcommon -flto-partition=none } \
+	      {-O2 -flto -fcommon -flto-partition=none } \
+	      {-O0 -flto -fcommon -flto-partition=1to1 } \
+	      {-O2 -flto -fcommon -flto-partition=1to1 } \
+	      {-O0 -flto -fcommon}	\
+	      {-O2 -flto -fcommon}		\
 	  ]
 	}
     }

Re: [PATCH v2] PR85678: Change default to -fno-common

[Richard Biener]

On Tue, Oct 29, 2019 at 1:33 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> 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  <wdijkstr@arm.com>
>
>         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.
>
>  @item -fno-ident
>  @opindex fno-ident
> diff --git a/gcc/testsuite/gcc.dg/alias-15.c b/gcc/testsuite/gcc.dg/alias-15.c
> index 0a8e69b61eceef95e61f16d7874b5a4204e0594f..304ad1fbaeb7fe68ef26efd31c2ee545db6a0ffc 100644
> --- a/gcc/testsuite/gcc.dg/alias-15.c
> +++ b/gcc/testsuite/gcc.dg/alias-15.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-additional-options  "-O2 -fdump-ipa-cgraph" } */
> +/* { dg-additional-options  "-O2 -fcommon -fdump-ipa-cgraph" } */
>
>  /* RTL-level CSE shouldn't introduce LCO (for the string) into varpool */
>  char *p;
> diff --git a/gcc/testsuite/gcc.dg/fdata-sections-1.c b/gcc/testsuite/gcc.dg/fdata-sections-1.c
> index e8a6639f32e38fdb539232288c92000f000ca79e..de5ddfc0179a588ce1bdb3d5f39a14438823c071 100644
> --- a/gcc/testsuite/gcc.dg/fdata-sections-1.c
> +++ b/gcc/testsuite/gcc.dg/fdata-sections-1.c
> @@ -2,7 +2,7 @@
>  /* Origin: Jonathan Larmour <jifl-bugzilla@jifvik.org> */
>
>  /* { dg-do compile { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
> -/* { dg-options "-fdata-sections" } */
> +/* { dg-options "-fcommon -fdata-sections" } */
>
>  int x;
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr77653.c b/gcc/testsuite/gcc.dg/ipa/pr77653.c
> index f0b2b224091583bcf51d8d7444033c2042de3b7b..2fddb7eab548690a7a9d2edb3172c2b17ff9ea63 100644
> --- a/gcc/testsuite/gcc.dg/ipa/pr77653.c
> +++ b/gcc/testsuite/gcc.dg/ipa/pr77653.c
> @@ -1,5 +1,5 @@
>  /* { dg-require-alias "" } */
> -/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
> +/* { dg-options "-O2 -fcommon -fdump-ipa-icf-details"  } */
>
>  int a, b, c, d, e, h, i, j, k, l;
>  const int f;
> diff --git a/gcc/testsuite/gcc.dg/lto/20090729_0.c b/gcc/testsuite/gcc.dg/lto/20090729_0.c
> index 05ae74f872e28c417a8032688b3c75cd38eb6f6f..13fe62b5923b7c868373d8ca269730aa588d4992 100644
> --- a/gcc/testsuite/gcc.dg/lto/20090729_0.c
> +++ b/gcc/testsuite/gcc.dg/lto/20090729_0.c
> @@ -1,4 +1,4 @@
> -/* { dg-lto-options "-w" } */
> +/* { dg-lto-options { {-fcommon -w} {-fcommon} } } */
>
>  double i;
>  int j;
> diff --git a/gcc/testsuite/gcc.dg/lto/20111207-1_0.c b/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
> index 486264066f6fcab4a248fd6293af2b6cae65caf5..5f11264af17a5a50c6d27e6eb8667bbdfce131f1 100644
> --- a/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
> +++ b/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
> @@ -1,4 +1,4 @@
>  /* { dg-lto-do run } */
> -/* { dg-lto-options { { -flto } } } */
> +/* { dg-lto-options { { -flto -fcommon } {-fcommon} {-fcommon} {-fcommon} } } */
>  /* { dg-require-linker-plugin "" } */
>  /* { dg-extra-ld-options "-fuse-linker-plugin" } */
> diff --git a/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c b/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
> index 376da00599d04a738b8b5d6c6d3625d915cae83b..45b03735a6befd47b5b24cab5c62ecc8792aed4c 100644
> --- a/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
> +++ b/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
> @@ -1,5 +1,5 @@
>  /* { dg-lto-do run } */
> -/* { dg-lto-options "-O3" } */
> +/* { dg-lto-options { {-O3 -fcommon} {-fcommon} } } */
>
>  /* By C standard Each enumerated type shall be compatible with char, a  signed
>     integer, type, or an unsigned integer type. The choice of type is
> diff --git a/gcc/testsuite/gcc.dg/lto/pr55525_0.c b/gcc/testsuite/gcc.dg/lto/pr55525_0.c
> index 7faaf806a75836be4eb9a3ede7559dd4d4151185..d8d16d11d32d3918bc47f4b41c7e7dabe255fd39 100644
> --- a/gcc/testsuite/gcc.dg/lto/pr55525_0.c
> +++ b/gcc/testsuite/gcc.dg/lto/pr55525_0.c
> @@ -1,5 +1,5 @@
>  /* { dg-lto-do link } */
> -/* { dg-lto-options { { -flto -w } } } */
> +/* { dg-lto-options { { -fcommon -flto -w } } } */
>
>  char s[sizeof (char *)];
>  int main(void)
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> index 156d04ae5ca222ddea3e12a3b785050c6113a548..e9afc2047e49e4382fd2cc5b150de51be89e2e3d 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> @@ -21,7 +21,7 @@ foo (void)
>  }
>
>  /* We should operate on aligned vectors.  */
> -/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
> +/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
>  /* We should use an induction that starts at -5, with only the last
>     7 elements of the first iteration being active.  */
>  /* { dg-final { scan-assembler {\tindex\tz[0-9]+\.s, #-5, #5\n} } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> index e792cdf2cad297e7044fdecd576343c9ac212078..6bd7fc73fb03d339b17c62d1ad818c9bdb351e1b 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> @@ -17,7 +17,7 @@ foo (void)
>  }
>
>  /* We should operate on aligned vectors.  */
> -/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
> +/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
>  /* We should unroll the loop three times.  */
>  /* { dg-final { scan-assembler-times "\tst1w\t" 3 } } */
>  /* { dg-final { scan-assembler {\tptrue\t(p[0-9]+)\.s, vl7\n.*\teor\tp[0-7]\.b, (p[0-7])/z, (\1\.b, \2\.b|\2\.b, \1\.b)\n} } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> index 441589eef600df0d1b264780774a9bdc4deb975e..3adddf3f4049a73bae99ab3468f83c535ef55170 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> @@ -17,5 +17,5 @@ foo (int start)
>  }
>
>  /* We should operate on aligned vectors.  */
> -/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
> +/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
>  /* { dg-final { scan-assembler {\tubfx\t} } } */
> diff --git a/gcc/testsuite/lib/lto.exp b/gcc/testsuite/lib/lto.exp
> index 25c934731df0bb94f2e61561142f75438c4294fa..e931f946ad2537a53a45e1855513fb4d1682351c 100644
> --- a/gcc/testsuite/lib/lto.exp
> +++ b/gcc/testsuite/lib/lto.exp
> @@ -191,21 +191,21 @@ proc lto_init { args } {
>      if ![info exists LTO_OPTIONS] {
>          if [check_linker_plugin_available] {
>           set LTO_OPTIONS [list \
> -             {-O0 -flto -flto-partition=none -fuse-linker-plugin} \
> -             {-O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects } \
> -             {-O0 -flto -flto-partition=1to1 -fno-use-linker-plugin } \
> -             {-O2 -flto -flto-partition=1to1 -fno-use-linker-plugin } \
> -             {-O0 -flto -fuse-linker-plugin -fno-fat-lto-objects }     \
> -             {-O2 -flto -fuse-linker-plugin}   \
> +             {-O0 -flto -fcommon -flto-partition=none -fuse-linker-plugin} \
> +             {-O2 -flto -fcommon -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects } \
> +             {-O0 -flto -fcommon -flto-partition=1to1 -fno-use-linker-plugin } \
> +             {-O2 -flto -fcommon -flto-partition=1to1 -fno-use-linker-plugin } \
> +             {-O0 -flto -fcommon -fuse-linker-plugin -fno-fat-lto-objects }    \
> +             {-O2 -flto -fcommon -fuse-linker-plugin}  \
>           ]
>         } else {
>           set LTO_OPTIONS [list \
> -             {-O0 -flto -flto-partition=none } \
> -             {-O2 -flto -flto-partition=none } \
> -             {-O0 -flto -flto-partition=1to1 } \
> -             {-O2 -flto -flto-partition=1to1 } \
> -             {-O0 -flto }      \
> -             {-O2 -flto}               \
> +             {-O0 -flto -fcommon -flto-partition=none } \
> +             {-O2 -flto -fcommon -flto-partition=none } \
> +             {-O0 -flto -fcommon -flto-partition=1to1 } \
> +             {-O2 -flto -fcommon -flto-partition=1to1 } \
> +             {-O0 -flto -fcommon}      \
> +             {-O2 -flto -fcommon}              \
>           ]
>         }
>      }

Re: [PATCH v2] PR85678: Change default to -fno-common

[Wilco Dijkstra]

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

[Richard Biener]

On Wed, Oct 30, 2019 at 3:33 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> 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

[Wilco Dijkstra]

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

[Richard Biener]

On Mon, Nov 4, 2019 at 3:39 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> 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 v3] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi Richard,

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

The odr-6 test mixes C and C++ using .C/.c extensions. But like you suggest, dg-options
works on the 2nd file, and with that odr-6 and pr88077 tests pass without needing changes
in lto.exp. I needed to change one of the types since default object alignment is different
between -fcommon and -fno-common and that causes linker failures when linking objects
built with different -fcommon settings. I also checked regress on x64, there was one minor
failure because of the alignment change, which is easily fixed.

So here is v3:

[PATCH v3] PR85678: Change default to -fno-common

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.

Passes bootstrap and regress on AArch64 and x64. OK for commit?

ChangeLog
2019-11-05  Wilco Dijkstra  <wdijkstr@arm.com>

	PR85678
	* common.opt (fcommon): Change init to 1.

doc/
	* invoke.texi (-fcommon): Update documentation.

testsuite/
	* g++.dg/lto/odr-6_1.c: Add -fcommon.
	* gcc.dg/alias-15.c: Likewise.
	* 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.dg/lto/pr88077_0.c: Use long to avoid alignment warning.
	* gcc.dg/lto/pr88077_1.c: Add -fcommon.
	* 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.
	* gcc.target/i386/volatile-bitfields-2.c: Allow movl or movq.

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.
 
 @item -fno-ident
 @opindex fno-ident
diff --git a/gcc/testsuite/g++.dg/lto/odr-6_1.c b/gcc/testsuite/g++.dg/lto/odr-6_1.c
index ee4bff44659d9ff22cdb92ca0031cfe86877ef15..8328bf59a4cb60173b86ea4153520d11d15a1b1f 100644
--- a/gcc/testsuite/g++.dg/lto/odr-6_1.c
+++ b/gcc/testsuite/g++.dg/lto/odr-6_1.c
@@ -1,3 +1,4 @@
+/* { dg-options {-fcommon} } */
 struct {} admbaserest_; // { dg-lto-message "type of " 2 }
 
 
diff --git a/gcc/testsuite/gcc.dg/alias-15.c b/gcc/testsuite/gcc.dg/alias-15.c
index 0a8e69b61eceef95e61f16d7874b5a4204e0594f..304ad1fbaeb7fe68ef26efd31c2ee545db6a0ffc 100644
--- a/gcc/testsuite/gcc.dg/alias-15.c
+++ b/gcc/testsuite/gcc.dg/alias-15.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-additional-options  "-O2 -fdump-ipa-cgraph" } */
+/* { dg-additional-options  "-O2 -fcommon -fdump-ipa-cgraph" } */
 
 /* RTL-level CSE shouldn't introduce LCO (for the string) into varpool */
 char *p;
diff --git a/gcc/testsuite/gcc.dg/fdata-sections-1.c b/gcc/testsuite/gcc.dg/fdata-sections-1.c
index e8a6639f32e38fdb539232288c92000f000ca79e..de5ddfc0179a588ce1bdb3d5f39a14438823c071 100644
--- a/gcc/testsuite/gcc.dg/fdata-sections-1.c
+++ b/gcc/testsuite/gcc.dg/fdata-sections-1.c
@@ -2,7 +2,7 @@
 /* Origin: Jonathan Larmour <jifl-bugzilla@jifvik.org> */
 
 /* { dg-do compile { target *-*-linux* *-*-gnu* *-*-uclinux* } } */
-/* { dg-options "-fdata-sections" } */
+/* { dg-options "-fcommon -fdata-sections" } */
 
 int x;
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr77653.c b/gcc/testsuite/gcc.dg/ipa/pr77653.c
index f0b2b224091583bcf51d8d7444033c2042de3b7b..2fddb7eab548690a7a9d2edb3172c2b17ff9ea63 100644
--- a/gcc/testsuite/gcc.dg/ipa/pr77653.c
+++ b/gcc/testsuite/gcc.dg/ipa/pr77653.c
@@ -1,5 +1,5 @@
 /* { dg-require-alias "" } */
-/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
+/* { dg-options "-O2 -fcommon -fdump-ipa-icf-details"  } */
 
 int a, b, c, d, e, h, i, j, k, l;
 const int f;
diff --git a/gcc/testsuite/gcc.dg/lto/20090729_0.c b/gcc/testsuite/gcc.dg/lto/20090729_0.c
index 05ae74f872e28c417a8032688b3c75cd38eb6f6f..13fe62b5923b7c868373d8ca269730aa588d4992 100644
--- a/gcc/testsuite/gcc.dg/lto/20090729_0.c
+++ b/gcc/testsuite/gcc.dg/lto/20090729_0.c
@@ -1,4 +1,4 @@
-/* { dg-lto-options "-w" } */
+/* { dg-lto-options { {-fcommon -w} {-fcommon} } } */
 
 double i;
 int j;
diff --git a/gcc/testsuite/gcc.dg/lto/20111207-1_0.c b/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
index 486264066f6fcab4a248fd6293af2b6cae65caf5..5f11264af17a5a50c6d27e6eb8667bbdfce131f1 100644
--- a/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
+++ b/gcc/testsuite/gcc.dg/lto/20111207-1_0.c
@@ -1,4 +1,4 @@
 /* { dg-lto-do run } */
-/* { dg-lto-options { { -flto } } } */
+/* { dg-lto-options { { -flto -fcommon } {-fcommon} {-fcommon} {-fcommon} } } */
 /* { dg-require-linker-plugin "" } */
 /* { dg-extra-ld-options "-fuse-linker-plugin" } */
diff --git a/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c b/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
index 376da00599d04a738b8b5d6c6d3625d915cae83b..45b03735a6befd47b5b24cab5c62ecc8792aed4c 100644
--- a/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
+++ b/gcc/testsuite/gcc.dg/lto/c-compatible-types-1_0.c
@@ -1,5 +1,5 @@
 /* { dg-lto-do run } */
-/* { dg-lto-options "-O3" } */
+/* { dg-lto-options { {-O3 -fcommon} {-fcommon} } } */
 
 /* By C standard Each enumerated type shall be compatible with char, a  signed
    integer, type, or an unsigned integer type. The choice of type is
diff --git a/gcc/testsuite/gcc.dg/lto/pr55525_0.c b/gcc/testsuite/gcc.dg/lto/pr55525_0.c
index 7faaf806a75836be4eb9a3ede7559dd4d4151185..d8d16d11d32d3918bc47f4b41c7e7dabe255fd39 100644
--- a/gcc/testsuite/gcc.dg/lto/pr55525_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr55525_0.c
@@ -1,5 +1,5 @@
 /* { dg-lto-do link } */
-/* { dg-lto-options { { -flto -w } } } */
+/* { dg-lto-options { { -fcommon -flto -w } } } */
 
 char s[sizeof (char *)];
 int main(void)
diff --git a/gcc/testsuite/gcc.dg/lto/pr88077_0.c b/gcc/testsuite/gcc.dg/lto/pr88077_0.c
index 9e464b6ad4a246d8b5b813626a4b858629db299a..924fe9fc3f01f64c2712480c5507693e66e7b5f8 100644
--- a/gcc/testsuite/gcc.dg/lto/pr88077_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr88077_0.c
@@ -1,3 +1,3 @@
 /* { dg-lto-do link } */
 
-int HeaderStr;
+long HeaderStr;
diff --git a/gcc/testsuite/gcc.dg/lto/pr88077_1.c b/gcc/testsuite/gcc.dg/lto/pr88077_1.c
index fd3de3e77a6215e4ab263cc277b408998fe6e505..43d783f2cc7dd4ad1cb34a8ea314bb062410539d 100644
--- a/gcc/testsuite/gcc.dg/lto/pr88077_1.c
+++ b/gcc/testsuite/gcc.dg/lto/pr88077_1.c
@@ -1,3 +1,5 @@
+/* { dg-options {-fcommon} } */
+
 char HeaderStr[1];
 
 int main()
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
index 156d04ae5ca222ddea3e12a3b785050c6113a548..e9afc2047e49e4382fd2cc5b150de51be89e2e3d 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
@@ -21,7 +21,7 @@ foo (void)
 }
 
 /* We should operate on aligned vectors.  */
-/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
+/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
 /* We should use an induction that starts at -5, with only the last
    7 elements of the first iteration being active.  */
 /* { dg-final { scan-assembler {\tindex\tz[0-9]+\.s, #-5, #5\n} } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
index e792cdf2cad297e7044fdecd576343c9ac212078..6bd7fc73fb03d339b17c62d1ad818c9bdb351e1b 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
@@ -17,7 +17,7 @@ foo (void)
 }
 
 /* We should operate on aligned vectors.  */
-/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
+/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
 /* We should unroll the loop three times.  */
 /* { dg-final { scan-assembler-times "\tst1w\t" 3 } } */
 /* { dg-final { scan-assembler {\tptrue\t(p[0-9]+)\.s, vl7\n.*\teor\tp[0-7]\.b, (p[0-7])/z, (\1\.b, \2\.b|\2\.b, \1\.b)\n} } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
index 441589eef600df0d1b264780774a9bdc4deb975e..3adddf3f4049a73bae99ab3468f83c535ef55170 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
@@ -17,5 +17,5 @@ foo (int start)
 }
 
 /* We should operate on aligned vectors.  */
-/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
+/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, (x|\.LANCHOR0)\n} } } */
 /* { dg-final { scan-assembler {\tubfx\t} } } */
diff --git a/gcc/testsuite/gcc.target/i386/volatile-bitfields-2.c b/gcc/testsuite/gcc.target/i386/volatile-bitfields-2.c
index 302625a199b5b79d950592dcc869895753bf4884..d84363315a85131f8d6b2df6527e4b2ef6ffc478 100644
--- a/gcc/testsuite/gcc.target/i386/volatile-bitfields-2.c
+++ b/gcc/testsuite/gcc.target/i386/volatile-bitfields-2.c
@@ -14,4 +14,4 @@ int foo ()
   return bits.b;
 }
 
-/* { dg-final { scan-assembler "movl.*bits" } } */
+/* { dg-final { scan-assembler "mov(q|l).*bits" } } */

Re: [PATCH v3] PR85678: Change default to -fno-common

[Jeff Law]

On 11/5/19 10:17 AM, Wilco Dijkstra wrote:
> Hi Richard,
> 
>> 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" }
> 
> The odr-6 test mixes C and C++ using .C/.c extensions. But like you suggest, dg-options
> works on the 2nd file, and with that odr-6 and pr88077 tests pass without needing changes
> in lto.exp. I needed to change one of the types since default object alignment is different
> between -fcommon and -fno-common and that causes linker failures when linking objects
> built with different -fcommon settings. I also checked regress on x64, there was one minor
> failure because of the alignment change, which is easily fixed.
> 
> So here is v3:
> 
> [PATCH v3] PR85678: Change default to -fno-common
> 
> 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.
> 
> Passes bootstrap and regress on AArch64 and x64. OK for commit?
> 
> ChangeLog
> 2019-11-05  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	PR85678
> 	* common.opt (fcommon): Change init to 1.
> 
> doc/
> 	* invoke.texi (-fcommon): Update documentation.
> 
> testsuite/
> 	* g++.dg/lto/odr-6_1.c: Add -fcommon.
> 	* gcc.dg/alias-15.c: Likewise.
> 	* 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.dg/lto/pr88077_0.c: Use long to avoid alignment warning.
> 	* gcc.dg/lto/pr88077_1.c: Add -fcommon.
> 	* 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.
> 	* gcc.target/i386/volatile-bitfields-2.c: Allow movl or movq.
I'd say let's go for it now.  That gives us plenty of time to work
through any problems.  I think it deserves a mention in the release notes.

jeff

[PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Jakub Jelinek]

On Tue, Nov 05, 2019 at 05:17:10PM +0000, Wilco Dijkstra wrote:
> Passes bootstrap and regress on AArch64 and x64. OK for commit?

This broke bootstrap on x86_64-linux as well as i686-linux (guess all
targets that go supports).
The following patch fixes it for me, though not sure which *.c file is best
and what location in there for the definition.
With this bootstrap succeeded on both x86_64-linux and i686-linux, regtest
is still pending, but without it it just failed to link libgo.

--- libgo/runtime/runtime.h.jj	2019-09-06 23:00:17.755294355 +0200
+++ libgo/runtime/runtime.h	2019-11-21 00:32:51.146992951 +0100
@@ -475,7 +475,7 @@ bool scanstackwithmap(void*)
 bool doscanstack(G*, void*)
   __asm__("runtime.doscanstack");
 
-bool runtime_usestackmaps;
+extern bool runtime_usestackmaps;
 
 bool probestackmaps(void)
   __asm__("runtime.probestackmaps");
--- libgo/runtime/proc.c.jj	2019-08-31 13:26:29.646735239 +0200
+++ libgo/runtime/proc.c	2019-11-21 00:34:23.509610234 +0100
@@ -676,6 +676,8 @@ makeGContext(G* gp, byte* sp, uintptr sp
 	__go_makecontext(uc, kickoff, sp, (size_t)spsize);
 }
 
+bool runtime_usestackmaps;
+
 // The goroutine g is about to enter a system call.
 // Record that it's not using the cpu anymore.
 // This is called only from the go syscall library and cgocall,

	Jakub

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Rainer Orth]

Hi Jakub,

> On Tue, Nov 05, 2019 at 05:17:10PM +0000, Wilco Dijkstra wrote:
>> Passes bootstrap and regress on AArch64 and x64. OK for commit?
>
> This broke bootstrap on x86_64-linux as well as i686-linux (guess all
> targets that go supports).

indeed: just saw it on Solaris with the native ld.

> The following patch fixes it for me, though not sure which *.c file is best
> and what location in there for the definition.

I've placed it in runtime/go-unwind.c: probestackmaps is defined there
which is declared close to runtime_usestackmaps.

> With this bootstrap succeeded on both x86_64-linux and i686-linux, regtest
> is still pending, but without it it just failed to link libgo.

Same on sparc-sun-solaris2.11 and i386-pc-solaris2.11.

There where quite a number of non-Go regressions all over the place.
Many are like this:

FAIL: gcc.c-torture/execute/complex-6.c   -O0  (test for excess errors)

ld: warning: symbol 'err' has differing types:
        (file /var/tmp//ccWQCyMc.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//ccWQCyMc.o definition taken

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Jakub Jelinek]

On Thu, Nov 21, 2019 at 01:41:47AM +0100, Rainer Orth wrote:
> Same on sparc-sun-solaris2.11 and i386-pc-solaris2.11.
> 
> There where quite a number of non-Go regressions all over the place.
> Many are like this:
> 
> FAIL: gcc.c-torture/execute/complex-6.c   -O0  (test for excess errors)
> 
> ld: warning: symbol 'err' has differing types:
>         (file /var/tmp//ccWQCyMc.o type=OBJT; file /lib/libc.so type=FUNC);
>         /var/tmp//ccWQCyMc.o definition taken

On i686-linux, I see just:
+FAIL: gcc.target/i386/memcpy-strategy-1.c scan-assembler-times movdqa 4
+FAIL: gcc.target/i386/memcpy-strategy-2.c scan-assembler-times movdqa 4
+FAIL: gcc.target/i386/memcpy-vector_loop-1.c scan-assembler-times movdqa 4
+FAIL: gcc.target/i386/pr69052.c scan-assembler-not leal[ \\t]ind@GOTOFF\\\\(%[^,]*\\\\), %
+FAIL: gfortran.dg/global_vars_f90_init.f90   -O0  (test for excess errors)
+UNRESOLVED: gfortran.dg/global_vars_f90_init.f90   -O0  compilation failed to produce executable
+FAIL: gfortran.dg/global_vars_f90_init.f90   -O1  (test for excess errors)
+UNRESOLVED: gfortran.dg/global_vars_f90_init.f90   -O1  compilation failed to produce executable
+FAIL: gfortran.dg/global_vars_f90_init.f90   -O2  (test for excess errors)
+UNRESOLVED: gfortran.dg/global_vars_f90_init.f90   -O2  compilation failed to produce executable
+FAIL: gfortran.dg/global_vars_f90_init.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess errors)
+UNRESOLVED: gfortran.dg/global_vars_f90_init.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  compilation failed to produce executable
+FAIL: gfortran.dg/global_vars_f90_init.f90   -O3 -g  (test for excess errors)
+UNRESOLVED: gfortran.dg/global_vars_f90_init.f90   -O3 -g  compilation failed to produce executable
+FAIL: gfortran.dg/global_vars_f90_init.f90   -Os  (test for excess errors)
+UNRESOLVED: gfortran.dg/global_vars_f90_init.f90   -Os  compilation failed to produce executable

	Jakub

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Ian Lance Taylor]

On Wed, Nov 20, 2019 at 4:18 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Nov 05, 2019 at 05:17:10PM +0000, Wilco Dijkstra wrote:
> > Passes bootstrap and regress on AArch64 and x64. OK for commit?
>
> This broke bootstrap on x86_64-linux as well as i686-linux (guess all
> targets that go supports).
> The following patch fixes it for me, though not sure which *.c file is best
> and what location in there for the definition.
> With this bootstrap succeeded on both x86_64-linux and i686-linux, regtest
> is still pending, but without it it just failed to link libgo.

I just committed a fix for this.  I put the variable in
libgo/runtime/stack.c.  Sorry about the difficulties.

Ian

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 7442 bytes --]

Hi Jakub,

> On Thu, Nov 21, 2019 at 01:41:47AM +0100, Rainer Orth wrote:
>> Same on sparc-sun-solaris2.11 and i386-pc-solaris2.11.
>> 
>> There where quite a number of non-Go regressions all over the place.
>> Many are like this:
>> 
>> FAIL: gcc.c-torture/execute/complex-6.c   -O0  (test for excess errors)
>> 
>> ld: warning: symbol 'err' has differing types:
>>         (file /var/tmp//ccWQCyMc.o type=OBJT; file /lib/libc.so type=FUNC);
>>         /var/tmp//ccWQCyMc.o definition taken
>
> On i686-linux, I see just:
> +FAIL: gcc.target/i386/memcpy-strategy-1.c scan-assembler-times movdqa 4
> +FAIL: gcc.target/i386/memcpy-strategy-2.c scan-assembler-times movdqa 4
> +FAIL: gcc.target/i386/memcpy-vector_loop-1.c scan-assembler-times movdqa 4
> +FAIL: gcc.target/i386/pr69052.c scan-assembler-not leal[
> \\t]ind@GOTOFF\\\\(%[^,]*\\\\), %
> +FAIL: gfortran.dg/global_vars_f90_init.f90   -O0  (test for excess errors)
> +UNRESOLVED: gfortran.dg/global_vars_f90_init.f90 -O0 compilation failed to
> produce executable
> +FAIL: gfortran.dg/global_vars_f90_init.f90   -O1  (test for excess errors)
> +UNRESOLVED: gfortran.dg/global_vars_f90_init.f90 -O1 compilation failed to
> produce executable
> +FAIL: gfortran.dg/global_vars_f90_init.f90   -O2  (test for excess errors)
> +UNRESOLVED: gfortran.dg/global_vars_f90_init.f90 -O2 compilation failed to
> produce executable
> +FAIL: gfortran.dg/global_vars_f90_init.f90 -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess
> errors)
> +UNRESOLVED: gfortran.dg/global_vars_f90_init.f90 -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions compilation failed
> to produce executable
> +FAIL: gfortran.dg/global_vars_f90_init.f90   -O3 -g  (test for excess errors)
> +UNRESOLVED: gfortran.dg/global_vars_f90_init.f90 -O3 -g compilation failed
> to produce executable
> +FAIL: gfortran.dg/global_vars_f90_init.f90   -Os  (test for excess errors)
> +UNRESOLVED: gfortran.dg/global_vars_f90_init.f90 -Os compilation failed to
> produce executable

I'm seeing those, too, plus the following that are apparently only
diagnosed by Solaris ld:

+FAIL: gcc.c-torture/execute/20030913-1.c   -O0  (test for excess errors)
+FAIL: gcc.c-torture/execute/20030913-1.c   -O1  (test for excess errors)
+FAIL: gcc.c-torture/execute/20030913-1.c   -O2  (test for excess errors)
+FAIL: gcc.c-torture/execute/20030913-1.c   -O2 -flto  (test for excess errors)
+FAIL: gcc.c-torture/execute/20030913-1.c   -O2 -flto -flto-partition=none  (test for excess errors)
+FAIL: gcc.c-torture/execute/20030913-1.c   -O3 -g  (test for excess errors)
+FAIL: gcc.c-torture/execute/20030913-1.c   -Os  (test for excess errors)

Excess errors:
ld: warning: symbol 'glob' has differing types:
        (file /var/tmp//ccdl_86b.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//ccdl_86b.o definition taken

+FAIL: gcc.c-torture/execute/960218-1.c   -O0  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -O1  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -O2  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -O2 -flto  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -O2 -flto -flto-partition=none  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -O3 -g  (test for excess errors)
+FAIL: gcc.c-torture/execute/960218-1.c   -Os  (test for excess errors)

Excess errors:
ld: warning: symbol 'glob' has differing types:
        (file /var/tmp//cc0ib2Zb.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//cc0ib2Zb.o definition taken

+FAIL: gcc.c-torture/execute/complex-6.c   -O0  (test for excess errors)
+FAIL: gcc.c-torture/execute/complex-6.c   -O1  (test for excess errors)
+FAIL: gcc.c-torture/execute/complex-6.c   -O2  (test for excess errors)
+FAIL: gcc.c-torture/execute/complex-6.c   -O2 -flto  (test for excess errors)
+FAIL: gcc.c-torture/execute/complex-6.c   -O2 -flto -flto-partition=none  (test for excess errors)
+FAIL: gcc.c-torture/execute/complex-6.c   -O3 -g  (test for excess errors)
+FAIL: gcc.c-torture/execute/complex-6.c   -Os  (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//ccz5Kj5d.o type=OBJT; file /lib/sparcv9/libc.so type=FUNC);
        /var/tmp//ccz5Kj5d.o definition taken

+FAIL: gcc.dg/torture/ssa-pta-fn-1.c   -O1  (test for excess errors)
+FAIL: gcc.dg/torture/ssa-pta-fn-1.c   -O2  (test for excess errors)
+FAIL: gcc.dg/torture/ssa-pta-fn-1.c   -O2 -flto  (test for excess errors)
+FAIL: gcc.dg/torture/ssa-pta-fn-1.c   -O2 -flto -flto-partition=none  (test for excess errors)
+FAIL: gcc.dg/torture/ssa-pta-fn-1.c   -O3 -g  (test for excess errors)
+FAIL: gcc.dg/torture/ssa-pta-fn-1.c   -Os  (test for excess errors)

Excess errors:
ld: warning: symbol 'glob' has differing types:
        (file /var/tmp//ccn1WXac.o type=OBJT; file /lib/sparcv9/libc.so type=FUNC);
        /var/tmp//ccn1WXac.o definition taken

+FAIL: libgomp.c/pr39591-1.c (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//cc_ExIUc.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//cc_ExIUc.o definition taken

+FAIL: libgomp.c/pr39591-2.c (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//cc4vqObc.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//cc4vqObc.o definition taken

+FAIL: libgomp.c/pr39591-3.c (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//ccivrjXd.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//ccivrjXd.o definition taken

+FAIL: libgomp.c/private-1.c (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//cch58Hvd.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//cch58Hvd.o definition taken

+FAIL: libgomp.c/task-1.c (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//ccBb4fsb.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//ccBb4fsb.o definition taken

+FAIL: libgomp.c/task-5.c (test for excess errors)

Excess errors:
ld: warning: symbol 'err' has differing types:
        (file /var/tmp//ccvQhrHd.o type=OBJT; file /lib/libc.so type=FUNC);
        /var/tmp//ccvQhrHd.o definition taken

Fixed as follows, tested on i386-pc-solaris2.11 and
sparc-sun-solaris2.11.  Ok for mainline?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2019-11-21  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	gcc/testsuite:
	* gcc.c-torture/execute/20030913-1.c: Rename glob to g.
	* gcc.c-torture/execute/960218-1.c: Rename glob to gl.
	* gcc.c-torture/execute/complex-6.c: Rename err to e.
	* gcc.dg/torture/ssa-pta-fn-1.c: Rename glob to g.

	libgomp:
	* testsuite/libgomp.c/pr39591-1.c: Rename err to e.
	* testsuite/libgomp.c/pr39591-2.c: Likewise.
	* testsuite/libgomp.c/pr39591-3.c: Likewise.
	* testsuite/libgomp.c/private-1.c: Likewise.
	* testsuite/libgomp.c/task-1.c: Likewise.
	* testsuite/libgomp.c/task-5.c: Renamed err to serr.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-testsuite-fno-common.patch --]
[-- Type: text/x-patch, Size: 6806 bytes --]

# HG changeset patch
# Parent  4f046c682e63b85a4e432f7f7cd0d7210bca06e4
Fix failures on Solaris with -fno-common default

diff --git a/gcc/testsuite/gcc.c-torture/execute/20030913-1.c b/gcc/testsuite/gcc.c-torture/execute/20030913-1.c
--- a/gcc/testsuite/gcc.c-torture/execute/20030913-1.c
+++ b/gcc/testsuite/gcc.c-torture/execute/20030913-1.c
@@ -1,12 +1,12 @@
 /* Assignments via pointers pointing to global variables were being killed
    by SSA-DCE.  Test contributed by Paul Brook <paul@nowt.org>  */
 
-int glob; 
+int g;
  
 void 
 fn2(int ** q) 
 { 
-  *q = &glob; 
+  *q = &g;
 } 
  
 void test() 
@@ -21,6 +21,6 @@ void test()
 int main() 
 { 
   test(); 
-  if (glob != 42) abort(); 
+  if (g != 42) abort();
   exit (0); 
 }
diff --git a/gcc/testsuite/gcc.c-torture/execute/960218-1.c b/gcc/testsuite/gcc.c-torture/execute/960218-1.c
--- a/gcc/testsuite/gcc.c-torture/execute/960218-1.c
+++ b/gcc/testsuite/gcc.c-torture/execute/960218-1.c
@@ -1,8 +1,8 @@
-int glob;
+int gl;
 
 g (x)
 {
-  glob = x;
+  gl = x;
   return 0;
 }
 
@@ -16,7 +16,7 @@ f (x)
 main ()
 {
   f (3);
-  if (glob != -4)
+  if (gl != -4)
     abort ();
   exit (0);
 }
diff --git a/gcc/testsuite/gcc.c-torture/execute/complex-6.c b/gcc/testsuite/gcc.c-torture/execute/complex-6.c
--- a/gcc/testsuite/gcc.c-torture/execute/complex-6.c
+++ b/gcc/testsuite/gcc.c-torture/execute/complex-6.c
@@ -6,7 +6,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 
-int err;
+int e;
 
 #define TEST(TYPE, FUNC)					\
 __complex__ TYPE						\
@@ -31,7 +31,7 @@ test_ ## FUNC (void)						\
   if (res != 1.0 - 2.0i)					\
     {								\
       printf ("test_" #FUNC " failed\n");			\
-      ++err;							\
+      ++e;							\
     }								\
 }
 
@@ -46,7 +46,7 @@ int
 main (void)
 {
 
-  err = 0;
+  e = 0;
 
   test_float ();
   test_double ();
@@ -54,7 +54,7 @@ main (void)
   test_int ();
   test_long_int ();
 
-  if (err != 0)
+  if (e != 0)
     abort ();
 
   return 0;
diff --git a/gcc/testsuite/gcc.dg/torture/ssa-pta-fn-1.c b/gcc/testsuite/gcc.dg/torture/ssa-pta-fn-1.c
--- a/gcc/testsuite/gcc.dg/torture/ssa-pta-fn-1.c
+++ b/gcc/testsuite/gcc.dg/torture/ssa-pta-fn-1.c
@@ -3,7 +3,7 @@
 /* { dg-skip-if "" { *-*-* } { "-O0" "-fno-fat-lto-objects" } { "" } } */
 
 extern void abort (void);
-int *glob;
+int *g;
 int dummy;
 
 int * __attribute__((noinline,const))
@@ -13,7 +13,7 @@ int * __attribute__((noinline,pure))
 foo_pure(int *p) { return p + dummy; }
 
 int * __attribute__((noinline))
-foo_normal(int *p) { glob = p; return p; }
+foo_normal(int *p) { g = p; return p; }
 
 void test_const(void)
 {
diff --git a/libgomp/testsuite/libgomp.c/pr39591-1.c b/libgomp/testsuite/libgomp.c/pr39591-1.c
--- a/libgomp/testsuite/libgomp.c/pr39591-1.c
+++ b/libgomp/testsuite/libgomp.c/pr39591-1.c
@@ -3,7 +3,7 @@
 
 extern void abort (void);
 
-int err;
+int e;
 
 int
 main (void)
@@ -23,10 +23,10 @@ main (void)
 	for (j = 0; j < sizeof array / sizeof array[0]; j++)
 	  if (array[j] != 0x55555555)
 #pragma omp atomic
-	    err++;
+	    e++;
       }
   }
-  if (err)
+  if (e)
     abort ();
   return 0;
 }
diff --git a/libgomp/testsuite/libgomp.c/pr39591-2.c b/libgomp/testsuite/libgomp.c/pr39591-2.c
--- a/libgomp/testsuite/libgomp.c/pr39591-2.c
+++ b/libgomp/testsuite/libgomp.c/pr39591-2.c
@@ -3,7 +3,7 @@
 
 extern void abort (void);
 
-int err;
+int e;
 
 void __attribute__((noinline))
 foo (int *array)
@@ -14,7 +14,7 @@ foo (int *array)
     for (j = 0; j < 40; j++)
       if (array[j] != 0x55555555)
 #pragma omp atomic
-	err++;
+	e++;
   }
 }
 
@@ -32,7 +32,7 @@ main (void)
     for (i = 0; i < 50; i++)
       foo (array);
   }
-  if (err)
+  if (e)
     abort ();
   return 0;
 }
diff --git a/libgomp/testsuite/libgomp.c/pr39591-3.c b/libgomp/testsuite/libgomp.c/pr39591-3.c
--- a/libgomp/testsuite/libgomp.c/pr39591-3.c
+++ b/libgomp/testsuite/libgomp.c/pr39591-3.c
@@ -3,7 +3,7 @@
 
 extern void abort (void);
 
-int err, a[40];
+int e, a[40];
 
 void __attribute__((noinline))
 foo (int *array)
@@ -14,7 +14,7 @@ foo (int *array)
     for (j = 0; j < 40; j++)
       if (array[j] != 0x55555555)
 #pragma omp atomic
-	err++;
+	e++;
   }
 }
 
@@ -33,7 +33,7 @@ main (void)
     for (i = 0; i < 50; i++)
       foo (a);
   }
-  if (err)
+  if (e)
     abort ();
   return 0;
 }
diff --git a/libgomp/testsuite/libgomp.c/private-1.c b/libgomp/testsuite/libgomp.c/private-1.c
--- a/libgomp/testsuite/libgomp.c/private-1.c
+++ b/libgomp/testsuite/libgomp.c/private-1.c
@@ -20,7 +20,7 @@ f1 (int i, int j, int k)
 }
 
 int v1 = 1, v2 = 2, v5 = 5;
-int err;
+int e;
 
 void
 f2 (void)
@@ -35,11 +35,11 @@ f2 (void)
       #pragma omp parallel num_threads(1) firstprivate(v1, v2, v3, v4)
 	{
 	  if (++v1 != 8 || ++v2 != 3 || ++v3 != 4 || ++v4 != 5 || ++v5 != 6)
-	    err = 1;
+	    e = 1;
 	}
       if (v1 != 7 || v2 != 2 || v3 != 3 || v4 != 4 || v5 != 6)
 	abort ();
-      if (err)
+      if (e)
 	abort ();
     }
   }
diff --git a/libgomp/testsuite/libgomp.c/task-1.c b/libgomp/testsuite/libgomp.c/task-1.c
--- a/libgomp/testsuite/libgomp.c/task-1.c
+++ b/libgomp/testsuite/libgomp.c/task-1.c
@@ -20,7 +20,7 @@ f1 (int i, int j, int k)
 }
 
 int v1 = 1, v2 = 2, v5 = 5;
-int err;
+int e;
 
 void
 f2 (void)
@@ -35,12 +35,12 @@ f2 (void)
       #pragma omp task
 	{
 	  if (++v1 != 8 || ++v2 != 3 || ++v3 != 4 || ++v4 != 5 || ++v5 != 6)
-	    err = 1;
+	    e = 1;
 	}
       #pragma omp taskwait
       if (v1 != 7 || v2 != 2 || v3 != 3 || v4 != 4 || v5 != 6)
 	abort ();
-      if (err)
+      if (e)
 	abort ();
     }
   }
diff --git a/libgomp/testsuite/libgomp.c/task-5.c b/libgomp/testsuite/libgomp.c/task-5.c
--- a/libgomp/testsuite/libgomp.c/task-5.c
+++ b/libgomp/testsuite/libgomp.c/task-5.c
@@ -3,42 +3,42 @@
 #include <omp.h>
 #include <stdlib.h>
 
-int err;
+int serr;
 
 int
 main ()
 {
   int e;
-#pragma omp parallel shared(err)
+#pragma omp parallel shared(serr)
   {
     if (omp_in_final ())
       #pragma omp atomic write
-	err = 1;
-    #pragma omp task if (0) shared(err)
+	serr = 1;
+    #pragma omp task if (0) shared(serr)
       {
 	if (omp_in_final ())
 	  #pragma omp atomic write
-	    err = 1;
-	#pragma omp task if (0) shared(err)
+	    serr = 1;
+	#pragma omp task if (0) shared(serr)
 	  if (omp_in_final ())
 	    #pragma omp atomic write
-	      err = 1;
+	      serr = 1;
       }
-    #pragma omp task final (1) shared(err)
+    #pragma omp task final (1) shared(serr)
       {
 	if (!omp_in_final ())
 	  #pragma omp atomic write
-	    err = 1;
+	    serr = 1;
 	#pragma omp taskyield
 	#pragma omp taskwait
-	#pragma omp task shared(err)
+	#pragma omp task shared(serr)
 	  if (!omp_in_final ())
 	    #pragma omp atomic write
-	      err = 1;
+	      serr = 1;
       }
   }
   #pragma omp atomic read
-    e = err;
+    e = serr;
   if (e)
     abort ();
   return 0;

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Jakub Jelinek]

On Thu, Nov 21, 2019 at 12:03:19PM +0100, Rainer Orth wrote:
> I'm seeing those, too, plus the following that are apparently only
> diagnosed by Solaris ld:
> 
> +FAIL: gcc.c-torture/execute/20030913-1.c   -O0  (test for excess errors)
> +FAIL: gcc.c-torture/execute/20030913-1.c   -O1  (test for excess errors)
> +FAIL: gcc.c-torture/execute/20030913-1.c   -O2  (test for excess errors)
> +FAIL: gcc.c-torture/execute/20030913-1.c   -O2 -flto  (test for excess errors)
> +FAIL: gcc.c-torture/execute/20030913-1.c   -O2 -flto -flto-partition=none  (test for excess errors)
> +FAIL: gcc.c-torture/execute/20030913-1.c   -O3 -g  (test for excess errors)
> +FAIL: gcc.c-torture/execute/20030913-1.c   -Os  (test for excess errors)
> 
> Excess errors:
> ld: warning: symbol 'glob' has differing types:
>         (file /var/tmp//ccdl_86b.o type=OBJT; file /lib/libc.so type=FUNC);
>         /var/tmp//ccdl_86b.o definition taken

That is arguably a weird ld feature, the libraries can contain various
extensions and even if one doesn't use them, there will be a warning if
chosen identifiers for variables match some function in the library.

> 2019-11-21  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	gcc/testsuite:
> 	* gcc.c-torture/execute/20030913-1.c: Rename glob to g.
> 	* gcc.c-torture/execute/960218-1.c: Rename glob to gl.
> 	* gcc.c-torture/execute/complex-6.c: Rename err to e.
> 	* gcc.dg/torture/ssa-pta-fn-1.c: Rename glob to g.
> 
> 	libgomp:
> 	* testsuite/libgomp.c/pr39591-1.c: Rename err to e.
> 	* testsuite/libgomp.c/pr39591-2.c: Likewise.
> 	* testsuite/libgomp.c/pr39591-3.c: Likewise.
> 	* testsuite/libgomp.c/private-1.c: Likewise.
> 	* testsuite/libgomp.c/task-1.c: Likewise.
> 	* testsuite/libgomp.c/task-5.c: Renamed err to serr.

That said, I can live with this, so ok for trunk.

	Jakub

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Wilco Dijkstra]

Hi Rainer,

>> ld: warning: symbol 'err' has differing types:
>>          (file /var/tmp//ccWQCyMc.o type=OBJT; file /lib/libc.so type=FUNC);
>>          /var/tmp//ccWQCyMc.o definition taken

So are glob and err somehow exported as globals by your GLIBC? I don't think those
are standard functions... It's odd the linker didn't give the warning when the mismatch
was on a common data symbol - in both cases you have a function/data mismatch.

Wilco

Re: [PATCH] Fix libgo build (was Re: [PATCH v3] PR85678: Change default to -fno-common)

[Jakub Jelinek]

On Thu, Nov 21, 2019 at 11:40:33AM +0000, Wilco Dijkstra wrote:
> Hi Rainer,
> 
> >> ld: warning: symbol 'err' has differing types:
> >>          (file /var/tmp//ccWQCyMc.o type=OBJT; file /lib/libc.so type=FUNC);
> >>          /var/tmp//ccWQCyMc.o definition taken
> 
> So are glob and err somehow exported as globals by your GLIBC? I don't think those
> are standard functions... It's odd the linker didn't give the warning when the mismatch
> was on a common data symbol - in both cases you have a function/data mismatch.

Both are functions on Linux too, see man glob and man err.

	Jakub

Re: [PATCH] PR85678: Change default to -fno-common

[Martin Liška]

Hello.

I've noticed quite significant package failures caused by the revision.
Would you please consider documenting this change in porting_to.html
(and in changes.html) for GCC 10 release?

Thank you

Re: [PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi Martin,

> I've noticed quite significant package failures caused by the revision.

How significant? Is it mostly the common mistake of forgetting extern?

> Would you please consider documenting this change in porting_to.html
> (and in changes.html) for GCC 10 release?

Sure, I already had a patch for changes.html - I've added an initial porting_to
as well:

[wwwdocs] Document -fcommon default change

Add an entry for the default change. Passes the W3 validator.

--
diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index f0f0d312171a54afede176f06ce76f9c8abaebc4..980e4e591781d04aa888ba5988981006bd30dd1f 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -47,6 +47,13 @@ a work-in-progress.</p>
 
 <!-- .................................................................. -->
 <h2 id="general">General Improvements</h2>
+<p>The following GCC command line options have been introduced or improved.</p>
+<ul>
+  <li>GCC now defaults to <code>-fno-common</code>.  In C, global variables with
+      multiple tentative definitions will result in linker errors.
+      Global variable accesses are also more efficient on various targets.
+  </li>
+</ul>
 
 <p>The following built-in functions have been introduced.</p>
 <ul>
diff --git a/htdocs/gcc-10/porting_to.html b/htdocs/gcc-10/porting_to.html
new file mode 100644
index 0000000000000000000000000000000000000000..2e652f6aa4bd3259a316af0c72ab7eb96bab53b7
--- /dev/null
+++ b/htdocs/gcc-10/porting_to.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+<title>Porting to GCC 10</title>
+<link rel="stylesheet" type="text/css" href="https://gcc.gnu.org/gcc.css" />
+</head>
+
+<body>
+<h1>Porting to GCC 10</h1>
+
+<p>
+The GCC 10 release series differs from previous GCC releases in
+<a href="changes.html">a number of ways</a>. Some of these are a result
+of bug fixing, and some old behaviors have been intentionally changed
+to support new standards, or relaxed in standards-conforming ways to
+facilitate compilation or run-time performance.
+</p>
+
+<p>
+Some of these changes are user visible and can cause grief when
+porting to GCC 10. This document is an effort to identify common issues
+and provide solutions. Let us know if you have suggestions for improvements!
+</p>
+
+
+<!--
+<h2 id="cpp">Preprocessor issues</h2>
+-->
+
+<h2 id="c">C language issues</h2>
+
+<h3 id="complit">Default to <code>-fno-common</code></h3>
+
+<p>
+  A common mistake in C is omitting <code>extern</code> when declaring a global
+  variable in a header file.  If the header is included by several files it
+  results in multiple definitions of the same variable.  In previous GCC
+  versions this error is ignored.  GCC 10 defaults to <code>-fno-common</code>,
+  which means a linker error will now be reported.
+  To fix this, use <code>extern</code> in header files when declaring global
+  variables, and ensure each global is defined in exactly one C file.
+  As a workaround, legacy C code can be compiled with <code>-fcommon</code>.
+</p>
+  <pre><code>
+      int x;  // tentative definition - avoid in header files
+
+      extern int y;  // correct declaration in a header file
+  </code></pre>
+
+<!--
+<h2 id="cxx">C++ language issues</h2>
+-->
+
+<!--
+<h2 id="fortran">Fortran language issues</h2>
+-->
+ 
+<!--
+<h2 id="links">Links</h2>
+-->
+
+</body>
+</html>

Re: [PATCH] PR85678: Change default to -fno-common

[Martin Liška]

On 11/29/19 3:43 PM, Wilco Dijkstra wrote:
> How significant? Is it mostly the common mistake of forgetting extern?

Likely, I see it in at least 400 packages out of 11000 which we have
in openSUSE:Factory. Plus there are many 'nm -B' configure script
defects:
https://lists.gnu.org/archive/html/bug-autoconf/2019-11/msg00001.html

Thank you for the porting to entry!

Martin

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 11/29/19 6:12 AM, Martin Liška wrote:
> Hello.
> 
> I've noticed quite significant package failures caused by the revision.
> Would you please consider documenting this change in porting_to.html
> (and in changes.html) for GCC 10 release?
I'm not in the office right now, but figured I'd chime in.  I'd estimate
400-500 packages are failing in Fedora because of this change.  I'll
have a hard number Monday.

It's significant enough that I'm not sure how we're going to get them
all fixed.

jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi Jeff,

>> I've noticed quite significant package failures caused by the revision.
>> Would you please consider documenting this change in porting_to.html
>> (and in changes.html) for GCC 10 release?
>
> I'm not in the office right now, but figured I'd chime in.  I'd estimate
> 400-500 packages are failing in Fedora because of this change.  I'll
> have a hard number Monday.
>
> It's significant enough that I'm not sure how we're going to get them
> all fixed.

So what normally happens with the numerous new warnings/errors in GCC
releases? I suppose that could cause package failures too. Would it be feasible
to override the options for any failing packages?

Cheers,
Wilco

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 12/4/19 8:24 AM, Wilco Dijkstra wrote:
> Hi Jeff,
> 
>>> I've noticed quite significant package failures caused by the revision.
>>> Would you please consider documenting this change in porting_to.html
>>> (and in changes.html) for GCC 10 release?
>>
>> I'm not in the office right now, but figured I'd chime in.  I'd estimate
>> 400-500 packages are failing in Fedora because of this change.  I'll
>> have a hard number Monday.
>>
>> It's significant enough that I'm not sure how we're going to get them
>> all fixed.
> 
> So what normally happens with the numerous new warnings/errors in GCC
> releases? I suppose that could cause package failures too. Would it be feasible
> to override the options for any failing packages?
Usually we're talking about a few dozen packages that are tripped by any
particular issue.  The -fno-common issue is a full order of magnitude
larger.  My builds show ~450 failures due to this issue.

We're investigating different approaches that don't just involve
reverting the patch or reverting for Fedora.  Those just kick the can
down the road with no real progress and we're in the same position next
year.

The approach that seems most feasible would be to have an opt-out
mechanism.  That would keep -fno-common as the default but provide a
mechanism for a package to opt-out.

Of the ~450 packages affected I'd estimate that even with the opt-out
mechanism we're still going to have to fix ~100 packages immediately
because they don't honor the flags injection mechanisms which the
opt-out approach relies upon.  I'm going to do some testing
today/tomorrow to see how many affected packages don't honor the flags
injection mechanisms we use.

If indeed it's ~100 packages that don't honor the flags injection, then
we're looking at adding 350 markers to opt-out plus ~100 real
fixes/workarounds.  That's still a lot of mindless work, but probably in
the realm of possible.

I will highlight that having the tester has proven hugely valuable here.
 If we'd found out about this later in the process we'd probably be
looking seriously at reversion.

jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Joseph Myers]

On Wed, 4 Dec 2019, Jeff Law wrote:

> > So what normally happens with the numerous new warnings/errors in GCC
> > releases? I suppose that could cause package failures too. Would it be feasible
> > to override the options for any failing packages?
> Usually we're talking about a few dozen packages that are tripped by any
> particular issue.  The -fno-common issue is a full order of magnitude
> larger.  My builds show ~450 failures due to this issue.

I'll note here as an advance warning that WG14 has indicated support for 
making bool, true and false into keywords for C2x, which seems likely to 
be the sort of change that would cause large numbers of build failures in 
pre-C99 code that does "typedef char bool;" and similar.  Assuming it does 
get into C2x, it might make sense, once implemented in GCC, to try such 
distribution builds with a local patch to change the default C language to 
-std=gnu2x, to see the extent of the build failures and possibly get them 
addressed well before the actual GCC default does change to gnu2x.

(The proposal also makes alignas, alignof, static_assert and thread_local 
into keywords, but I'd expect those to cause much smaller numbers of build 
failures than bool, true and false.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 12/4/19 2:03 PM, Joseph Myers wrote:
> On Wed, 4 Dec 2019, Jeff Law wrote:
> 
>>> So what normally happens with the numerous new warnings/errors in GCC
>>> releases? I suppose that could cause package failures too. Would it be feasible
>>> to override the options for any failing packages?
>> Usually we're talking about a few dozen packages that are tripped by any
>> particular issue.  The -fno-common issue is a full order of magnitude
>> larger.  My builds show ~450 failures due to this issue.
> 
> I'll note here as an advance warning that WG14 has indicated support for 
> making bool, true and false into keywords for C2x, which seems likely to 
> be the sort of change that would cause large numbers of build failures in 
> pre-C99 code that does "typedef char bool;" and similar.  Assuming it does 
> get into C2x, it might make sense, once implemented in GCC, to try such 
> distribution builds with a local patch to change the default C language to 
> -std=gnu2x, to see the extent of the build failures and possibly get them 
> addressed well before the actual GCC default does change to gnu2x.
Agreed.  In fact Jon and I have discussed similar testing for flipping
the C++ default as well.

Jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Martin Liška]

On 12/4/19 6:27 PM, Jeff Law wrote:
> On 12/4/19 8:24 AM, Wilco Dijkstra wrote:
>> Hi Jeff,
>>
>>>> I've noticed quite significant package failures caused by the revision.
>>>> Would you please consider documenting this change in porting_to.html
>>>> (and in changes.html) for GCC 10 release?
>>>
>>> I'm not in the office right now, but figured I'd chime in.  I'd estimate
>>> 400-500 packages are failing in Fedora because of this change.  I'll
>>> have a hard number Monday.
>>>
>>> It's significant enough that I'm not sure how we're going to get them
>>> all fixed.
>>
>> So what normally happens with the numerous new warnings/errors in GCC
>> releases? I suppose that could cause package failures too. Would it be feasible
>> to override the options for any failing packages?
> Usually we're talking about a few dozen packages that are tripped by any
> particular issue.  The -fno-common issue is a full order of magnitude
> larger.  My builds show ~450 failures due to this issue.
> 
> We're investigating different approaches that don't just involve
> reverting the patch or reverting for Fedora.  Those just kick the can
> down the road with no real progress and we're in the same position next
> year.
> 
> The approach that seems most feasible would be to have an opt-out
> mechanism.  That would keep -fno-common as the default but provide a
> mechanism for a package to opt-out.

Hello.

I'm going to discuss an approach that will be selected for openSUSE distribution.

> 
> Of the ~450 packages affected I'd estimate that even with the opt-out
> mechanism we're still going to have to fix ~100 packages immediately
> because they don't honor the flags injection mechanisms which the
> opt-out approach relies upon.

Fortunately, we switch at openSUSE to use -fpie by default and our packages
honor the flags ;)

>  I'm going to do some testing
> today/tomorrow to see how many affected packages don't honor the flags
> injection mechanisms we use.
> 
> If indeed it's ~100 packages that don't honor the flags injection, then
> we're looking at adding 350 markers to opt-out plus ~100 real
> fixes/workarounds.  That's still a lot of mindless work, but probably in
> the realm of possible.

I would like to mention here that key work is to report and explain that
to upstream. Only that will help for the future to reduce number of packages
that will need the -fcommon option. That's the biggest effort in my opinion.

> 
> I will highlight that having the tester has proven hugely valuable here.
>   If we'd found out about this later in the process we'd probably be
> looking seriously at reversion.

Fully agree here.
Martin

> 
> jeff
> 

Re: [PATCH] PR85678: Change default to -fno-common

[Tobias Burnus]

On 12/5/19 10:16 AM, Martin Liška wrote:
> I would like to mention here that key work is to report and explain that
> to upstream. Only that will help for the future to reduce number of 
> packages
> that will need the -fcommon option. That's the biggest effort in my 
> opinion.

For this, it would help to update the porting-to web page. Wilco had a 
first version of such a patch at: 
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02662.html – I think it 
should be reviewed and eventually committed.

(Regarding the changes.html change, I think it should be under "C" and 
not under general improvements; as far as I can see, it only affects C 
(and ObjC?), even though the flag is also used for 
lto_symtab_resolve_replaceable_p and in ada/gcc-interface/utils.c.)

Cheers,

Tobias

Re: [PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi,

I have updated the documentation patch here and added relevant maintainers
so hopefully this can go in soon: https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00311.html

I moved the paragraph in changes.html to the C section like you suggested. Would
it make sense to link to the porting_to entry?

Cheers,
Wilco

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 12/5/19 2:16 AM, Martin Liška wrote:
> 
>>
>> Of the ~450 packages affected I'd estimate that even with the opt-out
>> mechanism we're still going to have to fix ~100 packages immediately
>> because they don't honor the flags injection mechanisms which the
>> opt-out approach relies upon.
> 
> Fortunately, we switch at openSUSE to use -fpie by default and our packages
> honor the flags ;)
:-)  Fedora is in much worse shape than RHEL simply because we haven't
pushed hard on the injection issues for Fedora and because there's a ton
of stuff in Fedora that isn't in RHEL.

So it looks like ~150 packages in Fedora that are affected by the
-fno-common change and which aren't honoring the flags injection.  Not
great, but probably manageable.

> 
> I would like to mention here that key work is to report and explain that
> to upstream. Only that will help for the future to reduce number of
> packages
> that will need the -fcommon option. That's the biggest effort in my
> opinion.
Absolutely.  I've found this takes more time than fixing the issue in
the first place on other stuff and I'd expect it to be no different than
the -fno-common stuff.

jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 12/5/19 6:18 AM, Wilco Dijkstra wrote:
> Hi,
> 
> I have updated the documentation patch here and added relevant maintainers
> so hopefully this can go in soon: https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00311.html
> 
> I moved the paragraph in changes.html to the C section like you suggested. Would
> it make sense to link to the porting_to entry?
It might also be helpful to discuss the motivation behind the change.
We're making work for a lot of people with no obvious benefits.

And it's often more complex than just adding an "extern" in a header
file.  Often there isn't a definition anywhere, so you have to go add it
somewhere.  If the package happens to create DSOs, then you have to add
it to the right place or else the DSOs will have undefined references to
the symbols.  If the package doesn't link against and use the DSO, then
you may not know you put the definition in the wrong place until some
*other* package tries to use the DSO and gets an undefined symbol.

Jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Iain Sandoe]

Hi Wilco,

Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:

>> Testsuite fails are order “a few hundred” mostly seem to be related to tree-prof
>> and vector tests (plus the anticipated scan-asm stuff, where code-gen will have
>> changed).  I don’t have cycles to analyse the causes right now - but that gives
>> an idea.
> 
> Are those tests specific to Power? I got 14 failures in total across the full
> C and C++ test suites. Note it's easy to update the default options for a
> specific test directory if needed.

No, the test was on x86_64-darwin16 (m32/m64)  and a very rough estimate of the 
number of fails (but definitely in the hundreds, not tens) - I haven’t tried the same on
PPC; the h/w is tied up with gcc-7 backport testing.

the change for *-*-darwin* will be that the ABI mandates common items to be 
indirected, thus defaulting to no-common will have the effect of changing the
generated code - and it’s a fairly common pattern in the testsuite to have uninit globals.

 * I’d expect broken scan-asms (probably not difficult to fix, just tedious).
 * not sure why it seems to have a particularly adverse effect on the tree-prof stuff.

short on cycles in the run up to stage #1 end + gcc-7 release .. but will try to 
poke at it over the next few days.

cheers
Iain

Re: [PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi Iain,

> for the record,  Darwin bootstraps OK with the change (which is to be expected,
> since the preferred setting for it is -fno-common).

That's good to hear.

> Testsuite fails are order “a few hundred” mostly seem to be related to tree-prof
> and vector tests (plus the anticipated scan-asm stuff, where code-gen will have
> changed).  I don’t have cycles to analyse the causes right now - but that gives
> an idea.

Are those tests specific to Power? I got 14 failures in total across the full
C and C++ test suites. Note it's easy to update the default options for a
specific test directory if needed.

Wilco

Re: [PATCH] PR85678: Change default to -fno-common

[Richard Biener]

On Mon, Oct 28, 2019 at 10:39 PM Iain Sandoe <idsandoe@googlemail.com> wrote:
>
> Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> >>>
> >>> I suppose targets can override this decision.
> >> I think they probably could via the override_options mechanism.
> >
> > Yes, it's trivial to add this to target_option_override():
> >
> >  if (!global_options_set.x_flag_no_common)
> >    flag_no_common = 0;
>
> for the record,  Darwin bootstraps OK with the change (which is to be expected,
> since the preferred setting for it is -fno-common).
>
> Testsuite fails are order “a few hundred” mostly seem to be related to tree-prof
> and vector tests (plus the anticipated scan-asm stuff, where code-gen will have
> changed).  I don’t have cycles to analyse the causes right now - but that gives
> an idea.

Note the vectorizer tests use explicit -fno-common ... (because we
can't re-align
globals if they are COMMON)

Richard.

> cheers
> Iain
>
>
>

Re: [PATCH] PR85678: Change default to -fno-common

[Iain Sandoe]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>>> 
>>> I suppose targets can override this decision. 
>> I think they probably could via the override_options mechanism.
> 
> Yes, it's trivial to add this to target_option_override():
> 
>  if (!global_options_set.x_flag_no_common)
>    flag_no_common = 0;

for the record,  Darwin bootstraps OK with the change (which is to be expected,
since the preferred setting for it is -fno-common).

Testsuite fails are order “a few hundred” mostly seem to be related to tree-prof
and vector tests (plus the anticipated scan-asm stuff, where code-gen will have
changed).  I don’t have cycles to analyse the causes right now - but that gives
an idea.

cheers
Iain

Re: [PATCH] PR85678: Change default to -fno-common

[Wilco Dijkstra]

Hi,

>> I suppose targets can override this decision. 
> I think they probably could via the override_options mechanism.

Yes, it's trivial to add this to target_option_override():

  if (!global_options_set.x_flag_no_common)
    flag_no_common = 0;

Cheers,
Wilco

Re: [PATCH] PR85678: Change default to -fno-common

[Jeff Law]

On 10/28/19 1:43 PM, Richard Biener wrote:
> On October 28, 2019 7:43:33 PM GMT+01:00, David Edelsohn <dje.gcc@gmail.com> wrote:
>>>> Has this been bootstrapped and regression tested?
>>>
>>> Yes, it bootstraps OK of course. I ran regression over the weekend,
>> there
>>> are a few minor regressions in lto due to relying on tentative
>> definitions
>>> and a few latent bugs. I'd expect there will be a few similar
>> failures on
>>> other targets but nothing major since few testcases rely on -fcommon.
>>
>> This almost certainly will break AIX.
> 
> I suppose targets can override this decision. 
I think they probably could via the override_options mechanism.

Jeff

Re: [PATCH] PR85678: Change default to -fno-common

[Richard Biener]

On October 28, 2019 7:43:33 PM GMT+01:00, David Edelsohn <dje.gcc@gmail.com> wrote:
>>> Has this been bootstrapped and regression tested?
>>
>> Yes, it bootstraps OK of course. I ran regression over the weekend,
>there
>> are a few minor regressions in lto due to relying on tentative
>definitions
>> and a few latent bugs. I'd expect there will be a few similar
>failures on
>> other targets but nothing major since few testcases rely on -fcommon.
>
>This almost certainly will break AIX.

I suppose targets can override this decision. 

Richard. 

>- David

Re: [PATCH] PR85678: Change default to -fno-common

[David Edelsohn]

>> Has this been bootstrapped and regression tested?
>
> Yes, it bootstraps OK of course. I ran regression over the weekend, there
> are a few minor regressions in lto due to relying on tentative definitions
> and a few latent bugs. I'd expect there will be a few similar failures on
> other targets but nothing major since few testcases rely on -fcommon.

This almost certainly will break AIX.

- David