[llvm] 8c249c4 - [CodeGen][AArch64] Don't split functions with a red zone on AArch64
Daniel Hoekwater via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 24 15:04:19 PDT 2023
Author: Daniel Hoekwater
Date: 2023-08-24T21:57:35Z
New Revision: 8c249c44d41f69c867fcf47f65e4646b626368d7
URL: https://github.com/llvm/llvm-project/commit/8c249c44d41f69c867fcf47f65e4646b626368d7
DIFF: https://github.com/llvm/llvm-project/commit/8c249c44d41f69c867fcf47f65e4646b626368d7.diff
LOG: [CodeGen][AArch64] Don't split functions with a red zone on AArch64
Because unconditional branch relaxation on AArch64 grows the stack to
spill a register, splitting a function would cause the red zone to be
overwritten. Explicitly disable MFS for such functions.
Differential Revision: https://reviews.llvm.org/D157127
Added:
Modified:
llvm/include/llvm/CodeGen/TargetInstrInfo.h
llvm/lib/CodeGen/MachineFunctionSplitter.cpp
llvm/lib/CodeGen/TargetInstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.h
llvm/test/CodeGen/Generic/machine-function-splitter.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 5e9e72c531bb21..cefeb360d0dde6 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -2072,6 +2072,10 @@ class TargetInstrInfo : public MCInstrInfo {
return false;
}
+ /// Return true if the function is a viable candidate for machine function
+ /// splitting. The criteria for if a function can be split may vary by target.
+ virtual bool isFunctionSafeToSplit(const MachineFunction &MF) const;
+
/// Produce the expression describing the \p MI loading a value into
/// the physical register \p Reg. This hook should only be used with
/// \p MIs belonging to VReg-less functions.
diff --git a/llvm/lib/CodeGen/MachineFunctionSplitter.cpp b/llvm/lib/CodeGen/MachineFunctionSplitter.cpp
index 440f6d2cb461e8..8b2092dc4ec097 100644
--- a/llvm/lib/CodeGen/MachineFunctionSplitter.cpp
+++ b/llvm/lib/CodeGen/MachineFunctionSplitter.cpp
@@ -35,6 +35,7 @@
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/Passes.h"
+#include "llvm/CodeGen/TargetInstrInfo.h"
#include "llvm/IR/Function.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/CommandLine.h"
@@ -142,22 +143,10 @@ bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
if (!UseProfileData && !SplitAllEHCode)
return false;
- // TODO: We don't split functions where a section attribute has been set
- // since the split part may not be placed in a contiguous region. It may also
- // be more beneficial to augment the linker to ensure contiguous layout of
- // split functions within the same section as specified by the attribute.
- if (MF.getFunction().hasSection() ||
- MF.getFunction().hasFnAttribute("implicit-section-name"))
+ const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
+ if (!TII.isFunctionSafeToSplit(MF))
return false;
- // We don't want to proceed further for cold functions
- // or functions of unknown hotness. Lukewarm functions have no prefix.
- std::optional<StringRef> SectionPrefix = MF.getFunction().getSectionPrefix();
- if (SectionPrefix &&
- (*SectionPrefix == "unlikely" || *SectionPrefix == "unknown")) {
- return false;
- }
-
// Renumbering blocks here preserves the order of the blocks as
// sortBasicBlocksAndUpdateBranches uses the numeric identifier to sort
// blocks. Preserving the order of blocks is essential to retaining decisions
diff --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp
index 6779aa47bb67de..d640a52c940653 100644
--- a/llvm/lib/CodeGen/TargetInstrInfo.cpp
+++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp
@@ -1373,6 +1373,26 @@ bool TargetInstrInfo::hasLowDefLatency(const TargetSchedModel &SchedModel,
return (DefCycle != -1 && DefCycle <= 1);
}
+bool TargetInstrInfo::isFunctionSafeToSplit(const MachineFunction &MF) const {
+ // TODO: We don't split functions where a section attribute has been set
+ // since the split part may not be placed in a contiguous region. It may also
+ // be more beneficial to augment the linker to ensure contiguous layout of
+ // split functions within the same section as specified by the attribute.
+ if (MF.getFunction().hasSection() ||
+ MF.getFunction().hasFnAttribute("implicit-section-name"))
+ return false;
+
+ // We don't want to proceed further for cold functions
+ // or functions of unknown hotness. Lukewarm functions have no prefix.
+ std::optional<StringRef> SectionPrefix = MF.getFunction().getSectionPrefix();
+ if (SectionPrefix &&
+ (*SectionPrefix == "unlikely" || *SectionPrefix == "unknown")) {
+ return false;
+ }
+
+ return true;
+}
+
std::optional<ParamLoadedValue>
TargetInstrInfo::describeLoadedValue(const MachineInstr &MI,
Register Reg) const {
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 995b3baac06fac..8f72a538e8886a 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -8395,6 +8395,17 @@ describeORRLoadedValue(const MachineInstr &MI, Register DescribedReg,
return std::nullopt;
}
+bool AArch64InstrInfo::isFunctionSafeToSplit(const MachineFunction &MF) const {
+ // Functions cannot be split to
diff erent sections on AArch64 if they have
+ // a red zone. This is because relaxing a cross-section branch may require
+ // incrementing the stack pointer to spill a register, which would overwrite
+ // the red zone.
+ if (MF.getInfo<AArch64FunctionInfo>()->hasRedZone().value_or(true))
+ return false;
+
+ return TargetInstrInfo::isFunctionSafeToSplit(MF);
+}
+
std::optional<ParamLoadedValue>
AArch64InstrInfo::describeLoadedValue(const MachineInstr &MI,
Register Reg) const {
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index ca7e1e0834f17c..a3a4efba58f7e5 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -336,6 +336,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
std::optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
Register Reg) const override;
+ bool isFunctionSafeToSplit(const MachineFunction &MF) const override;
+
std::optional<ParamLoadedValue>
describeLoadedValue(const MachineInstr &MI, Register Reg) const override;
diff --git a/llvm/test/CodeGen/Generic/machine-function-splitter.ll b/llvm/test/CodeGen/Generic/machine-function-splitter.ll
index 20171453e4fa0d..87aa958c5bb369 100644
--- a/llvm/test/CodeGen/Generic/machine-function-splitter.ll
+++ b/llvm/test/CodeGen/Generic/machine-function-splitter.ll
@@ -12,6 +12,7 @@
; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -enable-split-machine-functions -mfs-allow-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-allow-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-allow-unsupported-triple -mfs-split-ehcode | FileCheck %s -check-prefixes=MFS-EH-SPLIT,MFS-EH-SPLIT-AARCH64
+; RUN: llc < %s -mtriple=aarch64 -enable-split-machine-functions -mfs-allow-unsupported-triple -aarch64-redzone | FileCheck %s -check-prefixes=MFS-REDZONE-AARCH64
; COM: Machine function splitting with AFDO profiles
; RUN: sed 's/InstrProf/SampleProfile/g' %s > %t.ll
@@ -486,6 +487,36 @@ define void @foo16(i1 zeroext %0) nounwind !prof !14 !section_prefix !15 {
ret void
}
+define i32 @foo17(i1 zeroext %0, i32 %a, i32 %b) nounwind !prof !14 !section_prefix !15 {
+;; Check that cold blocks in functions with red zones aren't split.
+; MFS-DEFAULTS-LABEL: foo17
+; MFS-DEFAULTS-X86: foo17.cold:
+; MFS-REDZONE-AARCH64-NOT: foo17.cold:
+ %a.addr = alloca i32, align 4
+ %b.addr = alloca i32, align 4
+ %x = alloca i32, align 4
+
+ br i1 %0, label %2, label %3, !prof !17
+
+2: ; preds = %1
+ store i32 %a, ptr %a.addr, align 4
+ store i32 %b, ptr %b.addr, align 4
+ br label %4
+
+3: ; preds = %1
+ store i32 %a, ptr %b.addr, align 4
+ store i32 %b, ptr %a.addr, align 4
+ br label %4
+
+4: ; preds = %3, %2
+ %tmp = load i32, ptr %a.addr, align 4
+ %tmp1 = load i32, ptr %b.addr, align 4
+ %add = add nsw i32 %tmp, %tmp1
+ store i32 %add, ptr %x, align 4
+ %tmp2 = load i32, ptr %x, align 4
+ ret i32 %tmp2
+}
+
declare i32 @bar()
declare i32 @baz()
declare i32 @bam()
More information about the llvm-commits
mailing list