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 <[email protected]>
>
> 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
