[PATCH] D138778: Output alignment in zerofill and comm only is needed

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet created this revision.
gchatelet added reviewers: MaskRay, courbet, ddunbar.
Herald added subscribers: StephenFan, pengfei, hiraditya, nemanjai.
Herald added a project: All.
gchatelet requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

13 years ago, the following commit changed the way alignment was represented moving from Log2 of alignment to plain number of bytes.
https://github.com/llvm/llvm-project/commit/6a715dccdf596d39d2b480beac48385f1fa73219#diff-d6520fbf2b954c8679d0835ca53c6ae423b4354e5e517dcfb043446042249f98

This patch introduced a regression, relevant lines are the following:
Before : `if (Pow2Alignment != 0) OS << ',' << Pow2Alignment;`
After  : `if (ByteAlignment != 0) OS << ',' << Log2_32(ByteAlignment);`

This impacted the `emitCommonSymbol` and `emitZeroFill` functions. Functions added later on like `emitLocalCommonSymbol` indeed output the alignment only when it is greater than one. e.g.,
https://github.com/llvm/llvm-project/blob/10d183b889daab4512d476c1645d24d4e8946e8c/llvm/lib/MC/MCAsmStreamer.cpp#L997
https://github.com/llvm/llvm-project/blob/10d183b889daab4512d476c1645d24d4e8946e8c/llvm/lib/MC/MCAsmStreamer.cpp#L1059

This patch makes the behavior uniform for all sections and outputs alignment only when greater than one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138778

Files:
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/test/CodeGen/ARM/2010-12-15-elf-lcomm.ll
  llvm/test/CodeGen/ARM/elf-lcomm-align.ll
  llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
  llvm/test/CodeGen/X86/global-sections.ll
  llvm/test/MC/COFF/comm.ll


Index: llvm/test/MC/COFF/comm.ll
===================================================================
--- llvm/test/MC/COFF/comm.ll
+++ llvm/test/MC/COFF/comm.ll
@@ -9,5 +9,5 @@
 ; CHECK: .lcomm	_a,1
 ; CHECK: .lcomm	_b,8,8
 ; .comm uses log2 alignment
-; CHECK: .comm	_c,1,0
+; CHECK: .comm	_c,1
 ; CHECK: .comm	_d,8,3
Index: llvm/test/CodeGen/X86/global-sections.ll
===================================================================
--- llvm/test/CodeGen/X86/global-sections.ll
+++ llvm/test/CodeGen/X86/global-sections.ll
@@ -317,9 +317,9 @@
 @G17 = internal global i8 0
 ; LINUX: .type	G17, at object
 ; LINUX: .local	G17
-; LINUX: .comm	G17,1,1
+; LINUX: .comm	G17,1
 
-; DARWIN: .zerofill __DATA,__bss,_G17,1,0
+; DARWIN: .zerofill __DATA,__bss,_G17,1
 
 ; LINUX-SECTIONS: .type	G17, at object
 ; LINUX-SECTIONS: .section	.bss.G17,"aw", at nobits
Index: llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
+++ llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
@@ -141,11 +141,11 @@
 
 ; CHECK-NEXT: .comm   a[RW],4,2
 ; CHECK-NEXT: .comm   b[RW],8,3
-; CHECK-NEXT: .comm   c[RW],2,1
+; CHECK-NEXT: .comm   c[RW],2
 ; CHECK-NEXT: .comm   d[RW],8,3
 ; CHECK-NEXT: .comm   f[RW],4,2
 ; CHECK-NEXT: .comm   over_aligned_comm[RW],8,5
-; CHECK-NEXT: .comm   array[RW],33,0
+; CHECK-NEXT: .comm   array[RW],33
 
 ; OBJ:      File: {{.*}}aix-xcoff-data.ll.tmp.o
 ; OBJ-NEXT: Format: aixcoff-rs6000
Index: llvm/test/CodeGen/ARM/elf-lcomm-align.ll
===================================================================
--- llvm/test/CodeGen/ARM/elf-lcomm-align.ll
+++ llvm/test/CodeGen/ARM/elf-lcomm-align.ll
@@ -6,7 +6,7 @@
 
 ; .lcomm doesn't support alignment, so we always use .local/.comm.
 ; CHECK: .local c
-; CHECK-NEXT: .comm c,1,1
+; CHECK-NEXT: .comm c,1
 ; CHECK: .local x
 ; CHECK-NEXT: .comm x,4,4
 
Index: llvm/test/CodeGen/ARM/2010-12-15-elf-lcomm.ll
===================================================================
--- llvm/test/CodeGen/ARM/2010-12-15-elf-lcomm.ll
+++ llvm/test/CodeGen/ARM/2010-12-15-elf-lcomm.ll
@@ -11,7 +11,7 @@
 
 ; ASM:          .type   array00,%object         @ @array00
 ; ASM-NEXT:     .local  array00
-; ASM-NEXT:     .comm   array00,80,1
+; ASM-NEXT:     .comm   array00,80
 ; ASM-NEXT:     .type   sum,%object  @ @sum
 
 
Index: llvm/lib/MC/MCAsmStreamer.cpp
===================================================================
--- llvm/lib/MC/MCAsmStreamer.cpp
+++ llvm/lib/MC/MCAsmStreamer.cpp
@@ -972,7 +972,7 @@
   Symbol->print(OS, MAI);
   OS << ',' << Size;
 
-  if (ByteAlignment != 0) {
+  if (ByteAlignment > 1) {
     if (MAI->getCOMMDirectiveAlignmentIsInBytes())
       OS << ',' << ByteAlignment;
     else
@@ -1030,7 +1030,7 @@
     OS << ',';
     Symbol->print(OS, MAI);
     OS << ',' << Size;
-    if (ByteAlignment != 0)
+    if (ByteAlignment > 1)
       OS << ',' << Log2_32(ByteAlignment);
   }
   EmitEOL();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138778.478153.patch
Type: text/x-patch
Size: 2966 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221128/db2e1f0c/attachment.bin>