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

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

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

public:
   void EmitValueToAlignment(unsigned, int64_t, unsigned, unsigned) override;
 
   void FinishImpl() override;
 
   void EmitBundleAlignMode(unsigned AlignPow2) override;
   void EmitBundleLock(bool AlignToEnd) override;
   void EmitBundleUnlock() override;
 
+  MCSection *getNamedGroupSection(StringRef Section, StringRef Identifer,
+                                  unsigned EntSize) override;
+
 private:
   bool isBundleLocked() const;
   void EmitInstToFragment(const MCInst &Inst, const MCSubtargetInfo &) override;
   void EmitInstToData(const MCInst &Inst, const MCSubtargetInfo &) override;
 
   void fixSymbolsInTLSFixups(const MCExpr *expr);
 
   /// \brief Merge the content of the fragment \p EF into the fragment \p DF.

include/llvm/MC/MCStreamer.h

public:
   /// Get the .pdata section used for the given section. Typically the given
   /// section is either the main .text section or some other COMDAT .text
   /// section, but it may be any section containing code.
   MCSection *getAssociatedPDataSection(const MCSection *TextSec);
 
   /// Get the .xdata section used for the given section.
   MCSection *getAssociatedXDataSection(const MCSection *TextSec);
 
+  /// Get a group section with provided group identifier. This is in ELF, for
+  /// instance, named by concatenating \p Section with '.' then \p Identifier.
+  virtual MCSection *getNamedGroupSection(StringRef Section,
+                                          StringRef Identifier,
+                                          unsigned EntSize);
+
   virtual void EmitSyntaxDirective();
 
   /// \brief Emit a .reloc directive.
   /// Returns true if the relocation could not be emitted because Name is not
   /// known.
   virtual bool EmitRelocDirective(const MCExpr &Offset, StringRef Name,
                                   const MCExpr *Expr, SMLoc Loc) {
     return true;

lib/MC/MCELFStreamer.cpp

void MCELFStreamer::EmitZerofill(MCSection *Section, MCSymbol *Symbol,
                                  uint64_t Size, unsigned ByteAlignment) {
   llvm_unreachable("ELF doesn't support this directive");
 }
 
 void MCELFStreamer::EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
                                    uint64_t Size, unsigned ByteAlignment) {
   llvm_unreachable("ELF doesn't support this directive");
 }
+
+MCSection *MCELFStreamer::getNamedGroupSection(StringRef Section,
+                                               StringRef Identifier,
+                                               unsigned EntSize) {
+  return getContext().getELFSection(Section, ELF::SHT_PROGBITS,
+                                    ELF::SHF_ALLOC | ELF::SHF_GROUP, EntSize,
+                                    Identifier);
+}

lib/MC/MCStreamer.cpp

MCSymbol *MCStreamer::endSection(MCSection *Section) {
   MCSymbol *Sym = Section->getEndSymbol(Context);
   if (Sym->isInSection())
     return Sym;
 
   SwitchSection(Section);
   EmitLabel(Sym);
   return Sym;
 }
+
+MCSection *MCStreamer::getNamedGroupSection(StringRef, StringRef, unsigned) {
+  llvm_unreachable("Unimplemented.");
+  return nullptr;
+}