[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

962–963

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)

990

See above comment.

[hubert.reinterpretcast]

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

llvm/lib/MC/MCAsmStreamer.cpp

962–963

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

[hubert.reinterpretcast]

• Rebase to 3d9bb031d13c884122a5da456659e47dd52ec1f7

[daltenty]

LGTM

[hubert.reinterpretcast]

llvm/lib/MC/MCAsmStreamer.cpp

905

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

[daltenty]

llvm/lib/MC/MCAsmStreamer.cpp

905

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

905

@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

902

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

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;
+
+  /// Format string in the style of printf 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 null.  Defaults to null.
+  const char *CharacterLiteralFormatString = nullptr;
+
   /// 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; }
+  const char *characterLiteralFormatString() const {
+    return CharacterLiteralFormatString;
+  }
   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() {
   IsLittleEndian = false;
   SupportsQuotedNames = false;
   UseDotAlignForAlignment = true;
   ZeroDirective = "\t.space\t";
   ZeroDirectiveSupportsNonZeroValue = false;
   AsciiDirective = nullptr; // not supported
   AscizDirective = nullptr; // not supported
+  ByteListDirective = "\t.byte\t";
+  CharacterLiteralFormatString = "'%c"; // single quote + the character
   Data64bitsDirective = "\t.llong\t";
   COMMDirectiveAlignmentIsInBytes = false;
   LCOMMDirectiveAlignmentType = LCOMM::Log2Alignment;
   HasDotTypeDotSizeDirective = false;
   HasDotExternDirective = true;
   HasDotLGloblDirective = true;
   SymbolsHaveSMC = true;
   UseIntegratedAssembler = false;

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,
+                          const char *CharacterLiteralFormatString) {
+  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 printOneCharacter = [&OS, CharacterLiteralFormatString,
+                                  printCharacterInOctal](unsigned char C) {
+    if (isPrint(C)) {
+      OS << format(CharacterLiteralFormatString, 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));
+  };
+  if (LLVM_LIKELY(CharacterLiteralFormatString)) {
+    printCharacterList(printOneCharacter);
+    return;
+  }
+  printCharacterList(printCharacterInOctal);
+}
+
 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->characterLiteralFormatString());
+      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:      .globl  over_aligned
 ; CHECK-NEXT: .align  5
 ; CHECK-NEXT: over_aligned:
 ; CHECK-NEXT: .llong  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:
 ; CHECK-NEXT: .llong  0x3ff0000000000000
 ; CHECK-NEXT: .llong  0x4000000000000000
 ; CHECK-NEXT: .llong  0x4008000000000000
 ; CHECK-NEXT: .llong  0x4010000000000000

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

 ; CHECK-NEXT:   .align  2
 ; CHECK-NEXT: .Lmagic32:
 ; CHECK-NEXT:   .long   464                     # 0x1d0
 ; CHECK-NEXT:   .long   472                     # 0x1d8
 ; CHECK-NEXT:   .long   413                     # 0x19d
 ; CHECK-NEXT:   .long   0                       # 0x0
 ; CHECK-NEXT:   .csect .rodata.str1.1[RO],2
 ; CHECK-NEXT: .LstrA:
-; 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:  const_dvar:
 ; CHECK-NEXT:          .llong  0x408c200000000000
 ; CHECK-NEXT:          .globl  const_over_aligned
 ; CHECK-NEXT:          .align  5
 ; CHECK-NEXT:  const_over_aligned:
 ; CHECK-NEXT:          .llong  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:
 ; CHECK-NEXT:          .llong  0x3ff0000000000000
 ; CHECK-NEXT:          .llong  0x4000000000000000
 ; CHECK-NEXT:          .llong  0x4008000000000000
 ; CHECK-NEXT:          .llong  0x4010000000000000