[AIX] asm output: use character literals in byte lists for strings

[hubert.reinterpretcast]

Summary

This patch improves the assembly output produced for string literals by using character literals in byte lists. This provides the benefits of having printable characters appear as such in the assembly output and of having strings kept as logical units on the same line.

[daltenty]

I'm sure this may have been considered, but given that .byte also supports a limited string syntax (given with the limitation that it can support ), is there a reason we can't just glue consecutive runs of printable characters together?

e.g. .byte 'h,'e,'l,'l,'o,' ,'w,'o,'r,'l,'d,'!,0012,0000 becomes:

.byte   'hello world!'
.byte 0012,0000

(Essentially mode shift between writing printable as a string and the comma seperated octal form, and just emitting a new .byte directive when we encounter the other type)

[hubert.reinterpretcast]

In D80953#2069244, @daltenty wrote:

I'm sure this may have been considered, but

Yes, the patch description says consideration was given towards "having strings kept as logical units on the same line". This is beneficial for the purposes of being able to grep even when sequences mix "printable" and "non-printable" characters. For example, the UTF-8 code unit sequence for e with acute would likely end up on a line of its own if the alternative you mention is used. For a program with strings in French, I imagine that could be rather annoying.

[daltenty]

llvm/lib/MC/MCAsmStreamer.cpp

1052–1053

Why not ward off trouble below and just make the check reflect whether AsciizDirective is actually usable in this case upfront?

Data.size() == 1 || !(MAI->getAsciiDirective() || MAI->getByteListDirective() || MAI->getAsciizDirective() && Data.back() == 0)

1080

See above comment.

[hubert.reinterpretcast]

• Address comment: Avoid situation where assert arises; reduce duplication

llvm/lib/MC/MCAsmStreamer.cpp

1052–1053

I've refactored to remove the duplication of the conditions.

[hubert.reinterpretcast]

• Rebase to 3d9bb031d13c884122a5da456659e47dd52ec1f7

[daltenty]

LGTM

[hubert.reinterpretcast]

llvm/lib/MC/MCAsmStreamer.cpp

986

May need to hedge against -Werror,-Wformat-security warning as seen recently on another commit.

[daltenty]

llvm/lib/MC/MCAsmStreamer.cpp

986

I'm assuming specifically we're worried about -Wformat-nonliteral.

I think the MAI property supporting other arbitrary character formatting strings is a nice to have, but not essential to this patch, since AIX is currently the only user. We could simplify to the just supporting a character prefix (single quote in our case) or something like that so we can use a fixed format for the character itself.

[hubert.reinterpretcast]

• Update to avoid stored format string by using switch on a enum value

llvm/lib/MC/MCAsmStreamer.cpp

986

@daltenty, the patch has been updated to encode the known (and used) formats into an enumeration type. The implementation of the formatting itself is selected via a switch, which passes different lambdas depending on the case chosen.

[daltenty]

LGTM, thanks!

llvm/lib/MC/MCAsmStreamer.cpp

983

nit: the name of this lambda isn't very descriptive, but I don't really have a better suggestion

llvm/include/llvm/MC/MCAsmInfo.h

 
 enum LCOMMType { NoAlignment, ByteAlignment, Log2Alignment };
 
 } // end namespace LCOMM
 
 /// This class is intended to be used as a base class for asm
 /// properties and features specific to the target.
 class MCAsmInfo {
+public:
+  /// Assembly character literal syntax types.
+  enum AsmCharLiteralSyntax {
+    ACLS_Unknown, /// Unknown; character literals not used by LLVM for this
+                  /// target.
+    ACLS_SingleQuotePrefix, /// The desired character is prefixed by a single
+                            /// quote, e.g., `'A`.
+  };
+
 protected:
   //===------------------------------------------------------------------===//
   // Properties to be set by the target writer, used to configure asm printer.
   //
 
   /// Code pointer size in bytes.  Default is 4.
   unsigned CodePointerSize = 4;
 
protected:
   /// can be set to null. Defaults to "\t.ascii\t"
   const char *AsciiDirective;
 
   /// If not null, this allows for special handling of zero terminated strings
   /// on this target.  This is commonly supported as ".asciz".  If a target
   /// doesn't support this, it can be set to null.  Defaults to "\t.asciz\t"
   const char *AscizDirective;
 
+  /// This directive accepts a comma-separated list of bytes for emission as a
+  /// string of bytes.  For targets that do not support this, it shall be set to
+  /// null.  Defaults to null.
+  const char *ByteListDirective = nullptr;
+
+  /// Form used for character literals in the assembly syntax.  Useful for
+  /// producing strings as byte lists.  If a target does not use or support
+  /// this, it shall be set to ACLS_Unknown.  Defaults to ACLS_Unknown.
+  AsmCharLiteralSyntax CharacterLiteralSyntax = ACLS_Unknown;
+
   /// These directives are used to output some unit of integer data to the
   /// current section.  If a data directive is set to null, smaller data
   /// directives will be used to emit the large sizes.  Defaults to "\t.byte\t",
   /// "\t.short\t", "\t.long\t", "\t.quad\t"
   const char *Data8bitsDirective;
   const char *Data16bitsDirective;
   const char *Data32bitsDirective;
   const char *Data64bitsDirective;
public:
   }
 
   const char *getZeroDirective() const { return ZeroDirective; }
   bool doesZeroDirectiveSupportNonZeroValue() const {
     return ZeroDirectiveSupportsNonZeroValue;
   }
   const char *getAsciiDirective() const { return AsciiDirective; }
   const char *getAscizDirective() const { return AscizDirective; }
+  const char *getByteListDirective() const { return ByteListDirective; }
+  AsmCharLiteralSyntax characterLiteralSyntax() const {
+    return CharacterLiteralSyntax;
+  }
   bool getAlignmentIsInBytes() const { return AlignmentIsInBytes; }
   unsigned getTextAlignFillValue() const { return TextAlignFillValue; }
   const char *getGlobalDirective() const { return GlobalDirective; }
 
   bool doesSetDirectiveSuppressReloc() const {
     return SetDirectiveSuppressesReloc;
   }
 

llvm/lib/MC/MCAsmInfoXCOFF.cpp

MCAsmInfoXCOFF::MCAsmInfoXCOFF() {
   PrivateGlobalPrefix = "L..";
   PrivateLabelPrefix = "L..";
   SupportsQuotedNames = false;
   UseDotAlignForAlignment = true;
   ZeroDirective = "\t.space\t";
   ZeroDirectiveSupportsNonZeroValue = false;
   AsciiDirective = nullptr; // not supported
   AscizDirective = nullptr; // not supported
+  ByteListDirective = "\t.byte\t";
+  CharacterLiteralSyntax = ACLS_SingleQuotePrefix;
 
   // Use .vbyte for data definition to avoid directives that apply an implicit
   // alignment.
   Data16bitsDirective = "\t.vbyte\t2, ";
   Data32bitsDirective = "\t.vbyte\t4, ";
 
   COMMDirectiveAlignmentIsInBytes = false;
   LCOMMDirectiveAlignmentType = LCOMM::Log2Alignment;

llvm/lib/MC/MCAsmStreamer.cpp

void MCAsmStreamer::emitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
   // that.
   if (ByteAlignment > 1) OS << ", " << Log2_32(ByteAlignment);
 
   EmitEOL();
 }
 
 static inline char toOctal(int X) { return (X&7)+'0'; }
 
+static void PrintByteList(StringRef Data, raw_ostream &OS,
+                          MCAsmInfo::AsmCharLiteralSyntax ACLS) {
+  assert(!Data.empty() && "Cannot generate an empty list.");
+  const auto printCharacterInOctal = [&OS](unsigned char C) {
+    OS << '0';
+    OS << toOctal(C >> 6);
+    OS << toOctal(C >> 3);
+    OS << toOctal(C >> 0);
+  };
+  const auto printOneCharacterFor = [printCharacterInOctal](
+                                        auto printOnePrintingCharacter) {
+    return [printCharacterInOctal, printOnePrintingCharacter](unsigned char C) {
+      if (isPrint(C)) {
+        printOnePrintingCharacter(static_cast<char>(C));
+        return;
+      }
+      printCharacterInOctal(C);
+    };
+  };
+  const auto printCharacterList = [Data, &OS](const auto &printOneCharacter) {
+    const auto BeginPtr = Data.begin(), EndPtr = Data.end();
+    for (const unsigned char C : make_range(BeginPtr, EndPtr - 1)) {
+      printOneCharacter(C);
+      OS << ',';
+    }
+    printOneCharacter(*(EndPtr - 1));
+  };
+  switch (ACLS) {
+  case MCAsmInfo::ACLS_Unknown:
+    printCharacterList(printCharacterInOctal);
+    return;
+  case MCAsmInfo::ACLS_SingleQuotePrefix:
+    printCharacterList(printOneCharacterFor([&OS](char C) {
+      const char AsmCharLitBuf[2] = {'\'', C};
+      OS << StringRef(AsmCharLitBuf, sizeof(AsmCharLitBuf));
+    }));
+    return;
+  }
+  llvm_unreachable("Invalid AsmCharLiteralSyntax value!");
+}
+
 static void PrintQuotedString(StringRef Data, raw_ostream &OS) {
   OS << '"';
 
   for (unsigned i = 0, e = Data.size(); i != e; ++i) {
     unsigned char C = Data[i];
     if (C == '"' || C == '\\') {
       OS << '\\' << (char)C;
       continue;
static void PrintQuotedString(StringRef Data, raw_ostream &OS) {
 
   OS << '"';
 }
 
 void MCAsmStreamer::emitBytes(StringRef Data) {
   assert(getCurrentSectionOnly() &&
          "Cannot emit contents before setting section!");
   if (Data.empty()) return;
 
-  // If only single byte is provided or no ascii or asciz directives is
-  // supported, emit as vector of 8bits data.
-  if (Data.size() == 1 ||
-      !(MAI->getAscizDirective() || MAI->getAsciiDirective())) {
-    if (MCTargetStreamer *TS = getTargetStreamer()) {
-      TS->emitRawBytes(Data);
-    } else {
-      const char *Directive = MAI->getData8bitsDirective();
-      for (const unsigned char C : Data.bytes()) {
-        OS << Directive << (unsigned)C;
-        EmitEOL();
-      }
-    }
-    return;
-  }
-
+  const auto emitAsString = [this](StringRef Data) {
     // If the data ends with 0 and the target supports .asciz, use it, otherwise
-  // use .ascii
+    // use .ascii or a byte-list directive
     if (MAI->getAscizDirective() && Data.back() == 0) {
       OS << MAI->getAscizDirective();
-    Data = Data.substr(0, Data.size()-1);
-  } else {
+      Data = Data.substr(0, Data.size() - 1);
+    } else if (LLVM_LIKELY(MAI->getAsciiDirective())) {
       OS << MAI->getAsciiDirective();
+    } else if (MAI->getByteListDirective()) {
+      OS << MAI->getByteListDirective();
+      PrintByteList(Data, OS, MAI->characterLiteralSyntax());
+      EmitEOL();
+      return true;
+    } else {
+      return false;
     }
 
     PrintQuotedString(Data, OS);
     EmitEOL();
+    return true;
+  };
+
+  if (Data.size() != 1 && emitAsString(Data))
+    return;
+
+  // Only single byte is provided or no ascii, asciz, or byte-list directives
+  // are applicable. Emit as vector of individual 8bits data elements.
+  if (MCTargetStreamer *TS = getTargetStreamer()) {
+    TS->emitRawBytes(Data);
+    return;
+  }
+  const char *Directive = MAI->getData8bitsDirective();
+  for (const unsigned char C : Data.bytes()) {
+    OS << Directive << (unsigned)C;
+    EmitEOL();
+  }
 }
 
 void MCAsmStreamer::emitBinaryData(StringRef Data) {
   // This is binary data. Print it in a grid of hex bytes for readability.
   const size_t Cols = 4;
   for (size_t I = 0, EI = alignTo(Data.size(), Cols); I < EI; I += Cols) {
     size_t J = I, EJ = std::min(I + Cols, Data.size());
     assert(EJ > 0);

llvm/test/CodeGen/PowerPC/aix-bytestring.ll

+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@str = constant [256 x i8] c"\01\02\03\04\05\06\07\08\09\0A\0B\0C\0D\0E\0F\10\11\12\13\14\15\16\17\18\19\1A\1B\1C\1D\1E\1F !\22#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\7F\80\81\82\83\84\85\86\87\88\89\8A\8B\8C\8D\8E\8F\90\91\92\93\94\95\96\97\98\99\9A\9B\9C\9D\9E\9F\A0\A1\A2\A3\A4\A5\A6\A7\A8\A9\AA\AB\AC\AD\AE\AF\B0\B1\B2\B3\B4\B5\B6\B7\B8\B9\BA\BB\BC\BD\BE\BF\C0\C1\C2\C3\C4\C5\C6\C7\C8\C9\CA\CB\CC\CD\CE\CF\D0\D1\D2\D3\D4\D5\D6\D7\D8\D9\DA\DB\DC\DD\DE\DF\E0\E1\E2\E3\E4\E5\E6\E7\E8\E9\EA\EB\EC\ED\EE\EF\F0\F1\F2\F3\F4\F5\F6\F7\F8\F9\FA\FB\FC\FD\FE\FF\00", align 1
+
+; CHECK-LABEL:str:
+;  CHECK-NEXT:        .byte   0001,0002,0003,0004,0005,0006,0007,0010,0011,0012,0013,0014,0015,0016,0017,0020,0021,0022,0023,0024,0025,0026,0027,0030,0031,0032,0033,0034,0035,0036,0037,' ,'!,'",'#,'$,'%,'&,'','(,'),'*,'+,',,'-,'.,'/,'0,'1,'2,'3,'4,'5,'6,'7,'8,'9,':,';,'<,'=,'>,'?,'@,'A,'B,'C,'D,'E,'F,'G,'H,'I,'J,'K,'L,'M,'N,'O,'P,'Q,'R,'S,'T,'U,'V,'W,'X,'Y,'Z,'[,'\,'],'^,'_,'`,'a,'b,'c,'d,'e,'f,'g,'h,'i,'j,'k,'l,'m,'n,'o,'p,'q,'r,'s,'t,'u,'v,'w,'x,'y,'z,'{,'|,'},'~,0177,0200,0201,0202,0203,0204,0205,0206,0207,0210,0211,0212,0213,0214,0215,0216,0217,0220,0221,0222,0223,0224,0225,0226,0227,0230,0231,0232,0233,0234,0235,0236,0237,0240,0241,0242,0243,0244,0245,0246,0247,0250,0251,0252,0253,0254,0255,0256,0257,0260,0261,0262,0263,0264,0265,0266,0267,0270,0271,0272,0273,0274,0275,0276,0277,0300,0301,0302,0303,0304,0305,0306,0307,0310,0311,0312,0313,0314,0315,0316,0317,0320,0321,0322,0323,0324,0325,0326,0327,0330,0331,0332,0333,0334,0335,0336,0337,0340,0341,0342,0343,0344,0345,0346,0347,0350,0351,0352,0353,0354,0355,0356,0357,0360,0361,0362,0363,0364,0365,0366,0367,0370,0371,0372,0373,0374,0375,0376,0377,0000

llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll

 ; CHECK-NEXT: .align  5
 ; CHECK-NEXT: over_aligned:
 ; CHECK32-NEXT: .vbyte  4, 1082925056
 ; CHECK32-NEXT: .vbyte  4, 0
 ; CHECK64-NEXT: .vbyte  8, 0x408c200000000000
 
 ; CHECK:      .globl  chrarray
 ; CHECK-NEXT: chrarray:
-; CHECK-NEXT: .byte   97
-; CHECK-NEXT: .byte   98
-; CHECK-NEXT: .byte   99
-; CHECK-NEXT: .byte   100
+; CHECK-NEXT: .byte   'a,'b,'c,'d
 
 ; CHECK:      .globl  dblarr
 ; CHECK-NEXT: .align  3
 ; CHECK-NEXT: dblarr:
 ; CHECK32-NEXT: .vbyte  4, 1072693248
 ; CHECK32-NEXT: .vbyte  4, 0
 ; CHECK64-NEXT: .vbyte  8, 0x3ff0000000000000
 ; CHECK32-NEXT: .vbyte  4, 1073741824

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll

 ; CHECK-NEXT:   .align  2
 ; CHECK-NEXT: L..magic32:
 ; CHECK-NEXT:   .vbyte  4, 464                     # 0x1d0
 ; CHECK-NEXT:   .vbyte  4, 472                     # 0x1d8
 ; CHECK-NEXT:   .vbyte  4, 413                     # 0x19d
 ; CHECK-NEXT:   .vbyte  4, 0                       # 0x0
 ; CHECK-NEXT:   .csect .rodata.str1.1[RO],2
 ; CHECK-NEXT: L..strA:
-; CHECK-NEXT: .byte   104
-; CHECK-NEXT: .byte   101
-; CHECK-NEXT: .byte   108
-; CHECK-NEXT: .byte   108
-; CHECK-NEXT: .byte   111
-; CHECK-NEXT: .byte   32
-; CHECK-NEXT: .byte   119
-; CHECK-NEXT: .byte   111
-; CHECK-NEXT: .byte   114
-; CHECK-NEXT: .byte   108
-; CHECK-NEXT: .byte   100
-; CHECK-NEXT: .byte   33
-; CHECK-NEXT: .byte   10
-; CHECK-NEXT: .byte   0
+; CHECK-NEXT: .byte   'h,'e,'l,'l,'o,' ,'w,'o,'r,'l,'d,'!,0012,0000
 ; CHECK-NEXT: L...str:
-; CHECK-NEXT: .byte   97
-; CHECK-NEXT: .byte   98
-; CHECK-NEXT: .byte   99
-; CHECK-NEXT: .byte   100
-; CHECK-NEXT: .byte   101
-; CHECK-NEXT: .byte   102
-; CHECK-NEXT: .byte   103
-; CHECK-NEXT: .byte   104
-; CHECK-NEXT: .byte   0
+; CHECK-NEXT: .byte   'a,'b,'c,'d,'e,'f,'g,'h,0000
 
 ; CHECKOBJ:     00000010 <.rodata.str2.2>:
 ; CHECKOBJ-NEXT:       10: 01 08 01 10
 ; CHECKOBJ-NEXT:       14: 00 d5 00 00    {{.*}}{{[[:space:]] *}}
 ; CHECKOBJ-NEXT: 00000018 <.rodata.str4.4>:
 ; CHECKOBJ-NEXT:       18: 00 00 01 d0
 ; CHECKOBJ-NEXT:       1c: 00 00 01 d8
 ; CHECKOBJ-NEXT:       20: 00 00 01 9d
 ; CHECKOBJ-NEXT:       24: 00 00 00 00    {{.*}}{{[[:space:]] *}}
 ; CHECKOBJ-NEXT: 00000028 <.rodata.str1.1>:
 ; CHECKOBJ-NEXT:       28: 68 65 6c 6c
 ; CHECKOBJ-NEXT:       2c: 6f 20 77 6f
 ; CHECKOBJ-NEXT:       30: 72 6c 64 21
 ; CHECKOBJ-NEXT:       34: 0a 00 61 62
 ; CHECKOBJ-NEXT:       38: 63 64 65 66
 ; CHECKOBJ-NEXT:       3c: 67 68 00 00

llvm/test/CodeGen/PowerPC/aix-xcoff-rodata.ll

 ; CHECK-NEXT:          .globl  const_over_aligned
 ; CHECK-NEXT:          .align  5
 ; CHECK-NEXT:  const_over_aligned:
 ; CHECK32-NEXT:        .vbyte 4, 1082925056
 ; CHECK32-NEXT:        .vbyte 4, 0
 ; CHECK64-NEXT:        .vbyte 8, 0x408c200000000000
 ; CHECK-NEXT:          .globl  const_chrarray
 ; CHECK-NEXT:  const_chrarray:
-; CHECK-NEXT:          .byte   97
-; CHECK-NEXT:          .byte   98
-; CHECK-NEXT:          .byte   99
-; CHECK-NEXT:          .byte   100
+; CHECK-NEXT:          .byte   'a,'b,'c,'d
 ; CHECK-NEXT:          .globl  const_dblarr
 ; CHECK-NEXT:          .align  3
 ; CHECK-NEXT:  const_dblarr:
 ; CHECK32-NEXT:        .vbyte 4, 1072693248
 ; CHECK32-NEXT:        .vbyte 4, 0
 ; CHECK64-NEXT:        .vbyte 8, 0x3ff0000000000000
 ; CHECK32-NEXT:        .vbyte 4, 1073741824
 ; CHECK32-NEXT:        .vbyte 4, 0