[mips] Implement -muninit-const-in-rodata

[sdardis]

Summary

This option when combined with -mgpopt and -membedded-data places all
uninitialized constant variables in the read-only section.

[sdardis]

Ping.

[atanasyan]

include/clang/Driver/Options.td

2065 ↗

(On Diff #108359)

s/Place/place/

lib/Driver/ToolChains/Clang.cpp

1523 ↗

(On Diff #108359)

What's happened if the -muninit-const-in-rodata is used without -membedded-data? Will the compiler shows a warning about unused command line option?

[sdardis]

Address review comment.

lib/Driver/ToolChains/Clang.cpp

1523 ↗

(On Diff #108359)

Yes, it warns that it's unused.

[atanasyan]

LGTM

[sdardis]

Thanks for the reviews of all of these options.

[efriedma]

Do you actually need a new flag for this? "-fno-common" will ensure clang doesn't generate globals with common linkage.

[efriedma]

cfe/trunk/lib/CodeGen/TargetInfo.cpp

6666

Also, this change is clearly unacceptable; among other things, messing with the linkage of static local variables is likely to lead to link errors.

[sdardis]

It's required for feature parity with GCC.

-fno-common will place globals into the bss section which uses memory at runtime. The -membedded-data and -muninit-const-in-rodata options are for reducing RAM usage in some embedded environments.

[efriedma]

LLVM never puts constant data into the BSS. See isSuitableForBSS in lib/Target/TargetLoweringObjectFile.cpp.

(gcc's behavior is just weird... apparently, whether or not constant data is placed in the BSS with -fno-common depends on the syntactic form of the initializer?)

[sdardis]

cfe/trunk/lib/CodeGen/TargetInfo.cpp

6666

I will fix that.

[joerg]

I don't see any reason why zero-initialised constants should be emitted in BSS. I know that GCC does that and I just fixed bugs in that because created wrong section flags for it. So yes, I'd prefer to revert this and fix the real problem.

[sdardis]

In D35917#830898, @joerg wrote:

I don't see any reason why zero-initialised constants should be emitted in BSS. I know that GCC does that and I just fixed bugs in that because created wrong section flags for it. So yes, I'd prefer to revert this and fix the real problem.

Will do.

[sdardis]

rL309978 for the revert.

cfe/trunk/include/clang/Driver/Options.td

def mno_extern_sdata : Flag<["-"], "mno-extern-sdata">, Group<m_Group>,
   HelpText<"Do not assume that externally defined data is in the small data if"
            " it meets the -G <size> threshold (MIPS)">;
 def membedded_data : Flag<["-"], "membedded-data">, Group<m_Group>,
   HelpText<"Place constants in the .rodata section instead of the .sdata "
            "section even if they meet the -G <size> threshold (MIPS)">;
 def mno_embedded_data : Flag<["-"], "mno-embedded-data">, Group<m_Group>,
   HelpText<"Do not place constants in the .rodata section instead of the "
            ".sdata if they meet the -G <size> threshold (MIPS)">;
+def muninit_const_in_rodata : Flag<["-"], "muninit-const-in-rodata">,
+  Group<m_Group>, Flags<[DriverOption,CC1Option]>, HelpText<"Place "
+  "uninitialized constants in the read-only data section instead of"
+  " the common section (MIPS)">;
+def mno_uninit_const_in_rodata : Flag<["-"], "mno-uninit-const-in-rodata">,
+  Group<m_Group>, HelpText<"Do not place uninitialized constants in the "
+                           "read-only data section instead of the common"
+                           " section (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group<m_Group>;
 def mabicalls : Flag<["-"], "mabicalls">, Group<m_Group>,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;
 def mno_abicalls : Flag<["-"], "mno-abicalls">, Group<m_Group>,
   HelpText<"Disable SVR4-style position-independent code (Mips only)">;
 def mips1 : Flag<["-"], "mips1">,
   Alias<march_EQ>, AliasArgs<["mips1"]>,
   HelpText<"Equivalent to -march=mips1">, Flags<[HelpHidden]>;

cfe/trunk/include/clang/Frontend/CodeGenOptions.def

CODEGENOPT(LTOUnit, 1, 0) ///< Emit IR to support LTO unit features (CFI, whole
                           ///< program vtable opt).
 CODEGENOPT(IncrementalLinkerCompatible, 1, 0) ///< Emit an object file which can
                                               ///< be used with an incremental
                                               ///< linker.
 CODEGENOPT(MergeAllConstants , 1, 1) ///< Merge identical constants.
 CODEGENOPT(MergeFunctions    , 1, 0) ///< Set when -fmerge-functions is enabled.
 CODEGENOPT(MSVolatile        , 1, 0) ///< Set when /volatile:ms is enabled.
 CODEGENOPT(NoCommon          , 1, 0) ///< Set when -fno-common or C++ is enabled.
+CODEGENOPT(UInitCstDataInROData, 1, 0) ///< Set when -mgpopt & -membedded-data
+                                       ///< & -muinit-const-in-rodata is set
 CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is
                                        ///< enabled.
 CODEGENOPT(NoExecStack       , 1, 0) ///< Set when -Wa,--noexecstack is enabled.
 CODEGENOPT(FatalWarnings     , 1, 0) ///< Set when -Wa,--fatal-warnings is
                                      ///< enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is enabled.
 CODEGENOPT(NoInfsFPMath      , 1, 0) ///< Assume FP arguments, results not +-Inf.

cfe/trunk/lib/CodeGen/TargetInfo.cpp

public:
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule &CGM) const override {
     return 29;
   }
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
                            CodeGen::CodeGenModule &CGM,
                            ForDefinition_t IsForDefinition) const override {
+
+    if (const VarDecl *VD = dyn_cast_or_null<VarDecl>(D)) {
+      if (CGM.getCodeGenOpts().UInitCstDataInROData &&
+          VD->getType().isConstQualified() && !VD->hasInit()) {
+        llvm::GlobalVariable *GVar = dyn_cast_or_null<llvm::GlobalVariable>(GV);
+        if (GVar && !GVar->hasSection()) {
+          GVar->setLinkage(llvm::GlobalValue::ExternalLinkage);
+          GVar->setSection("rodata");
+        }
+      }
+
+      return;
+    }
+
     const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
     if (!FD) return;
     llvm::Function *Fn = cast<llvm::Function>(GV);
 
     if (FD->hasAttr<MipsLongCallAttr>())
       Fn->addFnAttr("long-call");
     else if (FD->hasAttr<MipsShortCallAttr>())
       Fn->addFnAttr("short-call");

cfe/trunk/lib/Driver/ToolChains/Clang.cpp

if (NoABICalls && (!GPOpt || WantGPOpt)) {
     if (EmbeddedData) {
       CmdArgs.push_back("-mllvm");
       if (EmbeddedData->getOption().matches(options::OPT_membedded_data)) {
         CmdArgs.push_back("-membedded-data=1");
       } else {
         CmdArgs.push_back("-membedded-data=0");
       }
       EmbeddedData->claim();
+
+      if (Arg *A = Args.getLastArg(options::OPT_muninit_const_in_rodata,
+                                   options::OPT_mno_uninit_const_in_rodata)) {
+        if (A->getOption().matches(options::OPT_muninit_const_in_rodata)) {
+          CmdArgs.push_back("-muninit-const-in-rodata");
+          A->claim();
+        }
+      }
     }
 
   } else if ((!ABICalls || (!NoABICalls && ABICalls)) && WantGPOpt)
     D.Diag(diag::warn_drv_unsupported_gpopt) << (ABICalls ? 0 : 1);
 
   if (GPOpt)
     GPOpt->claim();
 

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

parseSanitizerKinds("-fsanitize-trap=",
                       Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
                       Opts.SanitizeTrap);
 
   Opts.CudaGpuBinaryFileNames =
       Args.getAllArgValues(OPT_fcuda_include_gpubinary);
 
   Opts.Backchain = Args.hasArg(OPT_mbackchain);
 
+  Opts.UInitCstDataInROData = Args.hasArg(OPT_muninit_const_in_rodata);
+
   Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(
       Args, OPT_fsanitize_undefined_strip_path_components_EQ, 0, Diags);
 
   return Success;
 }
 
 static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
                                       ArgList &Args) {

cfe/trunk/test/CodeGen/mips-uninit-const-in-ro.c

+// RUN: %clang_cc1 -triple mips-mti--elf -emit-llvm -mrelocation-model static \
+// RUN:            -target-feature +noabicalls -mllvm -mgpopt -mllvm \
+// RUN:            -membedded-data=1 -muninit-const-in-rodata -o - %s | \
+// RUN:   FileCheck %s
+
+// Test that -muninit-const-in-rodata places constant uninitialized structures
+// in the .rodata section rather than the commeon section.
+
+// CHECK: @a = global [8 x i32] zeroinitializer, section "rodata", align 4
+const int a[8];