[llvm] 9ff24f5 - Revert "[CFIFixup] Factor logic into helpers and use range-based loops (NFC) (#125137)"
Daniel Hoekwater via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 31 12:23:25 PST 2025
Author: Daniel Hoekwater
Date: 2025-01-31T20:22:00Z
New Revision: 9ff24f5114e5cd3f6f44d2269005e6b18e0906b3
URL: https://github.com/llvm/llvm-project/commit/9ff24f5114e5cd3f6f44d2269005e6b18e0906b3
DIFF: https://github.com/llvm/llvm-project/commit/9ff24f5114e5cd3f6f44d2269005e6b18e0906b3.diff
LOG: Revert "[CFIFixup] Factor logic into helpers and use range-based loops (NFC) (#125137)"
This reverts commit f14f19738916572322c310e84196134545c15c49, which
breaks a number of build bots:
- https://lab.llvm.org/buildbot/#/builders/163/builds/12726
- https://lab.llvm.org/buildbot/#/builders/144/builds/17106
- https://lab.llvm.org/buildbot/#/builders/123/builds/12855
- https://lab.llvm.org/buildbot/#/builders/133/builds/10660
- https://lab.llvm.org/buildbot/#/builders/88/builds/7482
- https://lab.llvm.org/buildbot/#/builders/180/builds/12313
- https://lab.llvm.org/buildbot/#/builders/160/builds/12316
Added:
Modified:
llvm/lib/CodeGen/CFIFixup.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/CFIFixup.cpp b/llvm/lib/CodeGen/CFIFixup.cpp
index 86a52cea320d29..02152a136a2254 100644
--- a/llvm/lib/CodeGen/CFIFixup.cpp
+++ b/llvm/lib/CodeGen/CFIFixup.cpp
@@ -70,7 +70,6 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/CodeGen/MachineBasicBlock.h"
#include "llvm/CodeGen/MachineFunction.h"
@@ -123,57 +122,6 @@ findPrologueEnd(MachineFunction &MF, MachineBasicBlock::iterator &PrologueEnd) {
return nullptr;
}
-// Represents a basic block's relationship to the call frame. This metadata
-// reflects what the state *should* be, which may
diff er from the actual state
-// after final machine basic block layout.
-struct BlockFlags {
- bool Reachable : 1;
- bool StrongNoFrameOnEntry : 1;
- bool HasFrameOnEntry : 1;
- bool HasFrameOnExit : 1;
-};
-
-// Computes the frame information for each block in the function. Frame info
-// for a block is inferred from its predecessors.
-static SmallVector<BlockFlags>
-computeBlockInfo(const MachineFunction &MF,
- const MachineBasicBlock *PrologueBlock) {
- SmallVector<BlockFlags, 32> BlockInfo(MF.getNumBlockIDs(),
- {false, false, false, false});
- BlockInfo[0].Reachable = true;
- BlockInfo[0].StrongNoFrameOnEntry = true;
-
- // Compute the presence/absence of frame at each basic block.
- ReversePostOrderTraversal<const MachineBasicBlock *> RPOT(&*MF.begin());
- for (const MachineBasicBlock *MBB : RPOT) {
- BlockFlags &Info = BlockInfo[MBB->getNumber()];
-
- // Set to true if the current block contains the prologue or the epilogue,
- // respectively.
- bool HasPrologue = MBB == PrologueBlock;
- bool HasEpilogue = false;
-
- if (Info.HasFrameOnEntry || HasPrologue)
- HasEpilogue = containsEpilogue(*MBB);
-
- // If the function has a call frame at the entry of the current block or the
- // current block contains the prologue, then the function has a call frame
- // at the exit of the block, unless the block contains the epilogue.
- Info.HasFrameOnExit = (Info.HasFrameOnEntry || HasPrologue) && !HasEpilogue;
-
- // Set the successors' state on entry.
- for (MachineBasicBlock *Succ : MBB->successors()) {
- BlockFlags &SuccInfo = BlockInfo[Succ->getNumber()];
- SuccInfo.Reachable = true;
- SuccInfo.StrongNoFrameOnEntry |=
- Info.StrongNoFrameOnEntry && !HasPrologue;
- SuccInfo.HasFrameOnEntry = Info.HasFrameOnExit;
- }
- }
-
- return BlockInfo;
-}
-
// Represents the point within a basic block where we can insert an instruction.
// Note that we need the MachineBasicBlock* as well as the iterator since the
// iterator can point to the end of the block. Instructions are inserted
@@ -233,69 +181,13 @@ static InsertionPoint cloneCfiPrologue(const InsertionPoint &PrologueEnd,
return DstInsertPt;
}
-// Fixes up the CFI instructions in a basic block to be consistent with the
-// intended frame state, adding or removing CFI instructions as necessary.
-// Returns true if a change was made and false otherwise.
-static bool
-fixupBlock(MachineBasicBlock &CurrBB, const SmallVector<BlockFlags> &BlockInfo,
- SmallDenseMap<MBBSectionID, InsertionPoint> &InsertionPts,
- const InsertionPoint &Prologue) {
- const MachineFunction &MF = *CurrBB.getParent();
- const TargetFrameLowering &TFL = *MF.getSubtarget().getFrameLowering();
- const BlockFlags &Info = BlockInfo[CurrBB.getNumber()];
-
- if (!Info.Reachable)
- return false;
-
- // If the previous block and the current block are in the same section,
- // the frame info will propagate from the previous block to the current one.
- const BlockFlags &PrevInfo =
- BlockInfo[std::prev(CurrBB.getIterator())->getNumber()];
- bool HasFrame = PrevInfo.HasFrameOnExit && !CurrBB.isBeginSection();
- bool NeedsFrame = Info.HasFrameOnEntry && !Info.StrongNoFrameOnEntry;
-
-#ifndef NDEBUG
- if (!Info.StrongNoFrameOnEntry) {
- for (auto *Pred : CurrBB.predecessors()) {
- const BlockFlags &PredInfo = BlockInfo[Pred->getNumber()];
- assert((!PredInfo.Reachable ||
- Info.HasFrameOnEntry == PredInfo.HasFrameOnExit) &&
- "Inconsistent call frame state");
- }
- }
-#endif
-
- if (HasFrame == NeedsFrame)
- return false;
-
- if (!NeedsFrame) {
- // Reset to the state upon function entry.
- TFL.resetCFIToInitialState(CurrBB);
- return true;
- }
-
- // Reset to the "after prologue" state.
- InsertionPoint &InsertPt = InsertionPts[CurrBB.getSectionID()];
- if (InsertPt.MBB == nullptr) {
- // CurBB is the first block in its section, so there is no "after
- // prologue" state. Clone the CFI instructions from the prologue block
- // to create it.
- InsertPt = cloneCfiPrologue(Prologue, {&CurrBB, CurrBB.begin()});
- } else {
- // There's an earlier block known to have a stack frame. Insert a
- // `.cfi_remember_state` instruction into that block and a
- // `.cfi_restore_state` instruction at the beginning of the current
- // block.
- InsertPt = insertRememberRestorePair(InsertPt, {&CurrBB, CurrBB.begin()});
- }
- return true;
-}
-
bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
- if (!MF.getSubtarget().getFrameLowering()->enableCFIFixup(MF))
+ const TargetFrameLowering &TFL = *MF.getSubtarget().getFrameLowering();
+ if (!TFL.enableCFIFixup(MF))
return false;
- if (MF.getNumBlockIDs() < 2)
+ const unsigned NumBlocks = MF.getNumBlockIDs();
+ if (NumBlocks < 2)
return false;
// Find the prologue and the point where we can issue the first
@@ -305,7 +197,44 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
if (PrologueBlock == nullptr)
return false;
- SmallVector<BlockFlags> BlockInfo = computeBlockInfo(MF, PrologueBlock);
+ struct BlockFlags {
+ bool Reachable : 1;
+ bool StrongNoFrameOnEntry : 1;
+ bool HasFrameOnEntry : 1;
+ bool HasFrameOnExit : 1;
+ };
+ SmallVector<BlockFlags, 32> BlockInfo(NumBlocks,
+ {false, false, false, false});
+ BlockInfo[0].Reachable = true;
+ BlockInfo[0].StrongNoFrameOnEntry = true;
+
+ // Compute the presence/absence of frame at each basic block.
+ 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 = MBB == PrologueBlock;
+ bool HasEpilogue = false;
+
+ if (Info.HasFrameOnEntry || HasPrologue)
+ HasEpilogue = containsEpilogue(*MBB);
+
+ // If the function has a call frame at the entry of the current block or the
+ // current block contains the prologue, then the function has a call frame
+ // at the exit of the block, unless the block contains the epilogue.
+ Info.HasFrameOnExit = (Info.HasFrameOnEntry || HasPrologue) && !HasEpilogue;
+
+ // Set the successors' state on entry.
+ for (MachineBasicBlock *Succ : MBB->successors()) {
+ BlockFlags &SuccInfo = BlockInfo[Succ->getNumber()];
+ SuccInfo.Reachable = true;
+ SuccInfo.StrongNoFrameOnEntry |=
+ Info.StrongNoFrameOnEntry && !HasPrologue;
+ SuccInfo.HasFrameOnEntry = Info.HasFrameOnExit;
+ }
+ }
// Walk the blocks of the function in "physical" order.
// Every block inherits the frame state (as recorded in the unwind tables)
@@ -324,10 +253,57 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
// No point starting before the prologue block.
// TODO: the unwind tables will still be incorrect if an epilogue physically
// preceeds the prologue.
- for (MachineBasicBlock &MBB :
- make_range(std::next(PrologueBlock->getIterator()), MF.end())) {
- Change |=
- fixupBlock(MBB, BlockInfo, InsertionPts, {PrologueBlock, PrologueEnd});
+ MachineFunction::iterator CurrBB = std::next(PrologueBlock->getIterator());
+ bool HasFrame = BlockInfo[PrologueBlock->getNumber()].HasFrameOnExit;
+ while (CurrBB != MF.end()) {
+ const BlockFlags &Info = BlockInfo[CurrBB->getNumber()];
+ if (!Info.Reachable) {
+ ++CurrBB;
+ continue;
+ }
+
+#ifndef NDEBUG
+ if (!Info.StrongNoFrameOnEntry) {
+ for (auto *Pred : CurrBB->predecessors()) {
+ BlockFlags &PredInfo = BlockInfo[Pred->getNumber()];
+ assert((!PredInfo.Reachable ||
+ Info.HasFrameOnEntry == PredInfo.HasFrameOnExit) &&
+ "Inconsistent call frame state");
+ }
+ }
+#endif
+
+ // If the block is the first block in its section, then it doesn't have a
+ // frame on entry.
+ HasFrame &= !CurrBB->isBeginSection();
+ if (!Info.StrongNoFrameOnEntry && Info.HasFrameOnEntry && !HasFrame) {
+ // Reset to the "after prologue" state.
+
+ InsertionPoint &InsertPt = InsertionPts[CurrBB->getSectionID()];
+ if (InsertPt.MBB == nullptr) {
+ // CurBB is the first block in its section, so there is no "after
+ // prologue" state. Clone the CFI instructions from the prologue block
+ // to create it.
+ InsertPt = cloneCfiPrologue({PrologueBlock, PrologueEnd},
+ {&*CurrBB, CurrBB->begin()});
+ } else {
+ // There's an earlier block known to have a stack frame. Insert a
+ // `.cfi_remember_state` instruction into that block and a
+ // `.cfi_restore_state` instruction at the beginning of the current
+ // block.
+ InsertPt =
+ insertRememberRestorePair(InsertPt, {&*CurrBB, CurrBB->begin()});
+ }
+ Change = true;
+ } else if ((Info.StrongNoFrameOnEntry || !Info.HasFrameOnEntry) &&
+ HasFrame) {
+ // Reset to the state upon function entry.
+ TFL.resetCFIToInitialState(*CurrBB);
+ Change = true;
+ }
+
+ HasFrame = Info.HasFrameOnExit;
+ ++CurrBB;
}
return Change;
More information about the llvm-commits
mailing list