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

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

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

Done.

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

13

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/DataLayout.h

 /// target being codegen'd to.
 class DataLayout {
 private:
   /// Defaults to false.
   bool BigEndian;
 
   unsigned StackNaturalAlign;
 
-  enum ManglingModeT { MM_None, MM_ELF, MM_MachO, MM_WINCOFF, MM_Mips };
+  enum ManglingModeT {
+    MM_None,
+    MM_ELF,
+    MM_MachO,
+    MM_WinCOFF,
+    MM_WinCOFFX86,
+    MM_Mips
+  };
   ManglingModeT ManglingMode;
 
   SmallVector<unsigned char, 8> LegalIntWidths;
 
   /// \brief Primitive type alignment data.
   SmallVector<LayoutAlignElem, 16> Alignments;
 
   /// \brief The string representation used to create this DataLayout
public:
   /// Returns true if the given alignment exceeds the natural stack alignment.
   bool exceedsNaturalStackAlignment(unsigned Align) const {
     return (StackNaturalAlign != 0) && (Align > StackNaturalAlign);
   }
 
   unsigned getStackAlignment() const { return StackNaturalAlign; }
 
   bool hasMicrosoftFastStdCallMangling() const {
-    return ManglingMode == MM_WINCOFF;
+    return ManglingMode == MM_WinCOFFX86;
   }
 
   bool hasLinkerPrivateGlobalPrefix() const { return ManglingMode == MM_MachO; }
 
   const char *getLinkerPrivateGlobalPrefix() const {
     if (ManglingMode == MM_MachO)
       return "l";
+    if (ManglingMode == MM_WinCOFF || ManglingMode == MM_WinCOFFX86)
+      return "";
     return getPrivateGlobalPrefix();
   }
 
   char getGlobalPrefix() const {
     switch (ManglingMode) {
     case MM_None:
     case MM_ELF:
     case MM_Mips:
+    case MM_WinCOFF:
       return '\0';
     case MM_MachO:
-    case MM_WINCOFF:
+    case MM_WinCOFFX86:
       return '_';
     }
     llvm_unreachable("invalid mangling mode");
   }
 
   const char *getPrivateGlobalPrefix() const {
     switch (ManglingMode) {
     case MM_None:
       return "";
     case MM_ELF:
       return ".L";
     case MM_Mips:
       return "$";
     case MM_MachO:
-    case MM_WINCOFF:
+    case MM_WinCOFF:
+    case MM_WinCOFFX86:
       return "L";
     }
     llvm_unreachable("invalid mangling mode");
   }
 
   static const char *getManglingComponent(const Triple &T);
 
   /// \brief Returns true if the specified type fits in a native integer type

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, /*CannotUsePrivateLabel=*/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())))
+    CannotUsePrivateLabel = true;
+
+  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/DataLayout.cpp

 
 //===----------------------------------------------------------------------===//
 //                       DataLayout Class Implementation
 //===----------------------------------------------------------------------===//
 
 const char *DataLayout::getManglingComponent(const Triple &T) {
   if (T.isOSBinFormatMachO())
     return "-m:o";
-  if (T.isOSWindows() && T.getArch() == Triple::x86 && T.isOSBinFormatCOFF())
-    return "-m:w";
+  if (T.isOSWindows() && T.isOSBinFormatCOFF())
+    return T.getArch() == Triple::x86 ? "-m:x" : "-m:w";
   return "-m:e";
 }
 
 static const LayoutAlignElem DefaultAlignments[] = {
   { INTEGER_ALIGN, 1, 1, 1 },    // i1
   { INTEGER_ALIGN, 8, 1, 1 },    // i8
   { INTEGER_ALIGN, 16, 2, 2 },   // i16
   { INTEGER_ALIGN, 32, 4, 4 },   // i32
case 'm':
         break;
       case 'o':
         ManglingMode = MM_MachO;
         break;
       case 'm':
         ManglingMode = MM_Mips;
         break;
       case 'w':
-        ManglingMode = MM_WINCOFF;
+        ManglingMode = MM_WinCOFF;
+        break;
+      case 'x':
+        ManglingMode = MM_WinCOFFX86;
         break;
       }
       break;
     default:
       report_fatal_error("Unknown specifier in datalayout string");
       break;
     }
   }

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/ARM/Windows/long-calls.ll

 ; RUN: llc -mtriple=thumbv7-windows -mcpu=cortex-a9 -arm-long-calls -o - %s \
 ; RUN:    | FileCheck %s
 
 declare arm_aapcs_vfpcc void @callee()
 
 define arm_aapcs_vfpcc void @caller() nounwind {
 entry:
   tail call void @callee()
   ret void
 }
 
 ; CHECK-LABEL: caller
-; CHECK: ldr [[REG:r[0-9]+]], [[CPI:.LCPI[_0-9]+]]
+; CHECK: ldr [[REG:r[0-9]+]], [[CPI:LCPI[_0-9]+]]
 ; CHECK: bx [[REG]]
 ; CHECK: .align 2
 ; CHECK: [[CPI]]:
 ; CHECK: .long callee
 

test/CodeGen/X86/fastcall-correct-mangling.ll

 ; CHECK64-LABEL: {{^}}DoNotMangle:
 entry:
   ret i32 %a
 }
 
 define private x86_fastcallcc void @dontCrash() {
 ; The name is fairly arbitrary since it is private. Just don't crash.
 ; CHECK32-LABEL: {{^}}L@dontCrash@0:
-; CHECK64-LABEL: {{^}}.LdontCrash:
+; CHECK64-LABEL: {{^}}LdontCrash:
   ret void
 }

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: