[PATCH] D80934: [AIX] Update data directives for AIX assembly

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 1 20:01:55 PDT 2020


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/MCAsmInfoXCOFF.cpp:32
+
+  // The standard AIX assembly directives auto-align, so they are not usable.
+  Data16bitsDirective = "\t.vbyte\t2, ";
----------------
I'm sure that "standard" is the right word for classifying the directives that happen to be similarly named to ones on some other platforms.

Suggestion:
Use `.vbyte` for data definition to avoid directives that apply an implicit alignment.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.cpp:63
+
+  // A size of 8 is only supported by the assembler on 64-bit.
+  Data64bitsDirective = Is64Bit ? "\t.vbyte\t8, " : nullptr;
----------------
s/on/under/;


================
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:54
+; BIT32-NEXT: 	  .vbyte	4, foo_ext_weak_ref[DS]
+; BIT64-NEXT: 	  .vbyte	8,  foo_ext_weak_ref[DS]
 ; COMMON-NEXT:    .weak   b_w[UA]
----------------
Even if we aren't consistent on how many spaces we use for where the tabs are in the real output, let's be consistent at least on having exactly one space after the comma.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-lower-constant-pool-index.ll:51
 ; 32SMALL-ASM: .LCPI0_0:
-; 32SMALL-ASM:         .long  0x40b00000
+; 32SMALL-ASM:         .vbyte	4,  0x40b00000
 ; 32SMALL-ASM: .test_float:
----------------
Same comment re: spaces after the comma. Please address throughout.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll:102
 ; 32SMALL-ASM: .LJTI0_0:
-; 32SMALL-ASM: 	    .long   LBB0_2-.LJTI0_0
-; 32SMALL-ASM: 	    .long   LBB0_3-.LJTI0_0
-; 32SMALL-ASM: 	    .long   LBB0_4-.LJTI0_0
-; 32SMALL-ASM: 	    .long   LBB0_5-.LJTI0_0
+; 32SMALL-ASM: 	    .vbyte	4,   LBB0_2-.LJTI0_0
+; 32SMALL-ASM: 	    .vbyte	4,   LBB0_3-.LJTI0_0
----------------
Whichever patch, between this and D80831, that lands later is going to have to deal with the merge conflict.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll:124
+
+; CHECK:      .globl astruct
+; CHECK-NEXT: astruct:
----------------
Please adjust the spacing to get the directives to align within at least the block (with indentation in relation to the label).


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll:31
+;CHECK64-NEXT:         .vbyte	8,  4611686018427387954
+;CHECK-NEXT:         .vbyte	4,   0                       # 0x0
 ;CHECK-NEXT:         .space  4
----------------
Same comment re: aligning the lines. Please fix throughout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80934





More information about the llvm-commits mailing list