Convenience function for MCContext to get named sections when lowering.

[dberris]

Summary

This change allows us to create uniquely identified "COMDAT" or "ELF
Group" sections while lowering. In particular, for ELF sections this is
useful for creating function-specific groups that get merged into the
same named section.

Also use const Twine& instead of StringRef for the getELF functions
while we're here.

[majnemer]

Reid, didn't you add something like this for COFF?

Sent from my phone

[echristo]

My worry from looking at it briefly is that most of the uses of SHF_GROUP have additional flags that they want or'd into the section. Do you have an example refactor for the existing cases that we can use here?

Thanks!

-eric

[dberris]

I spent some time trying to find existing cases where this might be helpful, but I've yet to find suitable ones. Granted I've only looked at lib/Target/X86/*.

Most of the cases where this comes up are pretty straight-forward, using functions from TargetLoweringObjectFile to get the section for globals, etc. -- special section creation isn't very well supported through that interface either.

Extracting the flags might be doable but bringing ELF'isms out when lowering might be a little much.

Any suggestions on where else to look for cases where this interface (or an evolution of it) might be useful?

[dberris]

I've found the answer -- it seems this could be useful in TargetLoweringObjectFile and the various implementations there.

Let me update this to refactor some of the insides of TargetLoweringObjectFile to use this new interface.

[dberris]

• Refactor one call in TargetLoweringObjectFileImpl to use new API

[rnk]

In D21743#468102, @majnemer wrote:

Reid, didn't you add something like this for COFF?

MCContext::getAssociativeCOFFSection, which doesn't do this name appending stuff because it wastes object file space.

This patch is NFC, and that's fine, but at a high level, should we continue to name our sections things like .text._Z3foov (ala GCC), or should we be shortening to just .text? I thought Rafael did that at one point?

include/llvm/MC/MCStreamer.h

767 ↗

(On Diff #62181)

From reading the doc comments, this looks very ELF-specific. Maybe this should be getELFSectionInGroup? See a similar method getAssociativeCOFFSection.

[• rafael]

lib/MC/MCAsmStreamer.cpp

1576 ↗

(On Diff #62181)

I don't think this should be on the streamer, as the ELF and Asm implementations would be identical. This is just a convenience functions and should be implemented once in MCContext.

[rnk]

lib/MC/MCAsmStreamer.cpp

1576 ↗

(On Diff #62181)

+1, it would live along with the other getELF*, getCOFF* etc methods. The XData and PData stuff lives here for bad reasons.

[dberris]

• Move API from MCStreamer to MCContext

[dberris]

Thanks -- I've moved this to MCContext instead.

include/llvm/MC/MCStreamer.h

767 ↗

(On Diff #62181)

Correct -- renamed to 'getELFNamedSection' instead?

[• rafael]

Much better. It should at least rename Section and Group to just Prefix and Suffix, since there nothing specific about groups.

Having mentioned that, would it be too expensive to make the existing getELFSection overloads take a Twine instead of a StringRef?

[dberris]

• Rename "Section" -> "Prefix", "Group" -> "Suffix"

[dberris]

I'm not too sure about the change to use Twine instead of StringRef, but I'm happy to do do it as a separate change to see how expensive it could become.

[dberris]

Actually, having read a little about Twine, I'm convinced it's worth doing. I'll work on refactoring the other "getELF*" functions at least to use Twine instead of StringRef.

[dberris]

• Refactor getELF* functions to use const Twine& instead of StringRef

[dberris]

Suggested refactoring to use Twine instead of StringRef now done.

[• rafael]

LGTM. If the calls to str() are ever a problem we can replace them with the slightly more verbose toStringRef.

[dberris]

Thanks Rafael -- I don't have commit powers, anybody care to land on my behalf?

Thanks in advance!

[echristo]

Committed thusly:

M lib/MC/MCContext.cpp
M lib/CodeGen/TargetLoweringObjectFileImpl.cpp
M include/llvm/MC/MCContext.h
r274336 = e2f8e9e22b5937d691814657012bc0997a3380a4
(refs/remotes/origin/master)

-eric

include/llvm/MC/MCContext.h

MCSectionELF *getELFSection(StringRef Section, unsigned Type,
                                 const char *BeginSymName);
 
     MCSectionELF *getELFSection(StringRef Section, unsigned Type,
                                 unsigned Flags, unsigned EntrySize,
                                 const MCSymbolELF *Group, unsigned UniqueID,
                                 const char *BeginSymName,
                                 const MCSectionELF *Associated);
 
+    /// Get a section with the provided group identifier. This section is
+    /// named by concatenating \p Section with '.' then \p Group. The \p Type
+    /// describes the type of the section and \p Flags are used to further
+    /// configure this named section.
+    MCSectionELF *getELFNamedSection(StringRef Section, StringRef Group,
+                                     unsigned Type, unsigned Flags,
+                                     unsigned EntrySize = 0);
+
     MCSectionELF *createELFRelSection(StringRef Name, unsigned Type,
                                       unsigned Flags, unsigned EntrySize,
                                       const MCSymbolELF *Group,
                                       const MCSectionELF *Associated);
 
     void renameELFSection(MCSectionELF *Section, StringRef Name);
 
     MCSectionELF *createELFGroupSection(const MCSymbolELF *Group);

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

 void TargetLoweringObjectFileELF::emitPersonalityValue(
     MCStreamer &Streamer, const DataLayout &DL, const MCSymbol *Sym) const {
   SmallString<64> NameData("DW.ref.");
   NameData += Sym->getName();
   MCSymbolELF *Label =
       cast<MCSymbolELF>(getContext().getOrCreateSymbol(NameData));
   Streamer.EmitSymbolAttribute(Label, MCSA_Hidden);
   Streamer.EmitSymbolAttribute(Label, MCSA_Weak);
-  StringRef Prefix = ".data.";
-  NameData.insert(NameData.begin(), Prefix.begin(), Prefix.end());
   unsigned Flags = ELF::SHF_ALLOC | ELF::SHF_WRITE | ELF::SHF_GROUP;
-  MCSection *Sec = getContext().getELFSection(NameData, ELF::SHT_PROGBITS,
-                                              Flags, 0, Label->getName());
+  MCSection *Sec = getContext().getELFNamedSection(".data", Label->getName(),
+                                                   ELF::SHT_PROGBITS, Flags, 0);
   unsigned Size = DL.getPointerSize();
   Streamer.SwitchSection(Sec);
   Streamer.EmitValueToAlignment(DL.getPointerABIAlignment());
   Streamer.EmitSymbolAttribute(Label, MCSA_ELF_TypeObject);
   const MCExpr *E = MCConstantExpr::create(Size, getContext());
   Streamer.emitELFSize(Label, E);
   Streamer.EmitLabel(Label);
 

lib/MC/MCContext.cpp

MCSectionELF *MCContext::createELFRelSection(StringRef Name, unsigned Type,
   bool Inserted;
   std::tie(I, Inserted) = ELFRelSecNames.insert(std::make_pair(Name, true));
 
   return new (ELFAllocator.Allocate())
       MCSectionELF(I->getKey(), Type, Flags, SectionKind::getReadOnly(),
                    EntrySize, Group, true, nullptr, Associated);
 }
 
+MCSectionELF *MCContext::getELFNamedSection(StringRef Section, StringRef Group,
+                                            unsigned Type, unsigned Flags,
+                                            unsigned EntrySize) {
+  SmallString<64> FullName(Section);
+  FullName += ".";
+  FullName += Group;
+  return getELFSection(FullName, Type, Flags, EntrySize, Group);
+}
+
 MCSectionELF *MCContext::getELFSection(StringRef Section, unsigned Type,
                                        unsigned Flags, unsigned EntrySize,
                                        StringRef Group, unsigned UniqueID,
                                        const char *BeginSymName) {
   MCSymbolELF *GroupSym = nullptr;
   if (!Group.empty())
     GroupSym = cast<MCSymbolELF>(getOrCreateSymbol(Group));