[llvm] [CFIFixup] Allow function prologues to span more than one basic block (PR #68984)
Momchil Velikov via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 14 05:41:06 PST 2023
https://github.com/momchil-velikov updated https://github.com/llvm/llvm-project/pull/68984
>From 344ece61db66e79d49b7cca557848191d7046d70 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Mon, 2 Oct 2023 14:46:27 +0100
Subject: [PATCH 1/3] [CFIFixup] Allow function prologues to span more than one
basic block
The CFIFixup pass assumes a function prologue is contained in a single
basic block. This assumption is broken with upcoming support for stack
probing (`-fstack-clash-protection`) in AArch64 - the emitted probing
sequence in a prologue may contain loops, i.e. more than one basic
block. The generated CFG is not arbitrary though:
* CFI instructions are outside of any loops
* for any two CFI instructions of the function prologue one dominates
and is post-dominated by the other
Thus, for the prologue CFI instructions, if one is
executed then all are executed, there is a total order of
executions, and the last instruction in that order can be considered
the end of the prologoue for the purpose of inserting the initial
`.cfi_remember_state` directive.
That last instruction is found by finding the first block in the
post-order traversal which contains prologue CFI instructions.
---
llvm/lib/CodeGen/CFIFixup.cpp | 62 ++++++++++++-------
.../cfi-fixup-multi-block-prologue.mir | 7 ++-
2 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/CodeGen/CFIFixup.cpp b/llvm/lib/CodeGen/CFIFixup.cpp
index 837dbd77d07361a..964a8d56511fa1b 100644
--- a/llvm/lib/CodeGen/CFIFixup.cpp
+++ b/llvm/lib/CodeGen/CFIFixup.cpp
@@ -10,20 +10,25 @@
// This pass inserts the necessary instructions to adjust for the inconsistency
// of the call-frame information caused by final machine basic block layout.
// The pass relies in constraints LLVM imposes on the placement of
-// save/restore points (cf. ShrinkWrap):
-// * there is a single basic block, containing the function prologue
+// save/restore points (cf. ShrinkWrap) and has certain preconditions about
+// placement of CFI instructions:
+// * for any two CFI instructions of the function prologue one dominates
+// and is post-dominated by the other
// * possibly multiple epilogue blocks, where each epilogue block is
// complete and self-contained, i.e. CSR restore instructions (and the
// corresponding CFI instructions are not split across two or more blocks.
-// * prologue and epilogue blocks are outside of any loops
-// Thus, during execution, at the beginning and at the end of each basic block
-// the function can be in one of two states:
+// * CFI instructions are not contained in any loops
+// Thus, during execution, at the beginning and at the end of each basic block,
+// following the prologue, the function can be in one of two states:
// - "has a call frame", if the function has executed the prologue, and
// has not executed any epilogue
// - "does not have a call frame", if the function has not executed the
// prologue, or has executed an epilogue
// which can be computed by a single RPO traversal.
+// The location of the prologue is determined by finding the first block in the
+// post-order traversal which contains CFI instructions.
+
// In order to accommodate backends which do not generate unwind info in
// epilogues we compute an additional property "strong no call frame on entry",
// which is set for the entry point of the function and for every block
@@ -85,10 +90,6 @@ static bool isPrologueCFIInstruction(const MachineInstr &MI) {
MI.getFlag(MachineInstr::FrameSetup);
}
-static bool containsPrologue(const MachineBasicBlock &MBB) {
- return llvm::any_of(MBB.instrs(), isPrologueCFIInstruction);
-}
-
static bool containsEpilogue(const MachineBasicBlock &MBB) {
return llvm::any_of(llvm::reverse(MBB), [](const auto &MI) {
return MI.getOpcode() == TargetOpcode::CFI_INSTRUCTION &&
@@ -96,6 +97,25 @@ static bool containsEpilogue(const MachineBasicBlock &MBB) {
});
}
+static MachineBasicBlock *
+findPrologueEnd(MachineFunction &MF, MachineBasicBlock::iterator &PrologueEnd) {
+ MachineBasicBlock *PrologueBlock = nullptr;
+ for (auto It = po_begin(&MF.front()), End = po_end(&MF.front()); It != End;
+ ++It) {
+ MachineBasicBlock *MBB = *It;
+ llvm::for_each(MBB->instrs(), [&](MachineInstr &MI) {
+ if (isPrologueCFIInstruction(MI)) {
+ PrologueBlock = MBB;
+ PrologueEnd = std::next(MI.getIterator());
+ }
+ });
+ if (PrologueBlock)
+ return PrologueBlock;
+ }
+
+ return nullptr;
+}
+
bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
const TargetFrameLowering &TFL = *MF.getSubtarget().getFrameLowering();
if (!TFL.enableCFIFixup(MF))
@@ -105,6 +125,14 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
if (NumBlocks < 2)
return false;
+ // Find the prologue and the point where we can issue the first
+ // `.cfi_remember_state`.
+
+ MachineBasicBlock::iterator PrologueEnd;
+ MachineBasicBlock *PrologueBlock = findPrologueEnd(MF, PrologueEnd);
+ if (PrologueBlock == nullptr)
+ return false;
+
struct BlockFlags {
bool Reachable : 1;
bool StrongNoFrameOnEntry : 1;
@@ -116,21 +144,15 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
BlockInfo[0].StrongNoFrameOnEntry = true;
// Compute the presence/absence of frame at each basic block.
- MachineBasicBlock *PrologueBlock = nullptr;
ReversePostOrderTraversal<MachineBasicBlock *> RPOT(&*MF.begin());
for (MachineBasicBlock *MBB : RPOT) {
BlockFlags &Info = BlockInfo[MBB->getNumber()];
// Set to true if the current block contains the prologue or the epilogue,
// respectively.
- bool HasPrologue = false;
+ bool HasPrologue = MBB == PrologueBlock;
bool HasEpilogue = false;
- if (!PrologueBlock && !Info.HasFrameOnEntry && containsPrologue(*MBB)) {
- PrologueBlock = MBB;
- HasPrologue = true;
- }
-
if (Info.HasFrameOnEntry || HasPrologue)
HasEpilogue = containsEpilogue(*MBB);
@@ -149,9 +171,6 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
}
}
- if (!PrologueBlock)
- return false;
-
// Walk the blocks of the function in "physical" order.
// Every block inherits the frame state (as recorded in the unwind tables)
// of the previous block. If the intended frame state is different, insert
@@ -162,10 +181,7 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
// insert a `.cfi_remember_state`, in the case that the current block needs a
// `.cfi_restore_state`.
MachineBasicBlock *InsertMBB = PrologueBlock;
- MachineBasicBlock::iterator InsertPt = PrologueBlock->begin();
- for (MachineInstr &MI : *PrologueBlock)
- if (isPrologueCFIInstruction(MI))
- InsertPt = std::next(MI.getIterator());
+ MachineBasicBlock::iterator InsertPt = PrologueEnd;
assert(InsertPt != PrologueBlock->begin() &&
"Inconsistent notion of \"prologue block\"");
diff --git a/llvm/test/CodeGen/AArch64/cfi-fixup-multi-block-prologue.mir b/llvm/test/CodeGen/AArch64/cfi-fixup-multi-block-prologue.mir
index ddd9a9eaef55efb..31fa3832367becc 100644
--- a/llvm/test/CodeGen/AArch64/cfi-fixup-multi-block-prologue.mir
+++ b/llvm/test/CodeGen/AArch64/cfi-fixup-multi-block-prologue.mir
@@ -1,3 +1,4 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
# RUN: llc -run-pass=cfi-fixup %s -o - | FileCheck %s
--- |
source_filename = "cfi-fixup.ll"
@@ -111,9 +112,8 @@ body: |
; CHECK-NEXT: frame-setup CFI_INSTRUCTION offset $w29, -16
; CHECK-NEXT: $x9 = frame-setup SUBXri $sp, 7, 12
; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa $w9, 28688
- ; CHECK-NEXT: CFI_INSTRUCTION remember_state
-
- ; CHECK: bb.1.entry:
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1.entry:
; CHECK-NEXT: successors: %bb.2(0x40000000), %bb.1(0x40000000)
; CHECK-NEXT: liveins: $x9
; CHECK-NEXT: {{ $}}
@@ -129,6 +129,7 @@ body: |
; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa_register $wsp
; CHECK-NEXT: $sp = frame-setup SUBXri $sp, 1328, 0
; CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa_offset 30016
+ ; CHECK-NEXT: CFI_INSTRUCTION remember_state
; CHECK-NEXT: frame-setup STRXui $xzr, $sp, 0
; CHECK-NEXT: CBZW renamable $w0, %bb.6
; CHECK-NEXT: {{ $}}
>From 027a2a41aca26932fe9cf943f98a3f630f0fe1a4 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Sat, 28 Oct 2023 13:33:28 +0100
Subject: [PATCH 2/3] Reverse iteration within a block when looking for
prologue CFI insns
---
llvm/lib/CodeGen/CFIFixup.cpp | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/CodeGen/CFIFixup.cpp b/llvm/lib/CodeGen/CFIFixup.cpp
index 964a8d56511fa1b..40a2a3a142e1758 100644
--- a/llvm/lib/CodeGen/CFIFixup.cpp
+++ b/llvm/lib/CodeGen/CFIFixup.cpp
@@ -99,20 +99,16 @@ static bool containsEpilogue(const MachineBasicBlock &MBB) {
static MachineBasicBlock *
findPrologueEnd(MachineFunction &MF, MachineBasicBlock::iterator &PrologueEnd) {
- MachineBasicBlock *PrologueBlock = nullptr;
for (auto It = po_begin(&MF.front()), End = po_end(&MF.front()); It != End;
++It) {
MachineBasicBlock *MBB = *It;
- llvm::for_each(MBB->instrs(), [&](MachineInstr &MI) {
- if (isPrologueCFIInstruction(MI)) {
- PrologueBlock = MBB;
- PrologueEnd = std::next(MI.getIterator());
- }
- });
- if (PrologueBlock)
- return PrologueBlock;
+ for (MachineInstr &MI : reverse(MBB->instrs())) {
+ if (!isPrologueCFIInstruction(MI))
+ continue;
+ PrologueEnd = std::next(MI.getIterator());
+ return MBB;
+ }
}
-
return nullptr;
}
>From b2a2afe4fdea583099bffd75f266fb9e1abbb0a4 Mon Sep 17 00:00:00 2001
From: Momchil Velikov <momchil.velikov at arm.com>
Date: Tue, 7 Nov 2023 14:08:26 +0000
Subject: [PATCH 3/3] Use simple reverse traversal of basic blocks
---
llvm/lib/CodeGen/CFIFixup.cpp | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/CodeGen/CFIFixup.cpp b/llvm/lib/CodeGen/CFIFixup.cpp
index 40a2a3a142e1758..61888a42666524b 100644
--- a/llvm/lib/CodeGen/CFIFixup.cpp
+++ b/llvm/lib/CodeGen/CFIFixup.cpp
@@ -12,12 +12,14 @@
// The pass relies in constraints LLVM imposes on the placement of
// save/restore points (cf. ShrinkWrap) and has certain preconditions about
// placement of CFI instructions:
-// * for any two CFI instructions of the function prologue one dominates
-// and is post-dominated by the other
-// * possibly multiple epilogue blocks, where each epilogue block is
-// complete and self-contained, i.e. CSR restore instructions (and the
-// corresponding CFI instructions are not split across two or more blocks.
-// * CFI instructions are not contained in any loops
+// * For any two CFI instructions of the function prologue one dominates
+// and is post-dominated by the other.
+// * The function possibly contains multiple epilogue blocks, where each
+// epilogue block is complete and self-contained, i.e. CSR restore
+// instructions (and the corresponding CFI instructions)
+// are not split across two or more blocks.
+// * CFI instructions are not contained in any loops.
+
// Thus, during execution, at the beginning and at the end of each basic block,
// following the prologue, the function can be in one of two states:
// - "has a call frame", if the function has executed the prologue, and
@@ -27,7 +29,7 @@
// which can be computed by a single RPO traversal.
// The location of the prologue is determined by finding the first block in the
-// post-order traversal which contains CFI instructions.
+// reverse traversal which contains CFI instructions.
// In order to accommodate backends which do not generate unwind info in
// epilogues we compute an additional property "strong no call frame on entry",
@@ -99,14 +101,16 @@ static bool containsEpilogue(const MachineBasicBlock &MBB) {
static MachineBasicBlock *
findPrologueEnd(MachineFunction &MF, MachineBasicBlock::iterator &PrologueEnd) {
- for (auto It = po_begin(&MF.front()), End = po_end(&MF.front()); It != End;
- ++It) {
- MachineBasicBlock *MBB = *It;
- for (MachineInstr &MI : reverse(MBB->instrs())) {
+ // Even though we should theoretically traverse the blocks in post-order, we
+ // can't encode correctly cases where prologue blocks are not laid out in
+ // topological order. Then, assuming topological order, we can just traverse
+ // the function in reverse.
+ for (MachineBasicBlock &MBB : reverse(MF)) {
+ for (MachineInstr &MI : reverse(MBB.instrs())) {
if (!isPrologueCFIInstruction(MI))
continue;
PrologueEnd = std::next(MI.getIterator());
- return MBB;
+ return &MBB;
}
}
return nullptr;
@@ -123,7 +127,6 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
// Find the prologue and the point where we can issue the first
// `.cfi_remember_state`.
-
MachineBasicBlock::iterator PrologueEnd;
MachineBasicBlock *PrologueBlock = findPrologueEnd(MF, PrologueEnd);
if (PrologueBlock == nullptr)
More information about the llvm-commits
mailing list