COFF: Let globals with private linkage reside in their own section

[majnemer]

Summary

COFF COMDATs (for selection kinds other than 'select any') require at
least one non-section symbol in the symbol table.
Satisfy this by morally enhancing the linkage from private to internal.

[majnemer]

• Address review comments

[majnemer]

• Address Reid's comments

[• rafael]

include/llvm/IR/DataLayout.h

264 ↗

(On Diff #22137)

Given the discussion of making this "the closest to private that the linker still sees", this should probably return "" on all non-macho.

test/CodeGen/ARM/Windows/long-calls.ll

13 ↗

(On Diff #22137)

This is a bug fix, right? The old .L prefix would actully be visible but the L one is not?

[majnemer]

include/llvm/IR/DataLayout.h

264 ↗

(On Diff #22137)

Done.

test/CodeGen/ARM/Windows/long-calls.ll

13 ↗

(On Diff #22137)

It was necessary to fix because ARM and 64-bit x86 were not using MM_WinCOFF, they were using the ELF mangling.

[• rafael]

LGTM

[majnemer]

• Address newest round of feedback

include/llvm/CodeGen/TargetLoweringObjectFileImpl.h

getExplicitSectionGlobal(const GlobalValue *GV,
                              SectionKind Kind, Mangler &Mang,
                              const TargetMachine &TM) const override;
 
   const MCSection *
     SelectSectionForGlobal(const GlobalValue *GV,
                            SectionKind Kind, Mangler &Mang,
                            const TargetMachine &TM) const override;
 
+  void getNameWithPrefix(SmallVectorImpl<char> &OutName, const GlobalValue *GV,
+                         bool CannotUsePrivateLabel, Mangler &Mang,
+                         const TargetMachine &TM) const override;
+
   const MCSection *
   getSectionForJumpTable(const Function &F, Mangler &Mang,
                          const TargetMachine &TM) const override;
 
   /// Extract the dependent library name from a linker option string. Returns
   /// StringRef() if the option does not specify a library.
   StringRef getDepLibFromLinkerOpt(StringRef LinkerOption) const override;
 

include/llvm/IR/Mangler.h

 
 public:
   Mangler(const DataLayout *DL) : DL(DL), NextAnonGlobalID(1) {}
 
   /// Print the appropriate prefix and the specified global variable's name.
   /// If the global variable doesn't have a name, this fills in a unique name
   /// for the global.
   void getNameWithPrefix(raw_ostream &OS, const GlobalValue *GV,
-                         bool CannotUsePrivateLabel) const;
+                         bool CannotUsePrivateLabel,
+                         bool ForceNonPrivate = false) const;
   void getNameWithPrefix(SmallVectorImpl<char> &OutName, const GlobalValue *GV,
-                         bool CannotUsePrivateLabel) const;
+                         bool CannotUsePrivateLabel,
+                         bool ForceNonPrivate = false) const;
 
   /// Print the appropriate prefix and the specified name as the global variable
   /// name. GVName must not be empty.
   void getNameWithPrefix(raw_ostream &OS, const Twine &GVName,
                          ManglerPrefixTy PrefixTy = Mangler::Default) const;
   void getNameWithPrefix(SmallVectorImpl<char> &OutName, const Twine &GVName,
                          ManglerPrefixTy PrefixTy = Mangler::Default) const;
 };
 
 } // End llvm namespace
 
 #endif

include/llvm/Target/TargetLoweringObjectFile.h

public:
   /// variable or function definition. This should not be passed external (or
   /// available externally) globals.
   const MCSection *SectionForGlobal(const GlobalValue *GV,
                                     Mangler &Mang,
                                     const TargetMachine &TM) const {
     return SectionForGlobal(GV, getKindForGlobal(GV, TM), Mang, TM);
   }
 
+  virtual void getNameWithPrefix(SmallVectorImpl<char> &OutName,
+                                 const GlobalValue *GV,
+                                 bool CannotUsePrivateLabel, Mangler &Mang,
+                                 const TargetMachine &TM) const;
+
   virtual const MCSection *
   getSectionForJumpTable(const Function &F, Mangler &Mang,
                          const TargetMachine &TM) const;
 
   virtual bool shouldPutJumpTableInFunctionSection(bool UsesLabelDifference,
                                                    const Function &F) const;
 
   /// Targets should implement this method to assign a section to globals with

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

if ((EmitUniquedSection && !Kind.isCommon()) || GV->hasComdat()) {
     else
       ComdatGV = GV;
 
     if (!ComdatGV->hasPrivateLinkage()) {
       MCSymbol *Sym = TM.getSymbol(ComdatGV, Mang);
       StringRef COMDATSymName = Sym->getName();
       return getContext().getCOFFSection(Name, Characteristics, Kind,
                                          COMDATSymName, Selection);
+    } else {
+      SmallString<256> TmpData;
+      getNameWithPrefix(TmpData, GV, true, Mang, TM);
+      return getContext().getCOFFSection(Name, Characteristics, Kind, TmpData,
+                                         Selection);
     }
   }
 
   if (Kind.isText())
     return TextSection;
 
   if (Kind.isThreadLocal())
     return TLSDataSection;
 
   if (Kind.isReadOnly() || Kind.isReadOnlyWithRel())
     return ReadOnlySection;
 
   // Note: we claim that common symbols are put in BSSSection, but they are
   // really emitted with the magic .comm directive, which creates a symbol table
   // entry but not a section.
   if (Kind.isBSS() || Kind.isCommon())
     return BSSSection;
 
   return DataSection;
 }
 
+void TargetLoweringObjectFileCOFF::getNameWithPrefix(
+    SmallVectorImpl<char> &OutName, const GlobalValue *GV,
+    bool CannotUsePrivateLabel, Mangler &Mang, const TargetMachine &TM) const {
+  if (GV->hasPrivateLinkage() &&
+      ((isa<Function>(GV) && TM.getFunctionSections()) ||
+       (isa<GlobalVariable>(GV) && TM.getDataSections()))) {
+    SmallString<256> Tmp;
+    Mang.getNameWithPrefix(Tmp, GV, CannotUsePrivateLabel, /*ForceNonPrivate=*/true);
+    OutName.append(Tmp.begin(), Tmp.end());
+    return;
+  }
+  Mang.getNameWithPrefix(OutName, GV, CannotUsePrivateLabel);
+}
+
 const MCSection *TargetLoweringObjectFileCOFF::getSectionForJumpTable(
     const Function &F, Mangler &Mang, const TargetMachine &TM) const {
   // If the function can be removed, produce a unique section so that
   // the table doesn't prevent the removal.
   const Comdat *C = F.getComdat();
   bool EmitUniqueSection = TM.getFunctionSections() || C;
   if (!EmitUniqueSection)
     return ReadOnlySection;

lib/IR/Mangler.cpp

for (Function::const_arg_iterator AI = F->arg_begin(), AE = F->arg_end();
     unsigned PtrSize = DL.getPointerSize();
     ArgWords += RoundUpToAlignment(DL.getTypeAllocSize(Ty), PtrSize);
   }
 
   OS << '@' << ArgWords;
 }
 
 void Mangler::getNameWithPrefix(raw_ostream &OS, const GlobalValue *GV,
-                                bool CannotUsePrivateLabel) const {
+                                bool CannotUsePrivateLabel,
+                                bool ForceNonPrivate) const {
   ManglerPrefixTy PrefixTy = Mangler::Default;
-  if (GV->hasPrivateLinkage()) {
+  if (GV->hasPrivateLinkage() && !ForceNonPrivate) {
     if (CannotUsePrivateLabel)
       PrefixTy = Mangler::LinkerPrivate;
     else
       PrefixTy = Mangler::Private;
   }
 
   if (!GV->hasName()) {
     // Get the ID for the global, assigning a new one if we haven't got one
if (hasByteCountSuffix(CC) &&
       // "Pure" variadic functions do not receive @0 suffix.
       (!FT->isVarArg() || FT->getNumParams() == 0 ||
        (FT->getNumParams() == 1 && MSFunc->hasStructRetAttr())))
     addByteCountSuffix(OS, MSFunc, *DL);
 }
 
 void Mangler::getNameWithPrefix(SmallVectorImpl<char> &OutName,
                                 const GlobalValue *GV,
-                                bool CannotUsePrivateLabel) const {
+                                bool CannotUsePrivateLabel,
+                                bool ForceNonPrivate) const {
   raw_svector_ostream OS(OutName);
-  getNameWithPrefix(OS, GV, CannotUsePrivateLabel);
+  getNameWithPrefix(OS, GV, CannotUsePrivateLabel, ForceNonPrivate);
 }

lib/Target/TargetLoweringObjectFile.cpp

getTTypeReference(const MCSymbolRefExpr *Sym, unsigned Encoding,
   }
 }
 
 const MCExpr *TargetLoweringObjectFile::getDebugThreadLocalSymbol(const MCSymbol *Sym) const {
   // FIXME: It's not clear what, if any, default this should have - perhaps a
   // null return could mean 'no location' & we should just do that here.
   return MCSymbolRefExpr::Create(Sym, *Ctx);
 }
+
+void TargetLoweringObjectFile::getNameWithPrefix(
+    SmallVectorImpl<char> &OutName, const GlobalValue *GV,
+    bool CannotUsePrivateLabel, Mangler &Mang, const TargetMachine &TM) const {
+  Mang.getNameWithPrefix(OutName, GV, CannotUsePrivateLabel);
+}

lib/Target/TargetMachine.cpp

if (MayAlwaysUsePrivate || !GV->hasPrivateLinkage()) {
     // private labels are legal in this case or not.
     Mang.getNameWithPrefix(Name, GV, false);
     return;
   }
   SectionKind GVKind = TargetLoweringObjectFile::getKindForGlobal(GV, *this);
   const TargetLoweringObjectFile *TLOF = getObjFileLowering();
   const MCSection *TheSection = TLOF->SectionForGlobal(GV, GVKind, Mang, *this);
   bool CannotUsePrivateLabel = !canUsePrivateLabel(*AsmInfo, *TheSection);
-  Mang.getNameWithPrefix(Name, GV, CannotUsePrivateLabel);
+  TLOF->getNameWithPrefix(Name, GV, CannotUsePrivateLabel, Mang, *this);
 }
 
 MCSymbol *TargetMachine::getSymbol(const GlobalValue *GV, Mangler &Mang) const {
   SmallString<60> NameStr;
   getNameWithPrefix(NameStr, GV, Mang);
   const TargetLoweringObjectFile *TLOF = getObjFileLowering();
   return TLOF->getContext().GetOrCreateSymbol(NameStr.str());
 }

test/CodeGen/X86/global-sections.ll

 @G14 = private unnamed_addr constant [4 x i8] c"foo\00", align 1
 
 ; LINUX-SECTIONS:        .type   .LG14,@object           # @G14
 ; LINUX-SECTIONS:        .section        .rodata.str1.1,"aMS",@progbits,1
 ; LINUX-SECTIONS: .LG14:
 ; LINUX-SECTIONS:        .asciz  "foo"
 ; LINUX-SECTIONS:        .size   .LG14, 4
 
-; WIN32-SECTIONS:        .section        .rdata,"dr"
-; WIN32-SECTIONS: L_G14:
+; WIN32-SECTIONS:        .section        .rdata,"dr",one_only,_G14
+; WIN32-SECTIONS: _G14:
 ; WIN32-SECTIONS:        .asciz  "foo"
 
 ; cannot be merged on MachO, but can on other formats.
 @G15 = unnamed_addr constant i64 0
 
 ; LINUX: .section        .rodata.cst8,"aM",@progbits,8
 ; LINUX: G15: