[NFC][PPC] Refactor TOC representation to allow several entries for the same symbol

[NeHuang]

Summary

We currently represent TOC entries by an MCSymbol. This is not
enough in some situations. For example, when accessing an
initialized TLS variable v on AIX using the general dynamic model, we
need to generate the two following entries for v:

.tc .v[TC],v@m
.tc v[TC],v

One is for the region handle (with the @m relocation), the other is for
the variable offset.
This refactoring allows storing several entries for the same symbol
with different VariantKind in the TOC. If the VariantKind is not
specified, we default to VK_None.

The AIX TLS implementation using this refactoring to generate the two
entries will be posted in a subsequent patch.

[amyk]

llvm/lib/MC/MCExpr.cpp

225

nit: Address the clang format.

Also, sorry, this might be a silly question. What is the difference between VK_None and VK_Empty?

[bsaleil]

llvm/lib/MC/MCExpr.cpp

225

You can see in MCExpr.h that VK_Empty is used to represent an empty key in a DenseMap. If I understand correctly how DenseMap works, when inserting a value, if there is a collision, it looks for a slot with an empty key to put the value. So if we use VK_None as the empty key, this prevents us from actually inserting the VK_None value in the table, because it may be overwritten by a later insertion.

[sfertile]

llvm/include/llvm/MC/MCExpr.h

421

I'm not sure why we need this, or even why this works ... IIUC MapVector creates a dense map from the key type to the index of the value. Shouldn't we be creating a DenseMapInfo of the key type which is now a std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind>?

[bsaleil]

llvm/include/llvm/MC/MCExpr.h

421

There is a generic implementation of DenseMapInfo<std::pair<T, U>> in DenseMapInfo.h which relies on DenseMapInfo<T> and DenseMapInfo<U>.

[sfertile]

llvm/include/llvm/MC/MCExpr.h

421

Thanks Baptiste. I'm a little hesitant to add a new value to the variant kind enum that only makes sense as a special value in a dense map. Would you consider creating the DenseMapInfo for the std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind> and not using the generic implementation of DenseMapInfo<std::pair<T, U>>? I know it is ugly, but it will be relegated to PPCAsmPrinter.cpp and we can avoid adding a new value to the VariantKinds.

[bsaleil]

Specialize DenseMapInfo directly with the PPC TOC key type instead of VariantKind.

[bsaleil]

llvm/include/llvm/MC/MCExpr.h

421

Sure, I just updated the patch to make that change.

[sfertile]

Thanks for the update. LGTM.

[jankratochvil]

Unfortunately it fails to build with GCC which LLVM still tries to stay compatible with:
http://lab.llvm.org:8014/#/builders/16/builds/2211
gcc-11.0.0-0.19.fc34.x86_64
gcc-10.2.1-9.fc33.x86_64
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:86:8: error: explicit specialization of 'template<class T> struct llvm::DenseMapInfo' outside its namespace must use a nested-name-specifier [-fpermissive]

86 | struct DenseMapInfo<std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind>> {
   |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I have filed it to an existing GCC bugreport: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92598

[dyung]

In D96346#2567215, @jankratochvil wrote:

Unfortunately it fails to build with GCC which LLVM still tries to stay compatible with:
http://lab.llvm.org:8014/#/builders/16/builds/2211
gcc-11.0.0-0.19.fc34.x86_64
gcc-10.2.1-9.fc33.x86_64
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:86:8: error: explicit specialization of 'template<class T> struct llvm::DenseMapInfo' outside its namespace must use a nested-name-specifier [-fpermissive]

86 | struct DenseMapInfo<std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind>> {
   |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I have filed it to an existing GCC bugreport: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92598

I have committed 0e3d7e61867d69721b28e557272bdf4b66010327 in order to unblock the build on gcc builders.

llvm/include/llvm/MC/MCExpr.h

 /// A symbol reference in an expression may be a use of a label, a use of an
 /// assembler variable (defined constant), or constitute an implicit definition
 /// of the symbol as external.
 class MCSymbolRefExpr : public MCExpr {
 public:
   enum VariantKind : uint16_t {
     VK_None,
     VK_Invalid,
+    VK_Empty,
 
     VK_GOT,
     VK_GOTOFF,
     VK_GOTREL,
     VK_PCREL,
     VK_GOTPCREL,
     VK_GOTTPOFF,
     VK_INDNTPOFF,
public:
 
   /// @}
 
   static bool classof(const MCExpr *E) {
     return E->getKind() == MCExpr::SymbolRef;
   }
 };
 
+// Specialize DenseMapInfo to allow MCSymbolRefExpr::VariantKind in DenseMap
+template <> struct DenseMapInfo<MCSymbolRefExpr::VariantKind> {
+  static inline MCSymbolRefExpr::VariantKind getEmptyKey() {
+    return MCSymbolRefExpr::VariantKind::VK_Empty;
+  }
+  static inline MCSymbolRefExpr::VariantKind getTombstoneKey() {
+    return MCSymbolRefExpr::VariantKind::VK_Invalid;
+  }
+  static unsigned getHashValue(const MCSymbolRefExpr::VariantKind &V) {
+    return DenseMapInfo<int>::getHashValue(V);
+  }
+  static bool isEqual(const MCSymbolRefExpr::VariantKind &A,
+                      const MCSymbolRefExpr::VariantKind &B) {
+    return A == B;
+  }
+};
+
 /// Unary assembler expressions.
 class MCUnaryExpr : public MCExpr {
 public:
   enum Opcode {
     LNot,  ///< Logical negation.
     Minus, ///< Unary minus.
     Not,   ///< Bitwise negation.
     Plus   ///< Unary plus.

llvm/lib/MC/MCExpr.cpp

const MCSymbolRefExpr *MCSymbolRefExpr::create(StringRef Name, VariantKind Kind,
                                                MCContext &Ctx) {
   return create(Ctx.getOrCreateSymbol(Name), Kind, Ctx);
 }
 
 StringRef MCSymbolRefExpr::getVariantKindName(VariantKind Kind) {
   switch (Kind) {
   case VK_Invalid: return "<<invalid>>";
   case VK_None: return "<<none>>";
+  case VK_Empty: return "<<empty>>";
 
   case VK_DTPOFF: return "DTPOFF";
   case VK_DTPREL: return "DTPREL";
   case VK_GOT: return "GOT";
   case VK_GOTOFF: return "GOTOFF";
   case VK_GOTREL: return "GOTREL";
   case VK_PCREL: return "PCREL";
   case VK_GOTPCREL: return "GOTPCREL";

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

 using namespace llvm::XCOFF;
 
 #define DEBUG_TYPE "asmprinter"
 
 namespace {
 
 class PPCAsmPrinter : public AsmPrinter {
 protected:
-  MapVector<const MCSymbol *, MCSymbol *> TOC;
+  // For TLS on AIX, we need to be able to identify TOC entries of specific
+  // VariantKind so we can add the right relocations when we generate the
+  // entries. So each entry is represented by a pair of MCSymbol and
+  // VariantKind. For example, we need to be able to identify the following
+  // entry as a TLSGD entry so we can add the @m relocation:
+  //   .tc .i[TC],i[TL]@m
+  // By default, VK_None is used for the VariantKind.
+  MapVector<std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind>,
+            MCSymbol *>
+      TOC;
   const PPCSubtarget *Subtarget = nullptr;
   StackMaps SM;
 
 public:
   explicit PPCAsmPrinter(TargetMachine &TM,
                          std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)), SM(*this) {}
 
   StringRef getPassName() const override { return "PowerPC Assembly Printer"; }
 
-  MCSymbol *lookUpOrCreateTOCEntry(const MCSymbol *Sym);
+  MCSymbol *lookUpOrCreateTOCEntry(const MCSymbol *Sym,
+                                   MCSymbolRefExpr::VariantKind Kind =
+                                       MCSymbolRefExpr::VariantKind::VK_None);
 
   bool doInitialization(Module &M) override {
     if (!TOC.empty())
       TOC.clear();
     return AsmPrinter::doInitialization(M);
   }
 
   void emitInstruction(const MachineInstr *MI) override;
bool PPCAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
   printOperand(MI, OpNo, O);
   O << ")";
   return false;
 }
 
 /// lookUpOrCreateTOCEntry -- Given a symbol, look up whether a TOC entry
 /// exists for it.  If not, create one.  Then return a symbol that references
 /// the TOC entry.
-MCSymbol *PPCAsmPrinter::lookUpOrCreateTOCEntry(const MCSymbol *Sym) {
-  MCSymbol *&TOCEntry = TOC[Sym];
+MCSymbol *
+PPCAsmPrinter::lookUpOrCreateTOCEntry(const MCSymbol *Sym,
+                                      MCSymbolRefExpr::VariantKind Kind) {
+  MCSymbol *&TOCEntry = TOC[{Sym, Kind}];
   if (!TOCEntry)
     TOCEntry = createTempSymbol("C");
   return TOCEntry;
 }
 
 void PPCAsmPrinter::emitEndOfAsmFile(Module &M) {
   emitStackMaps(SM);
 }
auto getTOCRelocAdjustedExprForXCOFF = [this](const MCExpr *Expr,
     return MCBinaryExpr::createAdd(
         Expr, MCConstantExpr::create(-Adjustment, OutContext), OutContext);
   };
 
   auto getTOCEntryLoadingExprForXCOFF =
       [IsPPC64, getTOCRelocAdjustedExprForXCOFF,
        this](const MCSymbol *MOSymbol, const MCExpr *Expr) -> const MCExpr * {
     const unsigned EntryByteSize = IsPPC64 ? 8 : 4;
-    const auto TOCEntryIter = TOC.find(MOSymbol);
+    const auto TOCEntryIter =
+        TOC.find({MOSymbol, MCSymbolRefExpr::VariantKind::VK_None});
     assert(TOCEntryIter != TOC.end() &&
            "Could not find the TOC entry for this symbol.");
     const ptrdiff_t EntryDistanceFromTOCBase =
         (TOCEntryIter - TOC.begin()) * EntryByteSize;
     constexpr int16_t PositiveTOCRange = INT16_MAX;
 
     if (EntryDistanceFromTOCBase > PositiveTOCRange)
       return getTOCRelocAdjustedExprForXCOFF(Expr, EntryDistanceFromTOCBase);
if (!TOC.empty()) {
     const char *Name = isPPC64 ? ".toc" : ".got2";
     MCSectionELF *Section = OutContext.getELFSection(
         Name, ELF::SHT_PROGBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
     OutStreamer->SwitchSection(Section);
     if (!isPPC64)
       OutStreamer->emitValueToAlignment(4);
 
     for (const auto &TOCMapPair : TOC) {
-      const MCSymbol *const TOCEntryTarget = TOCMapPair.first;
+      const MCSymbol *const TOCEntryTarget = TOCMapPair.first.first;
       MCSymbol *const TOCEntryLabel = TOCMapPair.second;
 
       OutStreamer->emitLabel(TOCEntryLabel);
       if (isPPC64 && TS != nullptr)
         TS->emitTCEntry(*TOCEntryTarget);
       else
         OutStreamer->emitSymbolValue(TOCEntryTarget, 4);
     }
void PPCAIXAsmPrinter::emitEndOfAsmFile(Module &M) {
   OutStreamer->SwitchSection(getObjFileLowering().getTOCBaseSection());
 
   PPCTargetStreamer *TS =
       static_cast<PPCTargetStreamer *>(OutStreamer->getTargetStreamer());
 
   for (auto &I : TOC) {
     // Setup the csect for the current TC entry.
     MCSectionXCOFF *TCEntry = cast<MCSectionXCOFF>(
-        getObjFileLowering().getSectionForTOCEntry(I.first, TM));
+        getObjFileLowering().getSectionForTOCEntry(I.first.first, TM));
     OutStreamer->SwitchSection(TCEntry);
 
     OutStreamer->emitLabel(I.second);
     if (TS != nullptr)
-      TS->emitTCEntry(*I.first);
+      TS->emitTCEntry(*I.first.first);
   }
 }
 
 bool PPCAIXAsmPrinter::doInitialization(Module &M) {
   const bool Result = PPCAsmPrinter::doInitialization(M);
 
   auto setCsectAlignment = [this](const GlobalObject *GO) {
     // Declarations have 0 alignment which is set by default.