[AIX][MC][NFC] Explicitly set containing csects on XCOFF Symbols

[sfertile]

Summary

• Add a setter and getter for the containing csect of a symbol
• In setting containing csect, don't set twice. Also assert if trying to set a different section.
• Set containing csect explicitly for globals
• Map the TOC Anchor symbol to it's containing csect
• use getContainingCSect() to lookup csect rather than the fragment in XCOFFObjectWriter

[jasonliu]

llvm/lib/MC/MCSymbolXCOFF.cpp

14 ↗

(On Diff #214449)

Is it necessary to put these two functions in the .cpp file?
These two functions doesn't look complicated, and should be Okay to exist in the header.

[sfertile]

LGTM. (After either responding to or addressing Jason comment)

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1688

minor nit: 'Csect' --> 'csect'

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1688

Minor nits:

• s/it's/its/;
• Move "it" closer to its referent (the TOC anchor) by swapping the order of the first sentence: Since [ ... ], map [ ... ].

1689

That way, other methods can freely query it, assuming that it is set.

[daltenty]

llvm/lib/MC/MCSymbolXCOFF.cpp

14 ↗

(On Diff #214449)

I will move these to the header in the commit

[sfertile]

Commandeering this.

[sfertile]

We were stretching the definition of an NFC patch by creating but not using the TOC-base on AIX. I've striped this down to the NFC part and will post a separate patch for the TOC base change that includes a test change to reflect the TOC-base being created.

[hubert.reinterpretcast]

LGTM.

llvm/include/llvm/MC/MCSymbolXCOFF.h

40

s/contain/containing/;

llvm/include/llvm/MC/MCSymbolXCOFF.h

 //===- MCSymbolXCOFF.h -  ----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_MC_MCSYMBOLXCOFF_H
 #define LLVM_MC_MCSYMBOLXCOFF_H
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/BinaryFormat/XCOFF.h"
 #include "llvm/MC/MCSymbol.h"
 
 namespace llvm {
 
+class MCSectionXCOFF;
+
 class MCSymbolXCOFF : public MCSymbol {
 public:
   MCSymbolXCOFF(const StringMapEntry<bool> *Name, bool isTemporary)
       : MCSymbol(SymbolKindXCOFF, Name, isTemporary) {}
 
   static bool classof(const MCSymbol *S) { return S->isXCOFF(); }
 
   void setStorageClass(XCOFF::StorageClass SC) {
     assert((!StorageClass.hasValue() || StorageClass.getValue() == SC) &&
            "Redefining StorageClass of XCOFF MCSymbol.");
     StorageClass = SC;
   };
 
   XCOFF::StorageClass getStorageClass() const {
     assert(StorageClass.hasValue() &&
            "StorageClass not set on XCOFF MCSymbol.");
     return StorageClass.getValue();
   }
 
+  void setContainingCsect(const MCSectionXCOFF *C) {
+    assert((!ContainingCsect || ContainingCsect == C) &&
+           "Trying to set a contain csect that doesn't match the one that this "
+           "symbol is already mapped to.");
+    ContainingCsect = C;
+  }
+
+  const MCSectionXCOFF *getContainingCsect() const {
+    assert(ContainingCsect &&
+           "Trying to get containing csect but none was set.");
+    return ContainingCsect;
+  }
+
 private:
   Optional<XCOFF::StorageClass> StorageClass;
+  const MCSectionXCOFF *ContainingCsect = nullptr;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCSYMBOLXCOFF_H

llvm/lib/MC/XCOFFObjectWriter.cpp

void XCOFFObjectWriter::executePostLayoutBinding(
 
   for (const MCSymbol &S : Asm.symbols()) {
     // Nothing to do for temporary symbols.
     if (S.isTemporary())
       continue;
     const MCSymbolXCOFF *XSym = cast<MCSymbolXCOFF>(&S);
 
     // Map the symbol into its containing csect.
-    MCSectionXCOFF *ContainingCsect =
-        dyn_cast<MCSectionXCOFF>(XSym->getFragment(false)->getParent());
+    const MCSectionXCOFF *ContainingCsect = XSym->getContainingCsect();
     assert(WrapperMap.find(ContainingCsect) != WrapperMap.end() &&
            "Expected containing csect to exist in map");
 
     // Lookup the containing csect and add the symbol to it.
     WrapperMap[ContainingCsect]->Syms.emplace_back(XSym);
 
     // If the name does not fit in the storage provided in the symbol table
     // entry, add it to the string table.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

void PPCAIXAsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
   MCSectionXCOFF *CSect = cast<MCSectionXCOFF>(
       getObjFileLowering().SectionForGlobal(GV, GVKind, TM));
   OutStreamer->SwitchSection(CSect);
 
   // Create the symbol, set its storage class, and emit it.
   MCSymbolXCOFF *XSym = cast<MCSymbolXCOFF>(getSymbol(GV));
   XSym->setStorageClass(
       TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GV));
+  XSym->setContainingCsect(CSect);
+
   const DataLayout &DL = GV->getParent()->getDataLayout();
   unsigned Align =
       GV->getAlignment() ? GV->getAlignment() : DL.getPreferredAlignment(GV);
   uint64_t Size = DL.getTypeAllocSize(GV->getType()->getElementType());
 
   if (GVKind.isBSSLocal())
     OutStreamer->EmitXCOFFLocalCommonSymbol(XSym, Size, Align);
   else
     OutStreamer->EmitCommonSymbol(XSym, Size, Align);
 }
 
 /// createPPCAsmPrinterPass - Returns a pass that prints the PPC assembly code
 /// for a MachineFunction to the given output stream, in a format that the
 /// Darwin assembler can deal with.
 ///
 static AsmPrinter *
 createPPCAsmPrinterPass(TargetMachine &tm,
                         std::unique_ptr<MCStreamer> &&Streamer) {
   if (tm.getTargetTriple().isMacOSX())
     return new PPCDarwinAsmPrinter(tm, std::move(Streamer));
   if (tm.getTargetTriple().isOSAIX())
     return new PPCAIXAsmPrinter(tm, std::move(Streamer));