Enable assembly output of local commons for AIX

[daltenty]

Summary

This patch enable assembly output of local commons for AIX using .lcomm
directives. Adds a EmitXCOFFLocalCommonSymbol to MCStreamer so we can emit the
AIX version of .lcomm assembly directives which include a csect name. Handle the
case of BSS locals in PPCAIXAsmPrinter by using EmitXCOFFLocalCommonSymbol. Adds
a test for generating .lcomm on AIX Targets.

[sfertile]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1829 ↗

(On Diff #210190)

Knowing how the AIX linker treats csects with symbol type XTY_CM, should this be limited to Kind.isLocalBSS() || Kind.isCommon()?

llvm/lib/MC/MCSectionXCOFF.cpp

31 ↗

(On Diff #210190)

ditto.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1662 ↗

(On Diff #210190)

I think this also should be limited to isLocalBSS(), we can relax to allowing all BSS in a follow on patch that adds appropriate testing.

[hubert.reinterpretcast]

llvm/lib/MC/MCSectionXCOFF.cpp

35 ↗

(On Diff #210190)

"Unsupported" is ambiguous.

This patch needs a rebase. During/after the rebase we need to figure out what the llvm_unreachables should be. That is, the message text needs to clearly express the rationale so that the determination of whether llvm_unreachable is the appropriate mechanism can be done.

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1833 ↗

(On Diff #210190)

One or the other of D65159 or this needs to be rebased on the other when one lands.

[sfertile]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1833 ↗

(On Diff #210190)

I'd prefer to get this review moving and commited, then rebase D65159.

[daltenty]

Rebase after commit of dependencies

[daltenty]

Address comments:
  - Limit to local bss

[daltenty]

• Add missing other cases of isBSSLocal
• Update unreachable error message to reflect this is an internal error

[sfertile]

LGTM.

llvm/lib/MC/MCSectionXCOFF.cpp

34 ↗

(On Diff #213998)

minor nit: I think we could shorten common/bss --> '.bss' .

[hubert.reinterpretcast]

llvm/trunk/include/llvm/MC/MCStreamer.h

547

Spacing issue.

llvm/trunk/lib/MC/MCAsmStreamer.cpp

762

Minor nit: Compound adjective "XCOFF-specific" with a hyphen.

768

Minor nit: Have a period at the end of sentences.

769

Minor nit: Capitalize the first word of a sentence.

llvm/trunk/lib/MC/MCSectionXCOFF.cpp

34

I'm not sure that the "unreachables" (in the function currently, not just this one) are buying us enough to warrant the overhead of thinking about them. Generically, the llvm_unreachables in this function are of the type "we are about to do something, and given the check, what we are about to do is not compatible with the input". We believe this to be a error in the internal state of the program that we are not expecting to happen, so assert is okay.

llvm/trunk/lib/MC/MCXCOFFStreamer.cpp

63

"Not implemented" is not a case where llvm_unreachable is appropriate. Either nothing calls this and it might as well be report_fatal_error, or something does call this and it is probably not the caller's fault that this is unimplemented (as it should be report_fatal_error).

65

Missing newline at end of file. Windows editor woes?

llvm/trunk/include/llvm/MC/MCStreamer.h

public:
   /// \param Symbol - Symbol the section relative relocation should point to.
   virtual void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset);
 
   /// Emits a COFF image relative relocation.
   ///
   /// \param Symbol - Symbol the image relative relocation should point to.
   virtual void EmitCOFFImgRel32(MCSymbol const *Symbol, int64_t Offset);
 
+  /// Emits an lcomm directive with XCOFF csect information.
+  ///
+  /// \param Symbol - The symbol we are emiting.
+  /// \param Size - The size of the block of storage.
+  /// \param ByteAlignment - The alignment of the symbol in bytes. Must be a power
+  /// of 2.
+  virtual void EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                             unsigned ByteAlignment);
+
   /// Emit an ELF .size directive.
   ///
   /// This corresponds to an assembler statement such as:
   ///  .size symbol, expression
   virtual void emitELFSize(MCSymbol *Symbol, const MCExpr *Value);
 
   /// Emit an ELF .symver directive.
   ///

llvm/trunk/include/llvm/MC/MCXCOFFStreamer.h

public:
 
   bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
   void EmitZerofill(MCSection *Section, MCSymbol *Symbol = nullptr,
                     uint64_t Size = 0, unsigned ByteAlignment = 0,
                     SMLoc Loc = SMLoc()) override;
   void EmitInstToData(const MCInst &Inst, const MCSubtargetInfo &) override;
+  void EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                                  unsigned ByteAlign) override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCXCOFFSTREAMER_H

llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

 
 MCSection *TargetLoweringObjectFileXCOFF::SelectSectionForGlobal(
     const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const {
   assert(!TM.getFunctionSections() && !TM.getDataSections() &&
          "XCOFF unique sections not yet implemented.");
 
   // Common symbols go into a csect with matching name which will get mapped
   // into the .bss section.
-  if (Kind.isCommon()) {
+  if (Kind.isBSSLocal() || Kind.isCommon()) {
     SmallString<128> Name;
     getNameWithPrefix(Name, GO, TM);
-    return getContext().getXCOFFSection(Name, XCOFF::XMC_RW, XCOFF::XTY_CM,
-                                        Kind, /* BeginSymbolName */ nullptr);
+    return getContext().getXCOFFSection(
+        Name, Kind.isBSSLocal() ? XCOFF::XMC_BS : XCOFF::XMC_RW, XCOFF::XTY_CM,
   }
 
   if (Kind.isText())
     return TextSection;
 
   report_fatal_error("XCOFF other section types not yet implemented.");
 }
 

llvm/trunk/lib/MC/MCAsmInfoXCOFF.cpp

 using namespace llvm;
 
 void MCAsmInfoXCOFF::anchor() {}
 
 MCAsmInfoXCOFF::MCAsmInfoXCOFF() {
   IsLittleEndian = false;
   HasDotTypeDotSizeDirective = false;
   COMMDirectiveAlignmentIsInBytes = false;
+  LCOMMDirectiveAlignmentType = LCOMM::Log2Alignment;
 }

llvm/trunk/lib/MC/MCAsmStreamer.cpp

public:
   void EmitCOFFSymbolStorageClass(int StorageClass) override;
   void EmitCOFFSymbolType(int Type) override;
   void EndCOFFSymbolDef() override;
   void EmitCOFFSafeSEH(MCSymbol const *Symbol) override;
   void EmitCOFFSymbolIndex(MCSymbol const *Symbol) override;
   void EmitCOFFSectionIndex(MCSymbol const *Symbol) override;
   void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset) override;
   void EmitCOFFImgRel32(MCSymbol const *Symbol, int64_t Offset) override;
+  void EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                                  unsigned ByteAlign) override;
   void emitELFSize(MCSymbol *Symbol, const MCExpr *Value) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
 
   /// Emit a local common (.lcomm) symbol.
   ///
   /// @param Symbol - The common symbol to emit.
   /// @param Size - The size of the common symbol.
void MCAsmStreamer::EmitCOFFImgRel32(MCSymbol const *Symbol, int64_t Offset) {
   Symbol->print(OS, MAI);
   if (Offset > 0)
     OS << '+' << Offset;
   else if (Offset < 0)
     OS << '-' << -Offset;
   EmitEOL();
 }
 
+// We need an XCOFF specific version of this directive as the AIX syntax
+// requires a QualName argument identifying the csect name and storage mapping
+// class to appear before the alignment if we are specifying it.
+void MCAsmStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                                               unsigned ByteAlignment) {
+  assert(MAI->getLCOMMDirectiveAlignmentType() == LCOMM::Log2Alignment &&
+         "We only support writing log base-2 alignment format with XCOFF");
+  assert(isPowerOf2_32(ByteAlignment) && "alignment must be a power of 2");
+
+  OS << "\t.lcomm\t";
+  Symbol->print(OS, MAI);
+  OS << ',' << Size;
+  OS << ',' << Symbol->getName();
+  OS << ',' << Log2_32(ByteAlignment);
+
+  EmitEOL();
+}
+
 void MCAsmStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {
   assert(MAI->hasDotTypeDotSizeDirective());
   OS << "\t.size\t";
   Symbol->print(OS, MAI);
   OS << ", ";
   Value->print(OS, MAI);
   EmitEOL();
 }

llvm/trunk/lib/MC/MCSectionXCOFF.cpp

if (getMappingClass() != XCOFF::XMC_PR)
       llvm_unreachable("Unsupported storage-mapping class for .text csect");
 
     OS << "\t.csect " << getSectionName() << "["
        << "PR"
        << "]" << '\n';
     return;
   }
 
-  if (getKind().isCommon()) {
-    if (getMappingClass() != XCOFF::XMC_RW)
-      llvm_unreachable("Unsupported storage-mapping class for common csect");
+  if (getKind().isBSSLocal() || getKind().isCommon()) {
+    if (getMappingClass() != XCOFF::XMC_RW &&
+        getMappingClass() != XCOFF::XMC_BS)
+      llvm_unreachable("Generated a storage-mapping class for a common/bss "
+                       "csect we don't understand how to switch to.");
     if (getCSectType() != XCOFF::XTY_CM)
       llvm_unreachable("wrong csect type for .bss csect");
     // Don't have to print a directive for switching to section for commons.
     // '.comm' and '.lcomm' directives for the variable will create the needed
     // csect.
     return;
   }
 
   report_fatal_error("Printing for this SectionKind is unimplemented.");
 }
 
 bool MCSectionXCOFF::UseCodeAlign() const { return getKind().isText(); }
 
 bool MCSectionXCOFF::isVirtualSection() const { return XCOFF::XTY_CM == Type; }

llvm/trunk/lib/MC/MCStreamer.cpp

 }
 void MCStreamer::EmitFileDirective(StringRef Filename) {}
 void MCStreamer::EmitCOFFSymbolStorageClass(int StorageClass) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
 void MCStreamer::EmitCOFFSymbolType(int Type) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
+void MCStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                                            unsigned ByteAlign) {
+  llvm_unreachable("this directive only supported on XCOFF targets");
+}
 void MCStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {}
 void MCStreamer::emitELFSymverDirective(StringRef AliasName,
                                         const MCSymbol *Aliasee) {}
 void MCStreamer::EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                        unsigned ByteAlignment) {}
 void MCStreamer::EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
                                 uint64_t Size, unsigned ByteAlignment) {}
 void MCStreamer::ChangeSection(MCSection *, const MCExpr *) {}

llvm/trunk/lib/MC/MCXCOFFStreamer.cpp

MCStreamer *llvm::createXCOFFStreamer(MCContext &Context,
                                       std::unique_ptr<MCCodeEmitter> &&CE,
                                       bool RelaxAll) {
   MCXCOFFStreamer *S = new MCXCOFFStreamer(Context, std::move(MAB),
                                            std::move(OW), std::move(CE));
   if (RelaxAll)
     S->getAssembler().setRelaxAll(true);
   return S;
 }
+
+void MCXCOFFStreamer::EmitXCOFFLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                                            unsigned ByteAlign) {
+  llvm_unreachable("Not implemented yet.");
+}
 No newline at end of file

llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp

void PPCAIXAsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
 
   if (GV->hasSection())
     report_fatal_error("Custom section for Data not yet supported.");
 
   if (GV->hasComdat())
     report_fatal_error("COMDAT not yet supported by AIX.");
 
   SectionKind GVKind = getObjFileLowering().getKindForGlobal(GV, TM);
-  if (!GVKind.isCommon())
+  if (!GVKind.isCommon() && !GVKind.isBSSLocal())
     report_fatal_error("Only common variables are supported on AIX for now.");
 
   // Create the containing csect and switch to it.
   MCSectionXCOFF *CSect = cast<MCSectionXCOFF>(
       getObjFileLowering().SectionForGlobal(GV, GVKind, TM));
   OutStreamer->SwitchSection(CSect);
 
   // Create the symbol and emit it.
   MCSymbolXCOFF *XSym = cast<MCSymbolXCOFF>(getSymbol(GV));
   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,

llvm/trunk/test/CodeGen/PowerPC/aix-xcoff-lcomm.ll

+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@a = internal global i32 0, align 4
+@b = internal global i64 0, align 8
+@c = internal global i16 0, align 2
+
+; CHECK:      .lcomm a,4,a,2
+; CHECK-NEXT: .lcomm b,8,b,3
+; CHECK-NEXT: .lcomm c,2,c,1