[PATCH] D153600: Implement -frecord-command-line for XCOFF

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbbd0d123d3aa: Implement -frecord-command-line for XCOFF (authored by Jake-Egan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153600: Implement -frecord-command-line for XCOFF

scott.linder accepted this revision. scott.linder added a comment. This revision is now accepted and ready to land. Thank you for the patch and the changes! This LGTM, and AFAICT all of the comments have been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:981 + + // Metadata needs to be padded out to an even word size. + size_t MetadataSize = Metadata.size(); scott.linder wrote: > stephenpeckham wrote: > > There's no requirement to pad

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 537572. Jake-Egan added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/ https://reviews.llvm.org/D153600 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D153600: Implement -frecord-command-line for XCOFF

scott.linder added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:993 + // Metadata needs to be padded out to an even word size. + uint32_t PaddedSize = alignTo(std::max((int)MetadataSize, 1), 4); + uint32_t PaddingSize = PaddedSize - MetadataSize;

[PATCH] D153600: Implement -frecord-command-line for XCOFF

MaskRay added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:972 +void MCAsmStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) { + const char *InfoDirective = "\t.info "; + const char *Separator = ", "; Repository: rG LLVM Github

[PATCH] D153600: Implement -frecord-command-line for XCOFF

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2974 +RSOS << "@(#)" << MDS->getString(); +RSOS.write('\0'); + } hubert.reinterpretcast wrote: > stephenpeckham wrote: > > I would use a newline here.

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 537377. Jake-Egan added a comment. Rerun CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/ https://reviews.llvm.org/D153600 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 537160. Jake-Egan added a comment. Fix formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/ https://reviews.llvm.org/D153600 Files: clang/lib/Driver/ToolChains/Clang.cpp llvm/include/llvm/CodeGen/AsmPrinter.h

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 537156. Jake-Egan edited the summary of this revision. Jake-Egan added a comment. Updated to use both a newline and null byte to separate command lines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/ https://reviews.llvm.org/D153600

[PATCH] D153600: Implement -frecord-command-line for XCOFF

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2338 // Emit bytes for llvm.commandline metadata. - emitModuleCommandLines(M); + // The command line metadata waas emitted earlier on XCOFF. + if

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 536920. Jake-Egan edited the summary of this revision. Jake-Egan added a comment. Thanks for the review @stephenpeckham, I updated the patch with the requested changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/

[PATCH] D153600: Implement -frecord-command-line for XCOFF

stephenpeckham added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2338 // Emit bytes for llvm.commandline metadata. - emitModuleCommandLines(M); + if (!TM.getTargetTriple().isOSBinFormatXCOFF()) +emitModuleCommandLines(M); I

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 536890. Jake-Egan added a comment. Fix formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/ https://reviews.llvm.org/D153600 Files: clang/lib/Driver/ToolChains/Clang.cpp llvm/include/llvm/CodeGen/AsmPrinter.h

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan marked an inline comment as done. Jake-Egan added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:986 + + // If there's no metadata, the length is 0. + if (MetadataSize == 0) { Handled the 0 length case here. CHANGES SINCE LAST ACTION

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan marked 6 inline comments as done. Jake-Egan added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:1022 + if (PaddingSize) { +assert(PaddedSize - Index == WordSize); +std::array LastWord = {0}; I changed the assert that you requested

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan updated this revision to Diff 536795. Jake-Egan added a comment. Thanks for the review @scott.linder. I applied the changes you requested with some differences. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153600/new/ https://reviews.llvm.org/D153600 Files:

[PATCH] D153600: Implement -frecord-command-line for XCOFF

scott.linder added a comment. 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 Comment at:

[PATCH] D153600: Implement -frecord-command-line for XCOFF

Jake-Egan created this revision. Herald added subscribers: kbarton, hiraditya, nemanjai. Herald added a project: All. Jake-Egan requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. Repository: rG LLVM Github