| Message ID | DB6PR0801MB20537CD32063175699A3E50F832F0@DB6PR0801MB2053.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] Change default to -fcommon | expand |
On 11/17/2017 12:12 PM, Wilco Dijkstra wrote: > GCC currently defaults to -fcommon. This is an optional C feature dating > back to early C implementations. On many targets this means global variable > accesses having an unnecessary codesize and performance penalty in C code > (the same source generates better code when built as C++). Given there isn't > a lot of software that really requires this (mostly it's accidentally forgetting to > use 'extern' in a header), it is about time to change the default. > > What do people think? I presume someone with access to distro source code > and a fast build machine could try and see how many packages fail to get an > idea how feasible it is. We could keep defaulting to -fcommon with -std=c89 > if necessary. > > 2017-11-17 Wilco Dijkstra <wdijkstr@arm.com> > > * common.opt (fcommon): Change init to 1. > * doc/invoke.texi (-fcommon): Update documentation. I'd tentatively support this -- meaning I'd be willing to ack it as long as we all agree that if it causes significant problems during the mass rebuilds done by the distros (typically during Dec-Feb) that we'd revert. It'd need a release note as well. Jeff
On 11/17/2017 12:12 PM, Wilco Dijkstra wrote: > GCC currently defaults to -fcommon. This is an optional C feature dating > back to early C implementations. On many targets this means global variable > accesses having an unnecessary codesize and performance penalty in C code > (the same source generates better code when built as C++). Given there isn't > a lot of software that really requires this (mostly it's accidentally forgetting to > use 'extern' in a header), it is about time to change the default. > > What do people think? I presume someone with access to distro source code > and a fast build machine could try and see how many packages fail to get an > idea how feasible it is. We could keep defaulting to -fcommon with -std=c89 > if necessary. > > 2017-11-17 Wilco Dijkstra <wdijkstr@arm.com> > > * common.opt (fcommon): Change init to 1. > * doc/invoke.texi (-fcommon): Update documentation. The doc parts of this patch are OK. Personally, I think the change is a good idea, but I don't know how much breakage it would cause. One data point I can contribute is that when we looked at this several years ago, we found that all competing compilers on bare-metal targets implemented the -fno-common behavior. So another possible compromise is keeping -fcommon on Unix-like targets where it is the traditional behavior, and switching to -fno-common elsewhere where it's not. I don't see much benefit in tying the default to -std= since it's not behavior specified in the C standard at all. -Sandra
On Sun, Nov 19, 2017 at 2:48 AM, Sandra Loosemore <sandra@codesourcery.com> wrote: > On 11/17/2017 12:12 PM, Wilco Dijkstra wrote: >> >> GCC currently defaults to -fcommon. This is an optional C feature dating >> back to early C implementations. On many targets this means global >> variable >> accesses having an unnecessary codesize and performance penalty in C code >> (the same source generates better code when built as C++). Given there >> isn't >> a lot of software that really requires this (mostly it's accidentally >> forgetting to >> use 'extern' in a header), it is about time to change the default. >> >> What do people think? I presume someone with access to distro source code >> and a fast build machine could try and see how many packages fail to get >> an >> idea how feasible it is. We could keep defaulting to -fcommon with >> -std=c89 >> if necessary. >> >> 2017-11-17 Wilco Dijkstra <wdijkstr@arm.com> >> >> * common.opt (fcommon): Change init to 1. >> * doc/invoke.texi (-fcommon): Update documentation. > > > The doc parts of this patch are OK. > > Personally, I think the change is a good idea, but I don't know how much > breakage it would cause. One data point I can contribute is that when we > looked at this several years ago, we found that all competing compilers on > bare-metal targets implemented the -fno-common behavior. So another possible > compromise is keeping -fcommon on Unix-like targets where it is the > traditional behavior, and switching to -fno-common elsewhere where it's not. > I don't see much benefit in tying the default to -std= since it's not > behavior specified in the C standard at all. A target specific default might be a good idea if we decide to revert. Note I proposed this change a few times already, but the fear was always we'll break too much legacy code. Note you have to make sure GFortran still works! So I think the patch should be changed to make the default behavior be frontend dependent or have a fortran/ adjustment that fixes things up for the fortran dialects that need it. CCing fortran@ Richard. > -Sandra
Richard Biener wrote: > A target specific default might be a good idea if we decide to revert. > > Note I proposed this change a few times already, but the fear was always > we'll break too much legacy code. It will definitely break some code, but new warnings with -Werror might too... > Note you have to make sure GFortran still works! So I think the patch should > be changed to make the default behavior be frontend dependent or have a > fortran/ adjustment that fixes things up for the fortran dialects that need it. Fortran doesn't use flag_no_common, so COMMON globals are not affected. There is one use in Ada which looks like an optimization for specific targets: /* Ada doesn't feature Fortran-like COMMON variables so we shouldn't try to fiddle with DECL_COMMON. However, on platforms that don't support global BSS sections, uninitialized global variables would go in DATA instead, thus increasing the size of the executable. */ if (!flag_no_common && TREE_CODE (var_decl) == VAR_DECL && TREE_PUBLIC (var_decl) && !have_global_bss_p ()) DECL_COMMON (var_decl) = 1; I don't understand how this works - if there is no bss support in the linker, wouldn't common variables would still end up in the data section? Wilco
Hi, On Mon, 20 Nov 2017, Wilco Dijkstra wrote: > > Note you have to make sure GFortran still works! So I think the patch > > should be changed to make the default behavior be frontend dependent > > or have a fortran/ adjustment that fixes things up for the fortran > > dialects that need it. > > Fortran doesn't use flag_no_common, so COMMON globals are not affected. > > There is one use in Ada which looks like an optimization for specific targets: > > /* Ada doesn't feature Fortran-like COMMON variables so we shouldn't > try to fiddle with DECL_COMMON. However, on platforms that don't > support global BSS sections, uninitialized global variables would > go in DATA instead, thus increasing the size of the executable. */ > if (!flag_no_common > && TREE_CODE (var_decl) == VAR_DECL > && TREE_PUBLIC (var_decl) > && !have_global_bss_p ()) > DECL_COMMON (var_decl) = 1; > > I don't understand how this works - if there is no bss support in the > linker, wouldn't common variables would still end up in the data > section? bss _sections_ != bss-like segments in the executable. Targets might not have a bss section that could be named in the asm file, or no way to switch to it without disrupting surrounding code, but they might have common symbols, which ultimately might or might not be collected in some bss-like segment. In that case you want to use them instead of symbols in .data. What's your rationale for changing this? In your initial mail you said: "On many targets this means global variable accesses having an unnecessary codesize and performance penalty in C code (the same source generates better code when built as C++)." I have a hard time imaging that, so can you give details? FWIW I've personally always considered using common symbols nicer. Ciao, Michael.
On 11/20/2017 10:34 AM, Michael Matz wrote: > What's your rationale for changing this? In your initial mail you said: > > "On many targets this means global variable accesses having an unnecessary > codesize and performance penalty in C code (the same source generates > better code when built as C++)." > > I have a hard time imaging that, so can you give details? FWIW I've > personally always considered using common symbols nicer. The optimization case I'm familiar with is for targets that support -G and GP-relative addressing modes to address small data. Generally you can only do that if the variable is allocated in the same compilation unit as the reference, so the compiler knows definitely where it's placed (unless the backend also supports some other mechanism for asserting that a variable is small data). Putting tentatively-defined variables in common defeats that optimization. -Sandra
On November 20, 2017 9:02:55 PM GMT+01:00, Sandra Loosemore <sandra@codesourcery.com> wrote: >On 11/20/2017 10:34 AM, Michael Matz wrote: >> What's your rationale for changing this? In your initial mail you >said: >> >> "On many targets this means global variable accesses having an >unnecessary >> codesize and performance penalty in C code (the same source generates >> better code when built as C++)." >> >> I have a hard time imaging that, so can you give details? FWIW I've >> personally always considered using common symbols nicer. > >The optimization case I'm familiar with is for targets that support -G >and GP-relative addressing modes to address small data. Generally you >can only do that if the variable is allocated in the same compilation >unit as the reference, so the compiler knows definitely where it's >placed (unless the backend also supports some other mechanism for >asserting that a variable is small data). Putting tentatively-defined >variables in common defeats that optimization. Also we cannot raise alignment of commons and thus vectorization is pessimized (all vectorizer testcases use - fno-common). IIRC LTO promotes commons to locals. Richard. >-Sandra
Hi, On Mon, 20 Nov 2017, Sandra Loosemore wrote: > > I have a hard time imaging that, so can you give details? FWIW I've > > personally always considered using common symbols nicer. > > The optimization case I'm familiar with is for targets that support -G > and GP-relative addressing modes to address small data. Generally you > can only do that if the variable is allocated in the same compilation > unit as the reference, so the compiler knows definitely where it's > placed For this to work it doesn't have to be from the same compilation unit, but rather needs to be put into something like .sdata or .sbss. That in turn can be placed near whereever GP points to (GOT-like usually) by the link editor and then benefit from gp-relative addressing because the working theory is that by putting small stuff into .sdata it doesn't become larger than whatever the allowed offset for this addressing mode can be. What I'm getting at is that this needs help from the link editor. Same compilation unit doesn't really enter the picture, except if you indeed have different GPs per compilation unit. But sure, making those symbols SHN_COMMON (let's speak ELF :) ) indeed defeats this (well, except if the linker puts small SHN_COMMONs into the equivalent of .sbss). Could also be done by the symtab placing routines, remove DECL_COMMON and give it .sbss section or equivalent. I can see how just not enabling -fcommon is the easier path of course. Ciao, Michael.
Hi, On Mon, 20 Nov 2017, Richard Biener wrote: > Also we cannot raise alignment of commons and thus vectorization is > pessimized (all vectorizer testcases use - fno-common). That would be a simple oversight then. That's one of the nice things with common symbols, they contain their own alignment which you can freely adjust, you don't have to care for something like section alignment. > IIRC LTO promotes commons to locals. Might be, but if so, probably for no good reason. Ciao, Michael.
> There is one use in Ada which looks like an optimization for specific > targets: > > /* Ada doesn't feature Fortran-like COMMON variables so we shouldn't > try to fiddle with DECL_COMMON. However, on platforms that don't > support global BSS sections, uninitialized global variables would > go in DATA instead, thus increasing the size of the executable. */ > if (!flag_no_common > && TREE_CODE (var_decl) == VAR_DECL > && TREE_PUBLIC (var_decl) > && !have_global_bss_p ()) > DECL_COMMON (var_decl) = 1; It's for Darwin - you need to evaluate your patch on Darwin. > I don't understand how this works - if there is no bss support in the > linker, wouldn't common variables would still end up in the data section? There is, it's essentially a syntactic issue in the assembler IIRC.
On Mon, Nov 20, 2017 at 11:00 PM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Mon, 20 Nov 2017, Richard Biener wrote: > >> Also we cannot raise alignment of commons and thus vectorization is >> pessimized (all vectorizer testcases use - fno-common). > > That would be a simple oversight then. That's one of the nice things with > common symbols, they contain their own alignment which you can freely > adjust, you don't have to care for something like section alignment. You can't because of the need for merging a real definition with lower alignment with the adjusted tentative one. Richard. >> IIRC LTO promotes commons to locals. > > Might be, but if so, probably for no good reason. > > > Ciao, > Michael.
Hi, On Tue, 21 Nov 2017, Richard Biener wrote: > > That would be a simple oversight then. That's one of the nice things > > with common symbols, they contain their own alignment which you can > > freely adjust, you don't have to care for something like section > > alignment. > > You can't because of the need for merging a real definition with lower > alignment with the adjusted tentative one. Yes, that is the real neck breaker, and I hadn't considered this initially. Objections withdrawn :) Ciao, Michael.
Michael Matz wrote: > bss _sections_ != bss-like segments in the executable. Targets might not > have a bss section that could be named in the asm file, or no way to > switch to it without disrupting surrounding code, but they might have > common symbols, which ultimately might or might not be collected in some > bss-like segment. In that case you want to use them instead of symbols in > .data. OK, thanks for the explanation. For large symbols it obviously makes sense to keep the executable size reasonable. Is this really Ada specific and related to -fcommon? We could do this optimization without checking flag_no_common. > What's your rationale for changing this? In your initial mail you said: > > "On many targets this means global variable accesses having an unnecessary > codesize and performance penalty in C code (the same source generates > better code when built as C++)." > > I have a hard time imaging that, so can you give details? FWIW I've > personally always considered using common symbols nicer. Basically -fcommon disables the anchor optimization on RISC targets, here is a simple example on AArch64: int a, b, c; int f (void) { return a + b + c; } With -fcommon: f: adrp x1, a adrp x0, b adrp x2, c ldr w1, [x1, #:lo12:a] ldr w0, [x0, #:lo12:b] ldr w2, [x2, #:lo12:c] add w0, w1, w0 add w0, w0, w2 ret With -fno-common: f: adrp x0, .LANCHOR0 add x2, x0, :lo12:.LANCHOR0 ldr w1, [x0, #:lo12:.LANCHOR0] ldp w0, w2, [x2, 4] add w0, w1, w0 add w0, w0, w2 ret The anchor pointer is set up once and cached across the function, making accesses to multiple globals cheap and enabling other optimizations. On various targets (eg. PPC, Arm) creating the address of a global takes 2 instructions so the savings are larger. Wilco
diff --git a/gcc/common.opt b/gcc/common.opt index 59940c64356964f8f9b9d842ad3f1a1c02548bab..575472aee7572601b6276944679c0a55171e39b2 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1110,7 +1110,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) Do not put uninitialized globals in the common section. fcompare-debug diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 7ea9f765717d1ace59276406777a6055d6ed6b59..b618e648dbd9ab4a16ec856aea7a6660caad3e9a 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -12494,8 +12494,8 @@ 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 @cindex tentative definitions In C code, this option controls the placement of global variables defined without an initializer, known as @dfn{tentative definitions} @@ -12506,15 +12506,14 @@ 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 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. +definition. This is the behavior specified by @option{-fcommon}. 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 +C, and on several targets implies a speed and code size penalty on variable references. -The @option{-fno-common} option specifies that the compiler should instead -place uninitialized global variables in the data section of the object file. +The @option{-fno-common} option is the default. It specifies that the +compiler should instead place uninitialized global variables in the data +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.