Commit 0a9fc923 authored by Sjoerd Meijer's avatar Sjoerd Meijer
Browse files

[Driver] Default to -fno-common for all targets 

This makes -fno-common the default for all targets because this has performance
and code-size benefits and is more language conforming for C code.
Additionally, GCC10 also defaults to -fno-common and so we get consistent
behaviour with GCC.

With this change, C code that uses tentative definitions as definitions of a
variable in multiple translation units will trigger multiple-definition linker
errors. Generally, this occurs when the use of the extern keyword is neglected
in the declaration of a variable in a header file. In some cases, no specific
translation unit provides a definition of the variable. The previous behavior
can be restored by specifying -fcommon.

As GCC has switched already, we benefit from applications already being ported
and existing documentation how to do this. For example:
- https://gcc.gnu.org/gcc-10/porting_to.html
- https://wiki.gentoo.org/wiki/Gcc_10_porting_notes/fno_common

Differential revision: https://reviews.llvm.org/D75056
parent d58e383f

clang/docs/ClangCommandLineReference.rst

+4 −0
Original line number Diff line number Diff line
@@ -1307,6 +1307,10 @@ Use colors in diagnostics 


.. option:: -fcommon, -fno-common



Place definitions of variables with no storage class and no initializer

(tentative definitions) in a common block, instead of generating individual

zero-initialized definitions (default -fno-common).



.. option:: -fcompile-resource=<arg>, --resource <arg>, --resource=<arg>



.. option:: -fconstant-cfstrings, -fno-constant-cfstrings

clang/docs/ReleaseNotes.rst

+7 −0
Original line number Diff line number Diff line
@@ -84,6 +84,13 @@ future versions of Clang. 
Modified Compiler Flags

-----------------------



- -fno-common has been enabled as the default for all targets.  Therefore, C

  code that uses tentative definitions as definitions of a variable in multiple

  translation units will trigger multiple-definition linker errors.  Generally,

  this occurs when the use of the ``extern`` keyword is neglected in the declaration

  of a variable in a header file. In some cases, no specific translation unit

  provides a definition of the variable. The previous behavior can be restored by

  specifying ``-fcommon``.



New Pragmas in Clang

--------------------

clang/include/clang/Driver/Options.td

+2 −1
Original line number Diff line number Diff line
@@ -848,7 +848,8 @@ def fno_record_command_line : Flag<["-"], "fno-record-command-line">, 
  Group<f_clang_Group>;

def : Flag<["-"], "frecord-gcc-switches">, Alias<frecord_command_line>;

def : Flag<["-"], "fno-record-gcc-switches">, Alias<fno_record_command_line>;

def fcommon : Flag<["-"], "fcommon">, Group<f_Group>;

def fcommon : Flag<["-"], "fcommon">, Group<f_Group>,

  Flags<[CoreOption, CC1Option]>, HelpText<"Place uninitialized global variables in a common block">;

def fcompile_resource_EQ : Joined<["-"], "fcompile-resource=">, Group<f_Group>;

def fcomplete_member_pointers : Flag<["-"], "fcomplete-member-pointers">, Group<f_clang_Group>,

   Flags<[CoreOption, CC1Option]>,

clang/lib/Driver/ToolChains/Clang.cpp

+3 −19
Original line number Diff line number Diff line
@@ -1408,20 +1408,6 @@ static bool isSignedCharDefault(const llvm::Triple &Triple) { 
  }

}



static bool isNoCommonDefault(const llvm::Triple &Triple) {

  switch (Triple.getArch()) {

  default:

    if (Triple.isOSFuchsia())

      return true;

    return false;



  case llvm::Triple::xcore:

  case llvm::Triple::wasm32:

  case llvm::Triple::wasm64:

    return true;

  }

}



static bool hasMultipleInvocations(const llvm::Triple &Triple,

                                   const ArgList &Args) {

  // Supported only on Darwin where we invoke the compiler multiple times
@@ -5675,11 +5661,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, 
  if (!Args.hasFlag(options::OPT_Qy, options::OPT_Qn, true))

    CmdArgs.push_back("-Qn");



  // -fcommon is the default unless compiling kernel code or the target says so

  bool NoCommonDefault = KernelOrKext || isNoCommonDefault(RawTriple);

  if (!Args.hasFlag(options::OPT_fcommon, options::OPT_fno_common,

                    !NoCommonDefault))

    CmdArgs.push_back("-fno-common");

  // -fno-common is the default, set -fcommon only when that flag is set.

  if (Args.hasFlag(options::OPT_fcommon, options::OPT_fno_common, false))

    CmdArgs.push_back("-fcommon");



  // -fsigned-bitfields is default, and clang doesn't yet support

  // -funsigned-bitfields.

clang/lib/Frontend/CompilerInvocation.cpp

+1 −1
Original line number Diff line number Diff line
@@ -809,7 +809,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, 
  Opts.RecordCommandLine =

      std::string(Args.getLastArgValue(OPT_record_command_line));

  Opts.MergeAllConstants = Args.hasArg(OPT_fmerge_all_constants);

  Opts.NoCommon = Args.hasArg(OPT_fno_common);

  Opts.NoCommon = !Args.hasArg(OPT_fcommon);

  Opts.NoInlineLineTables = Args.hasArg(OPT_gno_inline_line_tables);

  Opts.NoImplicitFloat = Args.hasArg(OPT_no_implicit_float);

  Opts.OptimizeSize = getOptimizationLevelSize(Args);