[llvm] [CodeGen] Distinguish zero-sized call frames from no call frame in MachineBB (PR #106964)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 2 03:19:05 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-m68k
@llvm/pr-subscribers-backend-arm
Author: Fabian Ritter (ritter-x2a)
<details>
<summary>Changes</summary>
Call frames can be zero-sized. Distinguishing between instructions in a
zero-sized call frame and instructions outside of call frames via the
call frame size info stored in the MachineBBs is helpful to avoid nested
CALLSEQs (which are illegal and cause machine verifier errors).
This patch uses std::optional<unsigned> instead of unsigned to represent
call frame sizes. Locations outside of call frames are marked with
std::nullopt whereas locations in zero-sized call-frames are represented
with a std::optional that contains 0.
---
Full diff: https://github.com/llvm/llvm-project/pull/106964.diff
14 Files Affected:
- (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+14-5)
- (modified) llvm/include/llvm/CodeGen/TargetInstrInfo.h (+3-2)
- (modified) llvm/lib/CodeGen/MIRParser/MIParser.cpp (+3-3)
- (modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+2-2)
- (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+2-2)
- (modified) llvm/lib/CodeGen/PrologEpilogInserter.cpp (+10-10)
- (modified) llvm/lib/CodeGen/TargetInstrInfo.cpp (+3-3)
- (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+2-2)
- (modified) llvm/lib/Target/AVR/AVRISelLowering.cpp (+1-1)
- (modified) llvm/lib/Target/M68k/M68kISelLowering.cpp (+1-1)
- (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+1-1)
- (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+2-1)
- (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+1-1)
- (modified) llvm/test/CodeGen/MIR/ARM/call-frame-size.mir (+22)
``````````diff
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 6efb17c55493a9..cbc2c66d8bb5d8 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -146,12 +146,13 @@ class MachineBasicBlock
int Number;
/// The call frame size on entry to this basic block due to call frame setup
- /// instructions in a predecessor. This is usually zero, unless basic blocks
- /// are split in the middle of a call sequence.
+ /// instructions in a predecessor. Usually does not contain a value, unless
+ /// basic blocks are split in the middle of a call sequence. Zero-sized call
+ /// frames are possible.
///
/// This information is only maintained until PrologEpilogInserter eliminates
/// call frame pseudos.
- unsigned CallFrameSize = 0;
+ std::optional<unsigned> CallFrameSize;
MachineFunction *xParent;
Instructions Insts;
@@ -1210,9 +1211,17 @@ class MachineBasicBlock
void setNumber(int N) { Number = N; }
/// Return the call frame size on entry to this basic block.
- unsigned getCallFrameSize() const { return CallFrameSize; }
+ std::optional<unsigned> getCallFrameSize() const { return CallFrameSize; }
+
+ /// Return the call frame size on entry to this basic block if it is part of a
+ /// call sequence and 0 otherwise.
+ unsigned getCallFrameSizeOrZero() const { return CallFrameSize.value_or(0); }
+
/// Set the call frame size on entry to this basic block.
- void setCallFrameSize(unsigned N) { CallFrameSize = N; }
+ void setCallFrameSize(std::optional<unsigned> N) { CallFrameSize = N; }
+
+ /// Reset the stored call frame size.
+ void clearCallFrameSize() { CallFrameSize.reset(); }
/// Return the MCSymbol for this basic block.
MCSymbol *getSymbol() const;
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 49ce13dd8cbe39..0ae3a0ca53757a 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -2265,8 +2265,9 @@ class TargetInstrInfo : public MCInstrInfo {
return false;
}
- // Get the call frame size just before MI.
- unsigned getCallFrameSizeAt(MachineInstr &MI) const;
+ /// Get the call frame size just before MI. Contains no value if MI is not in
+ /// a call sequence. Zero-sized call frames are possible.
+ std::optional<unsigned> getCallFrameSizeAt(MachineInstr &MI) const;
/// Fills in the necessary MachineOperands to refer to a frame index.
/// The best way to understand this is to print `asm(""::"m"(x));` after
diff --git a/llvm/lib/CodeGen/MIRParser/MIParser.cpp b/llvm/lib/CodeGen/MIRParser/MIParser.cpp
index 47b220172602d4..4295956b2a5cc9 100644
--- a/llvm/lib/CodeGen/MIRParser/MIParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIParser.cpp
@@ -502,7 +502,7 @@ class MIParser {
bool parseAddrspace(unsigned &Addrspace);
bool parseSectionID(std::optional<MBBSectionID> &SID);
bool parseBBID(std::optional<UniqueBBID> &BBID);
- bool parseCallFrameSize(unsigned &CallFrameSize);
+ bool parseCallFrameSize(std::optional<unsigned> &CallFrameSize);
bool parseOperandsOffset(MachineOperand &Op);
bool parseIRValue(const Value *&V);
bool parseMemoryOperandFlag(MachineMemOperand::Flags &Flags);
@@ -685,7 +685,7 @@ bool MIParser::parseBBID(std::optional<UniqueBBID> &BBID) {
}
// Parse basic block call frame size.
-bool MIParser::parseCallFrameSize(unsigned &CallFrameSize) {
+bool MIParser::parseCallFrameSize(std::optional<unsigned> &CallFrameSize) {
assert(Token.is(MIToken::kw_call_frame_size));
lex();
unsigned Value = 0;
@@ -713,7 +713,7 @@ bool MIParser::parseBasicBlockDefinition(
std::optional<MBBSectionID> SectionID;
uint64_t Alignment = 0;
std::optional<UniqueBBID> BBID;
- unsigned CallFrameSize = 0;
+ std::optional<unsigned> CallFrameSize;
BasicBlock *BB = nullptr;
if (consumeIfPresent(MIToken::lparen)) {
do {
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index 5d06af3ebf3360..a948efb41d7152 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -578,9 +578,9 @@ void MachineBasicBlock::printName(raw_ostream &os, unsigned printNameFlags,
os << " " << getBBID()->CloneID;
hasAttributes = true;
}
- if (CallFrameSize != 0) {
+ if (CallFrameSize.has_value()) {
os << (hasAttributes ? ", " : " (");
- os << "call-frame-size " << CallFrameSize;
+ os << "call-frame-size " << CallFrameSize.value();
hasAttributes = true;
}
}
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 759201ed9dadc7..62ece9ef54b58b 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -3837,11 +3837,11 @@ void MachineVerifier::verifyStackFrame() {
BBState.ExitIsSetup = BBState.EntryIsSetup;
}
- if ((int)MBB->getCallFrameSize() != -BBState.EntryValue) {
+ if ((int)MBB->getCallFrameSizeOrZero() != -BBState.EntryValue) {
report("Call frame size on entry does not match value computed from "
"predecessor",
MBB);
- errs() << "Call frame size on entry " << MBB->getCallFrameSize()
+ errs() << "Call frame size on entry " << MBB->getCallFrameSizeOrZero()
<< " does not match value computed from predecessor "
<< -BBState.EntryValue << '\n';
}
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index ee03eaa8ae527c..b2f355c0aa8ccf 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -392,9 +392,9 @@ void PEI::calculateCallFrameInfo(MachineFunction &MF) {
TFI->eliminateCallFramePseudoInstr(MF, *I->getParent(), I);
// We can't track the call frame size after call frame pseudos have been
- // eliminated. Set it to zero everywhere to keep MachineVerifier happy.
+ // eliminated. Clear it everywhere to keep MachineVerifier happy.
for (MachineBasicBlock &MBB : MF)
- MBB.setCallFrameSize(0);
+ MBB.clearCallFrameSize();
}
}
@@ -1350,11 +1350,11 @@ void PEI::replaceFrameIndicesBackward(MachineFunction &MF) {
// Get the SP adjustment for the end of MBB from the start of any of its
// successors. They should all be the same.
assert(all_of(MBB.successors(), [&MBB](const MachineBasicBlock *Succ) {
- return Succ->getCallFrameSize() ==
- (*MBB.succ_begin())->getCallFrameSize();
+ return Succ->getCallFrameSizeOrZero() ==
+ (*MBB.succ_begin())->getCallFrameSizeOrZero();
}));
const MachineBasicBlock &FirstSucc = **MBB.succ_begin();
- SPAdj = TFI.alignSPAdjust(FirstSucc.getCallFrameSize());
+ SPAdj = TFI.alignSPAdjust(FirstSucc.getCallFrameSizeOrZero());
if (TFI.getStackGrowthDirection() == TargetFrameLowering::StackGrowsUp)
SPAdj = -SPAdj;
}
@@ -1362,8 +1362,8 @@ void PEI::replaceFrameIndicesBackward(MachineFunction &MF) {
replaceFrameIndicesBackward(&MBB, MF, SPAdj);
// We can't track the call frame size after call frame pseudos have been
- // eliminated. Set it to zero everywhere to keep MachineVerifier happy.
- MBB.setCallFrameSize(0);
+ // eliminated. Clear it everywhere to keep MachineVerifier happy.
+ MBB.clearCallFrameSize();
}
}
@@ -1373,15 +1373,15 @@ void PEI::replaceFrameIndices(MachineFunction &MF) {
const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering();
for (auto &MBB : MF) {
- int SPAdj = TFI.alignSPAdjust(MBB.getCallFrameSize());
+ int SPAdj = TFI.alignSPAdjust(MBB.getCallFrameSizeOrZero());
if (TFI.getStackGrowthDirection() == TargetFrameLowering::StackGrowsUp)
SPAdj = -SPAdj;
replaceFrameIndices(&MBB, MF, SPAdj);
// We can't track the call frame size after call frame pseudos have been
- // eliminated. Set it to zero everywhere to keep MachineVerifier happy.
- MBB.setCallFrameSize(0);
+ // eliminated. Clear it everywhere to keep MachineVerifier happy.
+ MBB.clearCallFrameSize();
}
}
diff --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp
index 38bd0b0ba4114c..f623ba0352de8b 100644
--- a/llvm/lib/CodeGen/TargetInstrInfo.cpp
+++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp
@@ -1624,15 +1624,15 @@ TargetInstrInfo::describeLoadedValue(const MachineInstr &MI,
return std::nullopt;
}
-// Get the call frame size just before MI.
-unsigned TargetInstrInfo::getCallFrameSizeAt(MachineInstr &MI) const {
+std::optional<unsigned>
+TargetInstrInfo::getCallFrameSizeAt(MachineInstr &MI) const {
// Search backwards from MI for the most recent call frame instruction.
MachineBasicBlock *MBB = MI.getParent();
for (auto &AdjI : reverse(make_range(MBB->instr_begin(), MI.getIterator()))) {
if (AdjI.getOpcode() == getCallFrameSetupOpcode())
return getFrameTotalSize(AdjI);
if (AdjI.getOpcode() == getCallFrameDestroyOpcode())
- return 0;
+ return std::nullopt;
}
// If none was found, use the call frame size from the start of the basic
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 9096617a948557..e2c57d8b9ac524 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -11594,7 +11594,7 @@ ARMTargetLowering::EmitStructByval(MachineInstr &MI,
MF->insert(It, exitMBB);
// Set the call frame size on entry to the new basic blocks.
- unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
+ std::optional<unsigned> CallFrameSize = TII->getCallFrameSizeAt(MI);
loopMBB->setCallFrameSize(CallFrameSize);
exitMBB->setCallFrameSize(CallFrameSize);
@@ -12195,7 +12195,7 @@ ARMTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
F->insert(It, sinkMBB);
// Set the call frame size on entry to the new basic blocks.
- unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
+ std::optional<unsigned> CallFrameSize = TII->getCallFrameSizeAt(MI);
copy0MBB->setCallFrameSize(CallFrameSize);
sinkMBB->setCallFrameSize(CallFrameSize);
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index 0046e757f4efa2..cf6f026869da80 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -2424,7 +2424,7 @@ AVRTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
MF->insert(I, falseMBB);
// Set the call frame size on entry to the new basic blocks.
- unsigned CallFrameSize = TII.getCallFrameSizeAt(MI);
+ std::optional<unsigned> CallFrameSize = TII.getCallFrameSizeAt(MI);
trueMBB->setCallFrameSize(CallFrameSize);
falseMBB->setCallFrameSize(CallFrameSize);
diff --git a/llvm/lib/Target/M68k/M68kISelLowering.cpp b/llvm/lib/Target/M68k/M68kISelLowering.cpp
index 316a6eebc2db0f..3b0a0c83d94d16 100644
--- a/llvm/lib/Target/M68k/M68kISelLowering.cpp
+++ b/llvm/lib/Target/M68k/M68kISelLowering.cpp
@@ -3199,7 +3199,7 @@ M68kTargetLowering::EmitLoweredSelect(MachineInstr &MI,
F->insert(It, SinkMBB);
// Set the call frame size on entry to the new basic blocks.
- unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
+ std::optional<unsigned> CallFrameSize = TII->getCallFrameSizeAt(MI);
Copy0MBB->setCallFrameSize(CallFrameSize);
SinkMBB->setCallFrameSize(CallFrameSize);
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 8a0858e2462520..f56443770095c7 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -13031,7 +13031,7 @@ PPCTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
// Set the call frame size on entry to the new basic blocks.
// See https://reviews.llvm.org/D156113.
- unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
+ std::optional<unsigned> CallFrameSize = TII->getCallFrameSizeAt(MI);
copy0MBB->setCallFrameSize(CallFrameSize);
sinkMBB->setCallFrameSize(CallFrameSize);
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index d02078372b24a2..7fcc624e834b9e 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -18444,7 +18444,8 @@ static MachineBasicBlock *emitSelectPseudo(MachineInstr &MI,
F->insert(I, TailMBB);
// Set the call frame size on entry to the new basic blocks.
- unsigned CallFrameSize = TII.getCallFrameSizeAt(*LastSelectPseudo);
+ std::optional<unsigned> CallFrameSize =
+ TII.getCallFrameSizeAt(*LastSelectPseudo);
IfFalseMBB->setCallFrameSize(CallFrameSize);
TailMBB->setCallFrameSize(CallFrameSize);
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index f011249d295040..bbee0af109c74b 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -35264,7 +35264,7 @@ X86TargetLowering::EmitLoweredSelect(MachineInstr &MI,
F->insert(It, SinkMBB);
// Set the call frame size on entry to the new basic blocks.
- unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
+ std::optional<unsigned> CallFrameSize = TII->getCallFrameSizeAt(MI);
FalseMBB->setCallFrameSize(CallFrameSize);
SinkMBB->setCallFrameSize(CallFrameSize);
diff --git a/llvm/test/CodeGen/MIR/ARM/call-frame-size.mir b/llvm/test/CodeGen/MIR/ARM/call-frame-size.mir
index d3e87120693cf3..58e7f50e583455 100644
--- a/llvm/test/CodeGen/MIR/ARM/call-frame-size.mir
+++ b/llvm/test/CodeGen/MIR/ARM/call-frame-size.mir
@@ -23,3 +23,25 @@ body: |
ADJCALLSTACKUP 100, 0, 14, $noreg, implicit-def $sp, implicit $sp
bb.2:
...
+
+---
+name: callframesizezero
+body: |
+ ; CHECK-LABEL: name: callframesizezero
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 0, 0, 14 /* CC::al */, $noreg, implicit-def $sp, implicit $sp
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1 (call-frame-size 0):
+ ; CHECK-NEXT: successors: %bb.2(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: ADJCALLSTACKUP 0, 0, 14 /* CC::al */, $noreg, implicit-def $sp, implicit $sp
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ bb.0:
+ ADJCALLSTACKDOWN 0, 0, 14, $noreg, implicit-def $sp, implicit $sp
+ bb.1 (call-frame-size 0):
+ ADJCALLSTACKUP 0, 0, 14, $noreg, implicit-def $sp, implicit $sp
+ bb.2:
+...
``````````
</details>
https://github.com/llvm/llvm-project/pull/106964
More information about the llvm-commits
mailing list