[AArch64] Move branch relaxation after bbsection assignment

[dhoekwater]

Summary

Because branch relaxation needs to factor in if branches target
a block in the same section or a different one, it needs to run
after the Basic Block Sections / Machine Function Splitting passes.

Because Jump table compression relies on block offsets remaining
fixed after the table is compressed, we must also move the JT
compression pass.

[dhoekwater]

Rename the pass name and give it descriptive documentation.

[dhoekwater]

Update a couple tests to account for RenumberBlocks not having run yet.

[dhoekwater]

Remove FIXMEs. We probably will want to rename preEmitPass functions to something
more reflective of the functions' purposes.

[arsenm]

llvm/include/llvm/CodeGen/TargetPassConfig.h

443

This is quite the claim, given that the wonderfully named "addPreEmitPass2" already exist

444

I don't understand this, how can such a restriction exist on the use of this?

[dhoekwater]

llvm/include/llvm/CodeGen/TargetPassConfig.h

443

I assumed that because addPreEmitPass2 happens after branch relaxation on AArch64 that it’s true on other targets, but ARM shows that’s not true. I’ll have to update this to loosen the guarantees to “happens after basic block sections are assigned”.

444

It can’t for the same reasons that your other comment addresses, so I’ll have to remove this. I named and documentation this function as I did to convey some of the invariants of the pipeline at the point they’re called, but now I’m confused. Are there _any_ guarantees between addPreEmitPass and addPreEmitPass2 other than Pass 2 happening after pass 1?

[dhoekwater]

Modify function name and documentation to loosen its guarantees.

Because targets can perform instruction insertion wherever they
like, we can't guarantee that this is a safe place to perform
branch relaxation for all targets.

[dhoekwater]

Make sure to push *all* changes.

[dhoekwater]

llvm/include/llvm/CodeGen/TargetPassConfig.h

444

Thanks for the feedback; the updated function name and documentation should be more accurate, if a little nondescript.

[nickdesaulniers]

Please run git clang-format HEAD~ to format the code.

llvm/test/CodeGen/AArch64/jti-correct-datatype.mir

1 ↗

(On Diff #536475)

is this the next pass that would have run after branch-relaxation before this change?

[dhoekwater]

Format code and remove changes to unaffected tests

[dhoekwater]

Turns out one of those tests was necessary.

[arsenm]

Not sure I follow how the test changes capture this but seems fine

llvm/test/DebugInfo/AArch64/fallthrough-branch.ll

10

How did this test escape opaquification?

[dhoekwater]

llvm/test/CodeGen/AArch64/jti-correct-datatype.mir

1 ↗

(On Diff #536475)

Short answer: it turns out I don't actually need to modify this test, so I'm removing the change from the patch.

Long answer:
Before this change, aarch64-jump-tables isn't the next pass that would have run after branch-relaxation, but it is the next pass that modifies jump table datatypes. IIUC, this test applies to the jump compression pass and CodeGen passes afterwards, so starting it before aarch64-jump-tables should be alright.

llvm/test/DebugInfo/AArch64/fallthrough-branch.ll

10

It's not exactly self-documenting, that's for sure.

llvm/include/llvm/CodeGen/TargetPassConfig.h

protected:
 
   /// Add standard basic block placement passes.
   virtual void addBlockPlacement();
 
   /// This pass may be implemented by targets that want to run passes
   /// immediately before machine code is emitted.
   virtual void addPreEmitPass() { }
 
+  /// This pass may be implemented by targets that want to run passes
+  /// immediately after basic block sections are assigned.
+  virtual void addPostBBSections() {}
+
   /// Targets may add passes immediately before machine code is emitted in this
   /// callback. This is called even later than `addPreEmitPass`.
   // FIXME: Rename `addPreEmitPass` to something more sensible given its actual
   // position and remove the `2` suffix here as this callback is what
   // `addPreEmitPass` *should* be but in reality isn't.
   virtual void addPreEmitPass2() {}
 
   /// Utilities for targets to add passes to the pass manager.

llvm/lib/CodeGen/TargetPassConfig.cpp

if (!ProfileFile.empty()) {
         WithColor::warning()
             << "Using AutoFDO without FSDiscriminator for MFS may regress "
                "performance.";
       }
     }
     addPass(createMachineFunctionSplitterPass());
   }
 
+  addPostBBSections();
+
   if (!DisableCFIFixup && TM->Options.EnableCFIFixup)
     addPass(createCFIFixup());
 
   PM->add(createStackFrameLayoutAnalysisPass());
 
   // Add passes that directly emit MI after all other MI passes.
   addPreEmitPass2();
 

llvm/lib/Target/AArch64/AArch64TargetMachine.cpp

public:
   bool addRegBankSelect() override;
   bool addGlobalInstructionSelect() override;
   void addMachineSSAOptimization() override;
   bool addILPOpts() override;
   void addPreRegAlloc() override;
   void addPostRegAlloc() override;
   void addPreSched2() override;
   void addPreEmitPass() override;
+  void addPostBBSections() override;
   void addPreEmitPass2() override;
 
   std::unique_ptr<CSEConfigBase> getCSEConfig() const override;
 };
 
 } // end anonymous namespace
 
 TargetTransformInfo
if (TM->getOptLevel() >= CodeGenOpt::Aggressive &&
       EnableAArch64CopyPropagation)
     addPass(createMachineCopyPropagationPass(true));
 
   addPass(createAArch64A53Fix835769());
 
   if (EnableBranchTargets)
     addPass(createAArch64BranchTargetsPass());
 
-  // Relax conditional branch instructions if they're otherwise out of
-  // range of their destination.
-  if (BranchRelaxation)
-    addPass(&BranchRelaxationPassID);
-
   if (TM->getTargetTriple().isOSWindows()) {
     // Identify valid longjmp targets for Windows Control Flow Guard.
     addPass(createCFGuardLongjmpPass());
     // Identify valid eh continuation targets for Windows EHCont Guard.
     addPass(createEHContGuardCatchretPass());
   }
 
-  if (TM->getOptLevel() != CodeGenOpt::None && EnableCompressJumpTables)
-    addPass(createAArch64CompressJumpTablesPass());
-
   if (TM->getOptLevel() != CodeGenOpt::None && EnableCollectLOH &&
       TM->getTargetTriple().isOSBinFormatMachO())
     addPass(createAArch64CollectLOHPass());
 }
 
+void AArch64PassConfig::addPostBBSections() {
+  // Relax conditional branch instructions if they're otherwise out of
+  // range of their destination.
+  if (BranchRelaxation)
+    addPass(&BranchRelaxationPassID);
+
+  if (TM->getOptLevel() != CodeGenOpt::None && EnableCompressJumpTables)
+    addPass(createAArch64CompressJumpTablesPass());
+}
+
 void AArch64PassConfig::addPreEmitPass2() {
   // SVE bundles move prefixes with destructive operations. BLR_RVMARKER pseudo
   // instructions are lowered to bundles as well.
   addPass(createUnpackMachineBundles(nullptr));
 }
 
 MachineFunctionInfo *AArch64TargetMachine::createMachineFunctionInfo(
     BumpPtrAllocator &Allocator, const Function &F,

llvm/test/CodeGen/AArch64/O0-pipeline.ll

 ; CHECK-NEXT:       AArch64 Indirect Thunks
 ; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       Analyze Machine Code For Garbage Collection
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       Workaround A53 erratum 835769 pass
 ; CHECK-NEXT:       AArch64 Branch Targets
-; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
+; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       Insert CFI remember/restore state instructions
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Stack Frame Layout Analysis
 ; CHECK-NEXT:       Unpack machine instruction bundles
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       AArch64 Assembly Printer
 ; CHECK-NEXT:       Free MachineFunction
 
 define void @f() {
   ret void
 }

llvm/test/CodeGen/AArch64/O3-pipeline.ll

 ; CHECK-NEXT:       Branch Probability Basic Block Placement
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       AArch64 load / store optimization pass
 ; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       Workaround A53 erratum 835769 pass
 ; CHECK-NEXT:       AArch64 Branch Targets
-; CHECK-NEXT:       Branch relaxation pass
-; CHECK-NEXT:       AArch64 Compress Jump Tables
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:     Machine Outliner
 ; CHECK-NEXT:     FunctionPass Manager
+; CHECK-NEXT:       Branch relaxation pass
+; CHECK-NEXT:       AArch64 Compress Jump Tables
 ; CHECK-NEXT:       Insert CFI remember/restore state instructions
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Stack Frame Layout Analysis
 ; CHECK-NEXT:       Unpack machine instruction bundles
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       AArch64 Assembly Printer

llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll

 ; Verify that we only populate MachineBFI on behalf of ORE when hotness is
 ; requested.  (This hard-codes the previous pass to the Assembly Printer,
 ; please adjust accordingly.)
 
 ; HOTNESS:      Freeing Pass 'Machine Outliner'
 ; HOTNESS-NEXT:  Executing Pass 'Function Pass Manager'
 ; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
 ; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; HOTNESS-NEXT: Executing Pass 'Branch relaxation pass' on Function 'empty_func'
+; HOTNESS-NEXT: Freeing Pass 'Branch relaxation pass' on Function 'empty_func'
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
+; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; HOTNESS-NEXT: Executing Pass 'AArch64 Compress Jump Tables' on Function 'empty_func'
+; HOTNESS-NEXT: Freeing Pass 'AArch64 Compress Jump Tables' on Function 'empty_func'
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
+; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
 ; HOTNESS-NEXT: Executing Pass 'Insert CFI remember/restore state instructions' on Function 'empty_func'
 ; HOTNESS-NEXT: Freeing Pass 'Insert CFI remember/restore state instructions' on Function 'empty_func'
 ; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
 ; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
 ; HOTNESS:      Executing Pass 'Unpack machine instruction bundles'
 ; HOTNESS-NEXT: Freeing Pass 'Unpack machine instruction bundles'
 ; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
 ; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'

 
 ; HOTNESS: arm64-summary-remarks.ll:5:0: 1 instructions in function (hotness: 33)
 
 
 ; NO_HOTNESS:      Freeing Pass 'Machine Outliner'
 ; NO_HOTNESS-NEXT:  Executing Pass 'Function Pass Manager'
 ; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
 ; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; NO_HOTNESS-NEXT: Executing Pass 'Branch relaxation pass' on Function 'empty_func'
+; NO_HOTNESS-NEXT: Freeing Pass 'Branch relaxation pass' on Function 'empty_func'
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
+; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; NO_HOTNESS-NEXT: Executing Pass 'AArch64 Compress Jump Tables' on Function 'empty_func'
+; NO_HOTNESS-NEXT: Freeing Pass 'AArch64 Compress Jump Tables' on Function 'empty_func'
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
+; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
 ; NO_HOTNESS-NEXT: Executing Pass 'Insert CFI remember/restore state instructions' on Function 'empty_func'
 ; NO_HOTNESS-NEXT: Freeing Pass 'Insert CFI remember/restore state instructions' on Function 'empty_func'
 ; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
 ; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
 ; NO_HOTNESS:      Executing Pass 'Unpack machine instruction bundles'
 ; NO_HOTNESS-NEXT: Freeing Pass 'Unpack machine instruction bundles'
 ; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
 ; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'

llvm/test/CodeGen/AArch64/seh_funclet_x1.ll

 ; RUN: llc -o - %s -verify-machineinstrs -mtriple=aarch64-windows | FileCheck %s
 
 ; Windows runtime passes the establisher frame as the second argument to the
 ; termination handler.  Check that we copy it into fp.
 
-; CHECK:      ?dtor$3@?0?main@4HA":
-; CHECK:      .seh_proc "?dtor$3@?0?main@4HA"
+; CHECK:      ?dtor$6@?0?main@4HA":
+; CHECK:      .seh_proc "?dtor$6@?0?main@4HA"
 ; CHECK:      stp     x29, x30, [sp, #-16]!   // 16-byte Folded Spill
 ; CHECK-NEXT: .seh_save_fplr_x 16
 ; CHECK-NEXT: .seh_endprologue
 ; CHECK-NEXT: mov     x29, x1
 
 target datalayout = "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-unknown-windows-msvc19.15.26732"
 

llvm/test/CodeGen/AArch64/wineh-try-catch.ll

 ; This is the label for the throw that is encoded in the ip2state.
 ; We are inside the try block, where we make a call to func2
 ; CHECK-LABEL: .Ltmp0:
 ; CHECK:       bl      "?func2@@YAHXZ
 
 ; CHECK:        [[CATCHRETDEST:.LBB0_[0-9]+]]:      // %catchret.dest
 
 ; Check the catch funclet.
-; CHECK-LABEL: "?catch$2@?0??func@@YAHXZ@4HA":
+; CHECK-LABEL: "?catch$4@?0??func@@YAHXZ@4HA":
 
 ; Check that the stack space is allocated only for the callee saved registers.
 ; CHECK:       stp     x19, x20, [sp, #-48]!
 ; CHECK:       str     x21, [sp, #16]
 ; CHECK:       str     x28, [sp, #24]
 ; CHECK:       stp     x29, x30, [sp, #32]
 ; CHECK:       add     x20, x19, #12
 
 ; Check that there are no further stack updates.
 ; CHECK-NOT:   sub     sp, sp
 
 ; Check that the stack address passed to init2 is off the frame pointer, and
 ; that it matches the address of B in the parent function.
 ; CHECK:       sub     x0, x29, [[B_OFFSET]]
 ; CHECK:       bl      "?init2@@YAXPEAH@Z"
 
 ; Check that are storing x back to the same location off the frame pointer as in
 ; the parent function.
 ; CHECK:       stur    w8, [x29, [[X_OFFSET]]]
 
 ; Check that the funclet branches back to the catchret destination
-; CHECK:       adrp    x0, .LBB0_3
+; CHECK:       adrp    x0, .LBB0_2
 ; CHECK-NEXT:  add     x0, x0, [[CATCHRETDEST]]
 
 
 ; Now check that the offset of the unwind help object from the stack pointer on
 ; entry to func is encoded in cppxdata that is passed to __CxxFrameHandler3.  As
 ; computed above, this comes to -16.
 ; CHECK-LABEL:        "$cppxdata$?func@@YAHXZ":
 ; CHECK-NEXT:         .word   429065506               // MagicNumber
 ; CHECK-NEXT:         .word   2                       // MaxState
 ; CHECK-NEXT:         .word   ("$stateUnwindMap$?func@@YAHXZ")@IMGREL // UnwindMap
 ; CHECK-NEXT:         .word   1                       // NumTryBlocks
 ; CHECK-NEXT:         .word   ("$tryMap$?func@@YAHXZ")@IMGREL // TryBlockMap
 ; CHECK-NEXT:         .word   4                       // IPMapEntries
 ; CHECK-NEXT:         .word   ("$ip2state$?func@@YAHXZ")@IMGREL // IPToStateXData
 ; CHECK-NEXT:         .word   -16                     // UnwindHelp
 
 ; UNWIND: Function: ?func@@YAHXZ (0x0)
 ; UNWIND: Prologue [
 ; UNWIND-NEXT: ; add fp, sp, #32
 ; UNWIND-NEXT: ; stp x29, x30, [sp, #32]
 ; UNWIND-NEXT: ; str x28, [sp, #24]
 ; UNWIND-NEXT: ; str x21, [sp, #16]
 ; UNWIND-NEXT: ; stp x19, x20, [sp, #-64]!
 ; UNWIND-NEXT: ; end
-; UNWIND: Function: ?catch$2@?0??func@@YAHXZ@4HA
+; UNWIND: Function: ?catch$4@?0??func@@YAHXZ@4HA
 ; UNWIND: Prologue [
 ; UNWIND-NEXT: ; stp x29, x30, [sp, #32]
 ; UNWIND-NEXT: ; str x28, [sp, #24]
 ; UNWIND-NEXT: ; str x21, [sp, #16]
 ; UNWIND-NEXT: ; stp x19, x20, [sp, #-48]!
 ; UNWIND-NEXT: ; end
 
 target datalayout = "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128"

llvm/test/DebugInfo/AArch64/fallthrough-branch.ll

 ; RUN: llc -O0 -stop-before=livedebugvalues < %s | FileCheck %s
 
 ; ModuleID = '/tmp/t.o'
 source_filename = "/tmp/t.o"
 target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
 target triple = "arm64-apple-macosx11.0.0"
 
 define swiftcc void @"$s1t1f1bySb_tF"(i1 %0) !dbg !35 {
   %2 = alloca i1, align 8
   %3 = bitcast i1* %2 to i8*
   call void @llvm.memset.p0i8.i64(i8* align 8 %3, i8 0, i64 1, i1 false)
   store i1 %0, i1* %2, align 8, !dbg !37
-; CHECK:   B %bb.1, debug-location !{{[0-9]+}}
+; CHECK:   B %[[BB4:bb\.[0-9]+]], debug-location !{{[0-9]+}}
   br i1 %0, label %4, label %5, !dbg !38
 
 4:                                                ; preds = %1
+; CHECK:   [[BB4]]
 ; Check that at -O0 the branches and their debug locations are not eliminated.
-; CHECK:   B %bb.3, debug-location !{{[0-9]+}}
+; CHECK:   B %[[BB6:bb\.[0-9]+]], debug-location !{{[0-9]+}}
   br label %6, !dbg !39
 
 5:                                                ; preds = %1
-; CHECK:   B %bb.3, debug-location !{{[0-9]+}}
+; CHECK:   B %[[BB6]], debug-location !{{[0-9]+}}
   br label %6, !dbg !40
 
 6:                                                ; preds = %4, %5
   ret void, !dbg !39
 }
 
 ; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
 declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #1