Implement -frecord-command-line for XCOFF

[Jake-Egan]

Summary

This patch extends support of the option -frecord-command-line to XCOFF. XCOFF doesn’t have custom sections like ELF, so the command line data is emitted to a .info section instead. A C_INFO symbol is generated with the .info section to preserve the command line data past the link step. Multiple command lines are separated by newlines and null bytes. The command line data can be retrieved on AIX with command what file_name.

[scott.linder]

Sorry for the delay in review, I am not too familiar with XCOFF so I was hoping someone else would take a look first.

If my understanding of the COFF docs I could find is correct then this LGTM save for some nits/suggestions

llvm/lib/MC/MCAsmStreamer.cpp

970–1037

I sketched out some of the suggestions I had, although the bigger changes are just because the extra .info for the padded byte bugged me. If you aren't concerned with it I'm also happy with what you have

980

I couldn't quickly find a reference for the alignment requirement, but it seems like there is an additional requirement that the length must also be non-zero, not just even?

If so, can you update the comment?

I would also rather explicitly use alignTo and max to express this (see suggestion), but if we don't expect the optimizer to clean it up I'm fine with the more terse version.

980

Can you factor this out at function scope? It gets repeated below

984

Redundant cast

988

It seems odd to use a lambda when this is only used once. Why not just compute the last word directly?

1033

Can you factor this out as Separator at function scope?

llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll

21

Having the padded byte force a new .info directive doesn't seem ideal. I suggested something to avoid it, but I suppose it also doesn't really harm anything.

[Jake-Egan]

Thanks for the review @scott.linder. I applied the changes you requested with some differences.

[Jake-Egan]

llvm/lib/MC/MCAsmStreamer.cpp

980

The length can be zero. See second paragraph of the comment section: https://www.ibm.com/docs/en/aix/7.2?topic=formats-xcoff-object-file-format#XCOFF__a1pyfk2b4joyc__title__1

1021

I changed the assert that you requested because it would always fail:

assert(Length - Index == MetadataPaddingSize);

[Jake-Egan]

llvm/lib/MC/MCAsmStreamer.cpp

985

Handled the 0 length case here.

[Jake-Egan]

Fix formatting

[stephenpeckham]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

2337–2338

I would add a comment explaining that for XCOFF, the command line metadata was emitted earlier.

llvm/lib/MC/MCAsmStreamer.cpp

980

There's no requirement to pad the .info section. When you generate assembly language, the .info pseudo-op can only generate words of data, so the if the data length is not 0(mod 4), the last word will have to be padded with low-order 0s. This means that the length of the .info section will be a multiple of 4. The length, on the other hand, should be exact. At link time, only "length" bytes will be copied to the output file.

If you emit object code directly, you will not need to emit any padding bytes.

986

Can this be moved up right after the computation of MetadataSize? Can't you just emit "0,"? Why call format_hex()? It seems odd to have a literal comma here instead of using Separator.

993

You should use MetadataSize and Separator here. In fact, you could emit the length before checking for 0, expecially because a length of 0 is a rare case (and impossible for this patch).

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

2973

I would use a newline here. The AIX what command looks for @(#) and echos subsequent bytes until it sees a double quote, a backslash, a > symbol, newline, or null byte. The @(#) is not echoed, nor is the terminating character. The what command prints a newline after it finds a terminating character. This means that if the command line contains any of the special characters, the line will be truncated.

Exception: If the @(#) is followed by "opt " or " opt ", the terminating characters are only a newline or null byte. This allows any of the other special characters to be part of the command line. It doesn't really matter if you use a newline or a null byte, but the legacy XL compiler uses a newline. The "opt" keyword should appear if the command line can contain a double quote, a > or a backslash.

The legacy compiler also uses other keywords besides "opt", including "version" and "cfg". The what command doesn't do anything special with these keywords.

llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll

17

0x0000002e: The actual length should be emitted.

[Jake-Egan]

Thanks for the review @stephenpeckham, I updated the patch with the requested changes.

[hubert.reinterpretcast]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

2337–2338

Minor nit: Typo.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

2973

As mentioned offline, newline on its own has potential of ambiguity because it can appear in command line options (null bytes cannot). If there is a preference for newline to be present, then having a null byte after could help.

Note that @(#)opt can appear on the command line too. Using what will have limitations (but we should leave the possibility open for other tools/methods to work).

[Jake-Egan]

Updated to use both a newline and null byte to separate command lines.

[Jake-Egan]

Fix formatting

[Jake-Egan]

Rerun CI

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

2973

Thanks; I confirm that my comment has been addressed. I have no further comments at this time. Feel free to commit if another reviewer approves the patch.

[MaskRay]

llvm/lib/MC/MCAsmStreamer.cpp

971

[scott.linder]

llvm/lib/MC/MCAsmStreamer.cpp

980

Does the comment still need updating then? It could capture the fact that the "lowest common denominator" is the assembly syntax as it works in terms of words, and so may requiring padding in the final word. We can be clear that we apply the same restriction to the object case judiciously, as it make the output identical and avoids more code paths. We could also note that the linker can use the length to optimize the final linked binary.

These all clear things up to a fresh reader, so I think they are worthwhile things to include in comments.

992

With the MetadataSize == 0 case handled the std::max should be redundant now, right?

Also the literal 4 can be WordSize

1017

Nit: for pointer casts I would prefer an explicit reinterpret_cast

[Jake-Egan]

Address comments

[Jake-Egan]

llvm/lib/MC/MCAsmStreamer.cpp

980

I updated the comment. Please let me know if it's sufficient

[scott.linder]

Thank you for the patch and the changes! This LGTM, and AFAICT all of the comments have been addressed.

clang/lib/Driver/ToolChains/Clang.cpp

   // -grecord-gcc-switches options is set on.
   // By default, -gno-record-gcc-switches is set on and no recording.
   auto GRecordSwitches =
       Args.hasFlag(options::OPT_grecord_command_line,
                    options::OPT_gno_record_command_line, false);
   auto FRecordSwitches =
       Args.hasFlag(options::OPT_frecord_command_line,
                    options::OPT_fno_record_command_line, false);
-  if (FRecordSwitches && !Triple.isOSBinFormatELF())
+  if (FRecordSwitches && !Triple.isOSBinFormatELF() &&
+      !Triple.isOSBinFormatXCOFF())
     D.Diag(diag::err_drv_unsupported_opt_for_target)
         << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
         << TripleStr;
   if (TC.UseDwarfDebugFlags() || GRecordSwitches || FRecordSwitches) {
     ArgStringList OriginalArgs;
     for (const auto &Arg : Args)
       Arg->render(Args, OriginalArgs);
 

llvm/include/llvm/CodeGen/AsmPrinter.h

private:
   //===------------------------------------------------------------------===//
 
   void emitJumpTableEntry(const MachineJumpTableInfo *MJTI,
                           const MachineBasicBlock *MBB, unsigned uid) const;
   void emitLLVMUsedList(const ConstantArray *InitList);
   /// Emit llvm.ident metadata in an '.ident' directive.
   void emitModuleIdents(Module &M);
   /// Emit bytes for llvm.commandline metadata.
-  void emitModuleCommandLines(Module &M);
+  virtual void emitModuleCommandLines(Module &M);
 
   GCMetadataPrinter *getOrCreateGCPrinter(GCStrategy &S);
   void emitGlobalAlias(Module &M, const GlobalAlias &GA);
   void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
 
   /// This method decides whether the specified basic block requires a label.
   bool shouldEmitLabelForBasicBlock(const MachineBasicBlock &MBB) const;
 

llvm/include/llvm/MC/MCStreamer.h

virtual void emitXCOFFExceptDirective(const MCSymbol *Symbol,
                                         unsigned FunctionSize, bool hasDebug);
 
   /// Emit a XCOFF .ref directive which creates R_REF type entry in the
   /// relocation table for one or more symbols.
   ///
   /// \param Sym - The symbol on the .ref directive.
   virtual void emitXCOFFRefDirective(const MCSymbol *Symbol);
 
+  /// Emit a C_INFO symbol with XCOFF embedded metadata to the .info section.
+  ///
+  /// \param Name - The embedded metadata name
+  /// \param Metadata - The embedded metadata
+  virtual void emitXCOFFCInfoSym(StringRef Name, StringRef Metadata);
+
   /// 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/include/llvm/MC/MCXCOFFStreamer.h

public:
   void emitXCOFFRenameDirective(const MCSymbol *Name,
                                 StringRef Rename) override {
     report_fatal_error("emitXCOFFRenameDirective is not implemented yet on "
                        "object generation path");
   }
   void emitXCOFFExceptDirective(const MCSymbol *Symbol, const MCSymbol *Trap,
                                 unsigned Lang, unsigned Reason,
                                 unsigned FunctionSize, bool hasDebug) override;
+  void emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) override {
+    report_fatal_error("emitXCOFFCInfoSym is not implemented yet on "
+                       "object generation path");
+  }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCXCOFFSTREAMER_H

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

 #endif
       // TODO: Add timestamp and description.
       OutStreamer->emitFileDirective(FileName, VerStr, "", "");
     } else {
       OutStreamer->emitFileDirective(FileName);
     }
   }
 
+  // On AIX, emit bytes for llvm.commandline metadata after .file so that the
+  // C_INFO symbol is preserved if any csect is kept by the linker.
+  if (TM.getTargetTriple().isOSBinFormatXCOFF())
+    emitModuleCommandLines(M);
+
   GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
   assert(MI && "AsmPrinter didn't require GCModuleInfo?");
   for (const auto &I : *MI)
     if (GCMetadataPrinter *MP = getOrCreateGCPrinter(*I))
       MP->beginAssembly(M, *MI, *this);
 
   // Emit module-level inline asm if it exists.
   if (!M.getModuleInlineAsm().empty()) {
bool AsmPrinter::doFinalization(Module &M) {
   for (GCModuleInfo::iterator I = MI->end(), E = MI->begin(); I != E; )
     if (GCMetadataPrinter *MP = getOrCreateGCPrinter(**--I))
       MP->finishAssembly(M, *MI, *this);
 
   // Emit llvm.ident metadata in an '.ident' directive.
   emitModuleIdents(M);
 
   // Emit bytes for llvm.commandline metadata.
+  if (!TM.getTargetTriple().isOSBinFormatXCOFF())
     emitModuleCommandLines(M);
 
   // Emit .note.GNU-split-stack and .note.GNU-no-split-stack sections if
   // split-stack is used.
   if (TM.getTargetTriple().isOSBinFormatELF() && HasSplitStack) {
     OutStreamer->switchSection(OutContext.getELFSection(".note.GNU-split-stack",
                                                         ELF::SHT_PROGBITS, 0));
     if (HasNoSplitStack)
       OutStreamer->switchSection(OutContext.getELFSection(

llvm/lib/MC/MCAsmStreamer.cpp

public:
 
   void emitXCOFFRefDirective(const MCSymbol *Symbol) override;
 
   void emitXCOFFExceptDirective(const MCSymbol *Symbol,
                                 const MCSymbol *Trap,
                                 unsigned Lang, unsigned Reason,
                                 unsigned FunctionSize, bool hasDebug) override;
 
+  void emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) override;
+
   void emitELFSize(MCSymbol *Symbol, const MCExpr *Value) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         Align 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::emitXCOFFExceptDirective(const MCSymbol *Symbol,
                                              unsigned FunctionSize,
                                              bool hasDebug) {
   OS << "\t.except\t";
   Symbol->print(OS, MAI);
   OS << ", " << Lang << ", " << Reason;
   EmitEOL();
 }
 
+void MCAsmStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) {
+  const char *InfoDirective = "\t.info ";
+
+  // Start by emitting the .info pseudo-op and C_INFO symbol name
+  OS << InfoDirective;
+  PrintQuotedString(Name, OS);
+  OS << ", ";
+
+  // Metadata needs to be padded out to an even word size.
+  size_t MetadataSize = Metadata.size();
+  uint32_t MetadataPaddingSize = 3 - (MetadataSize - 1) % 4;
+
+  // Length of the metadata with padding
+  uint32_t Length = MetadataSize + MetadataPaddingSize;
+  OS << format_hex(uint32_t(Length), 10) << ",";
+  EmitEOL();
+
+  // Return the remaining bytes padded with 0s.
+  auto GetLastWord = [](const uint8_t *Data,
+                        uint32_t PaddingBytes) -> uint32_t {
+    uint32_t glw = 0;
+    switch (PaddingBytes) {
+    case 1:
+      glw |= Data[2] << 8;
+      LLVM_FALLTHROUGH;
+    case 2:
+      glw |= Data[1] << 16;
+      LLVM_FALLTHROUGH;
+    case 3:
+      glw |= Data[0] << 24;
+      return glw;
+    case 4:
+      // A special case which means a whole word of padding is needed.
+      return 0u;
+    }
+    report_fatal_error("Unexpected count of padding bytes.");
+  };
+
+  // Write out the payload a word at a time.
+  //
+  // The assembler has a limit on the number of operands in an expression,
+  // so we need multiple .info pseudo-ops. We choose a small number of words
+  // per pseudo-op to keep the assembly readable.
+  size_t Index = 0;
+  while (Index + 4 <= MetadataSize) {
+    const char *Separator = ", ";
+    OS << InfoDirective;
+    for (size_t End = std::min(Index + 20, MetadataSize); Index + 4 <= End;
+         Index += 4) {
+      OS << Separator;
+
+      uint32_t NextWord =
+          llvm::support::endian::read32be(Metadata.data() + Index);
+      OS << format_hex(NextWord, 10);
+    }
+    EmitEOL();
+  }
+
+  // If there is padding, then we have at least one byte of payload left
+  // to emit.
+  if (MetadataPaddingSize) {
+    uint32_t LastWord = GetLastWord((const uint8_t *)Metadata.data() + Index,
+                                    MetadataPaddingSize);
+    OS << InfoDirective << ", ";
+    OS << format_hex(LastWord, 10);
+    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/lib/MC/MCStreamer.cpp

void MCStreamer::emitXCOFFExceptDirective(const MCSymbol *Symbol,
                                           const MCSymbol *Trap,
                                           unsigned Lang, unsigned Reason,
                                           unsigned FunctionSize,
                                           bool hasDebug) {
   report_fatal_error("emitXCOFFExceptDirective is only supported on "
                      "XCOFF targets");
 }
 
+void MCStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) {
+  llvm_unreachable("emitXCOFFCInfoSym is only supported on"
+                   "XCOFF targets");
+}
+
 void MCStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {}
 void MCStreamer::emitELFSymverDirective(const MCSymbol *OriginalSym,
                                         StringRef Name, bool KeepOriginalSym) {}
 void MCStreamer::emitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                        Align ByteAlignment) {}
 void MCStreamer::emitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
                                 uint64_t Size, Align ByteAlignment) {}
 void MCStreamer::changeSection(MCSection *, const MCExpr *) {}

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

public:
 
   void emitLinkage(const GlobalValue *GV, MCSymbol *GVSym) const override;
 
   void emitInstruction(const MachineInstr *MI) override;
 
   bool doFinalization(Module &M) override;
 
   void emitTTypeReference(const GlobalValue *GV, unsigned Encoding) override;
+
+  void emitModuleCommandLines(Module &M) override;
 };
 
 } // end anonymous namespace
 
 void PPCAsmPrinter::PrintSymbolOperand(const MachineOperand &MO,
                                        raw_ostream &O) {
   // Computing the address of a global symbol, not calling it.
   const GlobalValue *GV = MO.getGlobal();
TargetRegistry::RegisterAsmPrinter(getThePPC32Target(),
                                      createPPCAsmPrinterPass);
   TargetRegistry::RegisterAsmPrinter(getThePPC32LETarget(),
                                      createPPCAsmPrinterPass);
   TargetRegistry::RegisterAsmPrinter(getThePPC64Target(),
                                      createPPCAsmPrinterPass);
   TargetRegistry::RegisterAsmPrinter(getThePPC64LETarget(),
                                      createPPCAsmPrinterPass);
 }
+
+void PPCAIXAsmPrinter::emitModuleCommandLines(Module &M) {
+  const NamedMDNode *NMD = M.getNamedMetadata("llvm.commandline");
+  if (!NMD || !NMD->getNumOperands())
+    return;
+
+  std::string S;
+  raw_string_ostream RSOS(S);
+  for (unsigned i = 0, e = NMD->getNumOperands(); i != e; ++i) {
+    const MDNode *N = NMD->getOperand(i);
+    assert(N->getNumOperands() == 1 &&
+           "llvm.commandline metadata entry can have only one operand");
+    const MDString *MDS = cast<MDString>(N->getOperand(0));
+    // Add "@(#)" to support retrieving the command line information with the
+    // AIX "what" command
+    RSOS << "@(#)" << MDS->getString();
+    RSOS.write('\0');
+  }
+  OutStreamer->emitXCOFFCInfoSym(".GCC.command.line", RSOS.str());
+}

llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll

+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=ASM %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=ASM %s
+
+; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff -filetype=obj  < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=OBJ %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj  < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=OBJ %s
+
+; Verify that llvm.commandline metadata is emitted to .info sections and that the
+; metadata is padded if necessary.
+
+; OBJ: LLVM ERROR: emitXCOFFCInfoSym is not implemented yet on object generation path
+
+; ASM: .info ".GCC.command.line", 0x00000030,
+; ASM: .info , 0x40282329, 0x636c616e, 0x67202d63, 0x6f6d6d61, 0x6e64202d
+; ASM: .info , 0x6c696e65, 0x00402823, 0x29736f6d, 0x65746869, 0x6e672065
+; ASM: .info , 0x6c736531
+; Trailing padding:
+; ASM: .info , 0x32330000
+
+!llvm.commandline = !{!0, !1}
+!0 = !{!"clang -command -line"}
+!1 = !{!"something else123"}