[CodeGen] Add AArch64 behavior to existing MFS tests

[dhoekwater]

Summary

X86 and AArch64 have some small differences in output because of non-MFS
factors (such as encoding function calls as callq vs bl). Add
AArch64 checks for tests where output varies between platforms even
if MFS logic doesn't. Add a debug flag to run MFS on any target.
Fix the encoding of nops in bbsections.

Without this change, there is no way to test that MFS functions correctly on AArch64.

[dhoekwater]

Rebase onto other MFS tests and fix the BBsections code that makes the test crash

[dhoekwater]

Don't warn for AArch64 if mfs-ignore-triple is enabled

[rahmanl]

llvm/lib/CodeGen/BasicBlockSections.cpp

294 ↗

(On Diff #551202)

Is this because of ARM? Maybe add a comment to explain the rationale.

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

73

How about introducing a permanent option named SkipUnsupportedTriple and default it True so we can use it for any future architectures?

[dhoekwater]

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

73

I'm onboard with making this a permanent flag for testing purposes only, but I'm not sure about inverting and defaulting to true. Isn't it standard to have default-false hidden flags that enable some special behavior (as opposed to default-true hidden flags that enable the default behavior)?

For example, most of the cl::opt<bool> options in llvm/lib/Analysis/InlineCost.cpp follow this pattern.

[rahmanl]

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

73

Up to you. I thought this naming better matches with the current code (UnsupportedTriple and isSupportedTriple). Ignore doesn't fully convey what the flag actually does.

[dhoekwater]

Add comments

[dhoekwater]

Rename the flag to split functions on unsupported targets

[rahmanl]

llvm/lib/CodeGen/BasicBlockSections.cpp

294 ↗

(On Diff #551202)

I see, this is target specific now. Although this code was target-specific before too, but we only wanted code for X86. Do you think we should implement insertNoop (https://llvm.org/doxygen/classllvm_1_1TargetInstrInfo.html#a4ee57d5d6295dfeb44f3b55301b20020) for X86 and AArch64 now?

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

71

Even better naming than mine!

[dhoekwater]

llvm/lib/CodeGen/BasicBlockSections.cpp

294 ↗

(On Diff #551202)

Yes! That's exactly what I was looking for, but I couldn't find the function since I was looking for something like "addNop". Should we do that in this patch or should I make another?

[rahmanl]

llvm/lib/CodeGen/BasicBlockSections.cpp

294 ↗

(On Diff #551202)

Right. Separate the entire avoidZeroOffsetLandingPad changes into its own patch if you can. You might get new reviewers when implement TargetInstrInfo.

[dhoekwater]

llvm/lib/CodeGen/BasicBlockSections.cpp

294 ↗

(On Diff #551202)

Unfortunately, we need AArch64 tests to test the avoidZeroOffsetLandingPad changes, and we need the changes so that the tests don't crash on AArch64.

[dhoekwater]

Implement the AArch64 InsertNoop hook and use it so that Arm tests don't crash

[shenhan]

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

72

nit: "mfs" already contains "split", so mfs-split-xxx seems a little bit redundant. "mfs-allow-unsupported-triple" seems more aligned with the variable name.

150

Thanks for fixing the format.

[thakis]

This breaks tests on windows: http://45.33.8.238/win/82918/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[thakis]

reverted in 3d22dac6c3b97d7bb92f243886dfb0d32a5c42e9 for now

[dhoekwater]

Relanded in 90ab85a1b2e72f63039fadf6669b23f52192defd

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

cl::desc(
         "Minimum number of times a block must be executed to be retained."),
     cl::init(1), cl::Hidden);
 
 static cl::opt<bool> SplitAllEHCode(
     "mfs-split-ehcode",
     cl::desc("Splits all EH code and it's descendants by default."),
     cl::init(false), cl::Hidden);
 
+static cl::opt<bool> AllowUnsupportedTriple(
+    "mfs-split-unsupported-triple",
+    cl::desc(
+        "Splits functions even if the target triple isn't supported. This is "
+        "testing flag for targets that don't yet support function splitting."),
+    cl::init(false), cl::Hidden);
+
 namespace {
 
 class MachineFunctionSplitter : public MachineFunctionPass {
 public:
   static char ID;
   MachineFunctionSplitter() : MachineFunctionPass(ID) {
     initializeMachineFunctionSplitterPass(*PassRegistry::getPassRegistry());
   }
if (!Count)
       return false;
   }
 
   return (*Count < ColdCountThreshold);
 }
 
 bool MachineFunctionSplitter::doInitialization(Module &M) {
   StringRef T = M.getTargetTriple();
-  if (!isSupportedTriple(Triple(T))) {
+  if (!isSupportedTriple(Triple(T)) && !AllowUnsupportedTriple) {
     UnsupportedTriple = true;
-    M.getContext().diagnose(
-        DiagnosticInfoMachineFunctionSplit(T, DS_Warning));
+    M.getContext().diagnose(DiagnosticInfoMachineFunctionSplit(T, DS_Warning));
     return false;
   }
   return MachineFunctionPass::doInitialization(M);
 }
 
 bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
   if (UnsupportedTriple)
     return false;

llvm/lib/Target/AArch64/AArch64InstrInfo.h

public:
   reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;
   bool canInsertSelect(const MachineBasicBlock &, ArrayRef<MachineOperand> Cond,
                        Register, Register, Register, int &, int &,
                        int &) const override;
   void insertSelect(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
                     const DebugLoc &DL, Register DstReg,
                     ArrayRef<MachineOperand> Cond, Register TrueReg,
                     Register FalseReg) const override;
+
+  void insertNoop(MachineBasicBlock &MBB,
+                  MachineBasicBlock::iterator MI) const override;
+
   MCInst getNop() const override;
 
   bool isSchedulingBoundary(const MachineInstr &MI,
                             const MachineBasicBlock *MBB,
                             const MachineFunction &MF) const override;
 
   /// analyzeCompare - For a comparison instruction, return the source registers
   /// in SrcReg and SrcReg2, and the value it compares against in CmpValue.

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

if (Status & AArch64FrameOffsetCanUpdate) {
 
     MI.getOperand(ImmIdx).ChangeToImmediate(NewOffset);
     return !Offset;
   }
 
   return false;
 }
 
+void AArch64InstrInfo::insertNoop(MachineBasicBlock &MBB,
+                                  MachineBasicBlock::iterator MI) const {
+  DebugLoc DL;
+  BuildMI(MBB, MI, DL, get(AArch64::HINT)).addImm(0);
+}
+
 MCInst AArch64InstrInfo::getNop() const {
   return MCInstBuilder(AArch64::HINT).addImm(0);
 }
 
 // AArch64 supports MachineCombiner.
 bool AArch64InstrInfo::useMachineCombiner() const { return true; }
 
 // True when Opc sets flag

llvm/test/CodeGen/Generic/machine-function-splitter.ll

 ; REQUIRES: aarch64-registered-target
 ; REQUIRES: x86-registered-target
 
 ; COM: Machine function splitting with FDO profiles
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s -check-prefixes=MFS-DEFAULTS,MFS-DEFAULTS-X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -mfs-psi-cutoff=0 -mfs-count-threshold=2000 | FileCheck %s --dump-input=always -check-prefixes=MFS-OPTS1,MFS-OPTS1-X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -mfs-psi-cutoff=950000 | FileCheck %s -check-prefixes=MFS-OPTS2,MFS-OPTS2-X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -mfs-split-ehcode | FileCheck %s -check-prefixes=MFS-EH-SPLIT,MFS-EH-SPLIT-X86
 
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-split-unsupported-triple | FileCheck %s -check-prefixes=MFS-DEFAULTS,MFS-DEFAULTS-AARCH64
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-split-unsupported-triple -mfs-psi-cutoff=0 -mfs-count-threshold=2000 | FileCheck %s --dump-input=always -check-prefixes=MFS-OPTS1,MFS-OPTS1-AARCH64
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-split-unsupported-triple -mfs-psi-cutoff=950000 | FileCheck %s -check-prefixes=MFS-OPTS2,MFS-OPTS2-AARCH64
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-split-unsupported-triple -mfs-split-ehcode | FileCheck %s -check-prefixes=MFS-EH-SPLIT,MFS-EH-SPLIT-AARCH64
+
 ; COM: Machine function splitting with AFDO profiles
 ; RUN: sed 's/InstrProf/SampleProfile/g' %s > %t.ll
 ; RUN: llc < %t.ll -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s --check-prefix=FSAFDO-MFS
 ; RUN: llc < %t.ll -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s --check-prefix=FSAFDO-MFS2
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -debug-pass=Structure -fs-profile-file=%S/Inputs/fsloader-mfs.afdo -enable-fs-discriminator=true -improved-fs-discriminator=true -split-machine-functions 2>&1 | FileCheck %s --check-prefix=MFS_ON
 ; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -debug-pass=Structure -fs-profile-file=%S/Inputs/fsloader-mfs.afdo -enable-fs-discriminator=true -improved-fs-discriminator=true -split-machine-functions 2>&1 | FileCheck %s --check-prefix=MFS_OFF
 
+; RUN: llc < %t.ll -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-split-unsupported-triple | FileCheck %s --check-prefix=FSAFDO-MFS
+; RUN: llc < %t.ll -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-split-unsupported-triple | FileCheck %s --check-prefix=FSAFDO-MFS2
+
 ;; Check that MFS is on for X86 targets.
 ; MFS_ON: Machine Function Splitter Transformation
 ; MFS_ON_NO: warning: -fsplit-machine-functions is not valid for
 ;; Check that MFS is not on for non-X86 targets.
 ; MFS_OFF: warning: -fsplit-machine-functions is not valid for
 ; MFS_OFF_NO: Machine Function Splitter Transformation
 
 define void @foo1(i1 zeroext %0) nounwind !prof !14 !section_prefix !15 {
 ;; Check that cold block is moved to .text.split.
 ; MFS-DEFAULTS-LABEL:         foo1
 ; MFS-DEFAULTS:               .section        .text.split.foo1
 ; MFS-DEFAULTS-NEXT:          foo1.cold:
 ; MFS-DEFAULTS-X86-NOT:       callq   bar
 ; MFS-DEFAULTS-X86-NEXT:      callq   baz
+; MFS-DEFAULTS-AARCH64-NOT:   bl      bar
+; MFS-DEFAULTS-AARCH64-NEXT:  bl      baz
   br i1 %0, label %2, label %4, !prof !17
 
 2:                                                ; preds = %1
   %3 = call i32 @bar()
   br label %6
 
 4:                                                ; preds = %1
   %5 = call i32 @baz()

 define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind !prof !20 {
 ;; Check that count threshold works.
 ; MFS-OPTS1-LABEL:         foo4
 ; MFS-OPTS1:               .section        .text.split.foo4
 ; MFS-OPTS1-NEXT:          foo4.cold:
 ; MFS-OPTS1-X86-NOT:       callq    bar
 ; MFS-OPTS1-X86-NOT:       callq    baz
 ; MFS-OPTS1-X86-NEXT:      callq    bam
+; MFS-OPTS1-AARCH64-NOT:   bl       bar
+; MFS-OPTS1-AARCH64-NOT:   bl       baz
+; MFS-OPTS1-AARCH64-NEXT:  bl       bam
   br i1 %0, label %3, label %7, !prof !18
 
 3:
   %4 = call i32 @bar()
   br label %7
 
 5:
   %6 = call i32 @baz()

 define void @foo5(i1 zeroext %0, i1 zeroext %1) nounwind !prof !20 {
 ;; Check that profile summary info cutoff works.
 ; MFS-OPTS2-LABEL:         foo5
 ; MFS-OPTS2:               .section        .text.split.foo5
 ; MFS-OPTS2-NEXT:               foo5.cold:
 ; MFS-OPTS2-X86-NOT:       callq    bar
 ; MFS-OPTS2-X86-NOT:       callq    baz
 ; MFS-OPTS2-X86-NEXT:      callq    bam
+; MFS-OPTS2-AARCH64-NOT:   bl       bar
+; MFS-OPTS2-AARCH64-NOT:   bl       baz
+; MFS-OPTS2-AARCH64-NEXT:  bl       bam
   br i1 %0, label %3, label %7, !prof !21
 
 3:
   %4 = call i32 @bar()
   br label %7
 
 5:
   %6 = call i32 @baz()

 
 define i32 @foo7(i1 zeroext %0) personality ptr @__gxx_personality_v0 !prof !14 {
 ;; Check that a single cold ehpad is split out.
 ; MFS-DEFAULTS-LABEL:         foo7
 ; MFS-DEFAULTS:               .section        .text.split.foo7,"ax",@progbits
 ; MFS-DEFAULTS-NEXT:          foo7.cold:
 ; MFS-DEFAULTS-X86:           callq   baz
 ; MFS-DEFAULTS-X86:           callq   _Unwind_Resume@PLT
+; MFS-DEFAULTS-AARCH64:       bl      baz
 entry:
   invoke void @_Z1fv()
           to label %try.cont unwind label %lpad
 
 lpad:
   %1 = landingpad { ptr, i32 }
           cleanup
           catch ptr @_ZTIi

 define i32 @foo8(i1 zeroext %0) personality ptr @__gxx_personality_v0 !prof !14 {
 ;; Check that all ehpads are treated as hot if one of them is hot.
 ; MFS-DEFAULTS-LABEL:         foo8
 ; MFS-DEFAULTS-X86:           callq   _Unwind_Resume@PLT
 ; MFS-DEFAULTS-X86:           callq   _Unwind_Resume@PLT
 ; MFS-DEFAULTS:               .section        .text.split.foo8,"ax",@progbits
 ; MFS-DEFAULTS-NEXT:          foo8.cold:
 ; MFS-DEFAULTS-X86:           callq   baz
+; MFS-DEFAULTS-AARCH64:       bl      baz
 
 ;; Check that all ehpads are by default treated as cold with -mfs-split-ehcode.
 ; MFS-EH-SPLIT-LABEL:         foo8
 ; MFS-EH-SPLIT-X86:           callq   baz
-; MFS-EH-SPLIT:               .section        .text.split.foo8,"ax",@progbits
-; MFS-EH-SPLIT-NEXT:          foo8.cold:
+; MFS-EH-SPLIT-AARCH64:       bl      baz
+; MFS-EH-SPLIT-X86:           .section        .text.split.foo8,"ax",@progbits
+; MFS-EH-SPLIT-X86-NEXT:      foo8.cold:
 ; MFS-EH-SPLIT-X86:           callq   _Unwind_Resume@PLT
 ; MFS-EH-SPLIT-X86:           callq   _Unwind_Resume@PLT
 entry:
   invoke void @_Z1fv()
           to label %try.cont unwind label %lpad1
 
 lpad1:
   %1 = landingpad { ptr, i32 }
6:                                                ; preds = %4, %2
   %7 = tail call i32 @qux()
   ret void
 }
 
 define i32 @foo10(i1 zeroext %0) personality ptr @__gxx_personality_v0 !prof !14 {
 ;; Check that nop is inserted just before the EH pad if it's beginning a section.
 ; MFS-DEFAULTS-LABEL:         foo10
 ; MFS-DEFAULTS-X86-LABEL:     callq   baz
-; MFS-DEFAULTS:               .section        .text.split.foo10,"ax",@progbits
-; MFS-DEFAULTS-NEXT:          foo10.cold:
+; MFS-DEFAULTS-AARCH64:       bl      baz
+; MFS-DEFAULTS-X86:           .section        .text.split.foo10,"ax",@progbits
+; MFS-DEFAULTS-X86-NEXT:      foo10.cold:
 ; MFS-DEFAULTS-X86:           nop
 ; MFS-DEFAULTS-X86:           callq   _Unwind_Resume@PLT
 entry:
   invoke void @_Z1fv()
           to label %try.cont unwind label %lpad, !prof !17
 
 lpad:
   %1 = landingpad { ptr, i32 }
           cleanup
           catch ptr @_ZTIi
   resume { ptr, i32 } %1
 
 try.cont:
   %2 = call i32 @baz()
   ret i32 %2
 }
 
 define void @foo11(i1 zeroext %0) personality ptr @__gxx_personality_v0 {
 ;; Check that function having landing pads are split with mfs-split-ehcode
 ;; even in the absence of profile data
 ; MFS-EH-SPLIT-LABEL:         foo11
-; MFS-EH-SPLIT:               .section        .text.split.foo11,"ax",@progbits
-; MFS-EH-SPLIT-NEXT:          foo11.cold:
+; MFS-EH-SPLIT-X86:           .section        .text.split.foo11,"ax",@progbits
+; MFS-EH-SPLIT-X86-NEXT:      foo11.cold:
 ; MFS-EH-SPLIT-X86:           nop
 ; MFS-EH-SPLIT-X86:           callq   _Unwind_Resume@PLT
 entry:
   invoke void @_Z1fv()
         to label %2 unwind label %lpad
 
 lpad:
   %1 = landingpad { ptr, i32 }

 define i32 @foo12(i1 zeroext %0) personality ptr @__gxx_personality_v0 !prof !14 {
 ;; Check that all code reachable from ehpad is split out with cycles.
 ; MFS-EH-SPLIT-LABEL:         foo12
 ; MFS-EH-SPLIT:               .section        .text.split.foo12,"ax",@progbits
 ; MFS-EH-SPLIT-NEXT:          foo12.cold:
 ; MFS-EH-SPLIT-X86:           callq   bar
 ; MFS-EH-SPLIT-X86:           callq   baz
 ; MFS-EH-SPLIT-X86:           callq   qux
+; MFS-EH-SPLIT-AARCH64:       bl      bar
+; MFS-EH-SPLIT-AARCH64:       bl      baz
+; MFS-EH-SPLIT-AARCH64:       bl      qux
 entry:
   invoke void @_Z1fv()
           to label %8 unwind label %lpad
 
 lpad:
   %1 = landingpad { ptr, i32 }
           cleanup
           catch ptr @_ZTIi

 }
 
 define i32 @foo13(i1 zeroext %0) personality ptr @__gxx_personality_v0 !prof !14{
 ;; Check that all code reachable from EH
 ;; that is also reachable from outside EH pad
 ;; is not touched.
 ; MFS-EH-SPLIT-LABEL:         foo13
 ; MFS-EH-SPLIT-X86:           callq   bam
+; MFS-EH-SPLIT-AARCH64:       bl      bam
 ; MFS-EH-SPLIT:               .section        .text.split.foo13,"ax",@progbits
 ; MFS-EH-SPLIT-NEXT:          foo13.cold:
 ; MFS-EH-SPLIT-X86:           callq   baz
 ; MFS-EH-SPLIT-X86:           callq   bar
 ; MFS-EH-SPLIT-X86:           callq   qux
+; MFS-EH-SPLIT-AARCH64:       bl      baz
+; MFS-EH-SPLIT-AARCH64:       bl      bar
+; MFS-EH-SPLIT-AARCH64:       bl      qux
 entry:
   invoke void @_Z1fv()
           to label %try.cont unwind label %lpad, !prof !17
 
 lpad:
   %1 = landingpad { ptr, i32 }
           cleanup
           catch ptr @_ZTIi