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

[Stephen Peckham via Phabricator via cfe-commits]

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 would add a comment explaining that for XCOFF, the command line metadata was emitted earlier.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:981
+
+  // Metadata needs to be padded out to an even word size.
+  size_t MetadataSize = Metadata.size();
----------------
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.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:987
+  // If there's no metadata, the length is 0.
+  if (MetadataSize == 0) {
+    OS << format_hex(uint32_t(0), 10) << ",";
----------------
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.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:994
+  // Emit length of the metadata with padding.
+  OS << format_hex(PaddedSize, 10) << ",";
+
----------------
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).


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2974
+    RSOS << "@(#)" << MDS->getString();
+    RSOS.write('\0');
+  }
----------------
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.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll:16
+
+; ASM: .info ".GCC.command.line", 0x00000030,
+; ASM: .info , 0x40282329, 0x636c616e, 0x67202d63, 0x6f6d6d61, 0x6e64202d, 0x6c696e65
----------------
0x0000002e:  The actual length should be emitted.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153600/new/

https://reviews.llvm.org/D153600