[PATCH] D137535: [CodeGen][BasicBlockSections] Fix wrong alignment directive placement in basic block section cases

Sinan Lin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 7 00:25:43 PST 2022


sinan created this revision.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
sinan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

MachineBlockPlacement pass sets an alignment attribute to the loop header MBB and this attribute will lead to an alignment directive during emitting asm. In the case of the basic block section, the alignment directive is put before the section label alignment, and therefore the alignment is set to the predecessor of the loop header, which is not what we expect and increases the code size (both inserting nop and set section alignment).

e.g. reify.__part.2 is the loop header and `.p2align 4, 0x90` should be moved below `.section .text.reify.reify.__part.2`.

  	.section	.text.reify.reify.__part.1,"ax", at progbits
  reify.__part.1:                         # %if.end
  	.cfi_startproc
  	.cfi_def_cfa %rsp, 8
  	movq	(%rdi), %rcx
  	movq	24(%rcx), %rdx
  	leaq	1(%rdx), %rax
  .Ltmp1:
  	#DEBUG_VALUE: reify:key <- $rax
  	cmpq	16(%rcx), %rdx
  	jle	reify.__part.3
  	jmp	reify.__part.2
  .LBB_END0_1:
  	.size	reify.__part.1, .LBB_END0_1-reify.__part.1
  	.cfi_endproc
  	.p2align	4, 0x90
  	.section	.text.reify.reify.__part.2,"ax", at progbits
  reify.__part.2:                         # %while.body
                                          # =>This Inner Loop Header: Depth=1
  	.cfi_startproc
  	.cfi_def_cfa %rsp, 8



  [Nr] Name                       Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 3] .text.reify                PROGBITS        0000000000000000 000040 00000f 00  AX  0   0 16
  [ 5] .text.reify.reify.__part.1 PROGBITS        0000000000000000 000050 000020 00  AX  0   0 16
  [ 7] .text.reify.reify.__part.2 PROGBITS        0000000000000000 000070 000024 00  AX  0   0  1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137535

Files:
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/X86/align-basic-block-sections.ll


Index: llvm/test/CodeGen/X86/align-basic-block-sections.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/align-basic-block-sections.ll
@@ -0,0 +1,66 @@
+; RUN: llc < %s -march=x86-64 --basic-block-sections=all --unique-basic-block-section-names | FileCheck %s --check-prefix=ASM
+; RUN: llc < %s -march=x86-64 --basic-block-sections=all --unique-basic-block-section-names -filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=OBJ
+
+define void @maxArray(double* %x, double* %y) {
+; ASM-LABEL: maxArray:
+; ASM:        .section	.text.maxArray.maxArray.__part.2,"ax", at progbits
+; ASM-NEXT: maxArray.__part.2:
+; ASM-NEXT:   .cfi_startproc
+; ASM-NEXT:   .cfi_def_cfa %rsp, 8
+; ASM-NEXT:   xorl	%eax, %eax
+; ASM-NEXT:   jmp	maxArray.__part.3
+; ASM-NEXT: .LBB_END0_2:
+; ASM-NEXT: .size	maxArray.__part.2, .LBB_END0_2-maxArray.__part.2
+; ASM-NEXT: .cfi_endproc
+; ASM-NEXT: .section	.text.maxArray.maxArray.__part.3,"ax", at progbits
+; ASM-NEXT: .p2align	4, 0x90
+; ASM-NEXT: maxArray.__part.3: # %vector.body
+; ASM-NEXT:   # =>This Inner Loop Header: Depth=1
+
+entry:
+  %x_addr = ptrtoint double* %x to i64
+  %y_addr = ptrtoint double* %y to i64
+  %nonull = icmp eq i64 %y_addr, %x_addr
+  br i1 %nonull, label %preheader, label %vector.body
+preheader:
+  %random_idx = sub i64 %x_addr, 20
+  br label %vector.body
+vector.body:
+  %index = phi i64 [ %random_idx, %preheader ], [ 0, %entry ], [ %index.next, %vector.body ]
+  %gepx = getelementptr inbounds double, double* %x, i64 %index
+  %gepy = getelementptr inbounds double, double* %y, i64 %index
+  %xptr = bitcast double* %gepx to <2 x double>*
+  %yptr = bitcast double* %gepy to <2 x double>*
+  %xval = load <2 x double>, <2 x double>* %xptr, align 8
+  %yval = load <2 x double>, <2 x double>* %yptr, align 8
+  %cmp = fcmp ogt <2 x double> %yval, %xval
+  %max = select <2 x i1> %cmp, <2 x double> %yval, <2 x double> %xval
+  %xptr_again = bitcast double* %gepx to <2 x double>*
+  store <2 x double> %max, <2 x double>* %xptr_again, align 8
+  %index.next = add i64 %index, 2
+  %done = icmp eq i64 %index.next, %y_addr
+  br i1 %done, label %exit, label %vector.body
+
+exit:
+  ret void
+}
+
+; OBJ-LABEL: Sections:
+; OBJ-LABEL:   - Name:            .text
+; OBJ-LABEL:     Type:            SHT_PROGBITS
+; OBJ-LABEL:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+; OBJ-LABEL:     AddressAlign:    0x4
+; OBJ-LABEL:   - Name:            .text.maxArray
+; OBJ-LABEL:     Type:            SHT_PROGBITS
+; OBJ-LABEL:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+; OBJ-LABEL:     AddressAlign:    0x10
+; OBJ-LABEL:     Content:         4839FE0F8500000000E900000000
+; OBJ-LABEL:   - Name:            .text.maxArray.maxArray.__part.1
+; OBJ-LABEL:     Type:            SHT_PROGBITS
+; OBJ-LABEL:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+; OBJ-LABEL:     AddressAlign:    0x1
+; OBJ-LABEL:     Content:         31C0E900000000
+; OBJ-LABEL:     - Name:            .text.maxArray.maxArray.__part.3
+; OBJ-LABEL:     Type:            SHT_PROGBITS
+; OBJ-LABEL:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+; OBJ-LABEL:     AddressAlign:    0x10
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3655,11 +3655,6 @@
     }
   }
 
-  // Emit an alignment directive for this block, if needed.
-  const Align Alignment = MBB.getAlignment();
-  if (Alignment != Align(1))
-    emitAlignment(Alignment, nullptr, MBB.getMaxBytesForAlignment());
-
   // Switch to a new section if this basic block must begin a section. The
   // entry block is always placed in the function section and is handled
   // separately.
@@ -3670,6 +3665,11 @@
     CurrentSectionBeginSym = MBB.getSymbol();
   }
 
+  // Emit an alignment directive for this block, if needed.
+  const Align Alignment = MBB.getAlignment();
+  if (Alignment != Align(1))
+    emitAlignment(Alignment, nullptr, MBB.getMaxBytesForAlignment());
+
   // If the block has its address taken, emit any labels that were used to
   // reference the block.  It is possible that there is more than one label
   // here, because multiple LLVM BB's may have been RAUW'd to this block after


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137535.473583.patch
Type: text/x-patch
Size: 4356 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221107/9fd94195/attachment.bin>


More information about the llvm-commits mailing list