-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[MIR] Support save/restore points with independent sets of registers #119358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[MIR] Support save/restore points with independent sets of registers #119358
Conversation
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-backend-risc-v Author: Elizaveta Noskova (enoskova-sc) ChangesWith this patch the possibility to store multiple Save and Restore points in MachineFrameInfo appears. As we assume to have multiple Save and Restore points we need to know the list of registers, The full support for operating with multiple Save / Restore points is supported only in RISCV backend. Shrink-Wrap points split Part 4. Part 1: #117862 Patch is 352.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119358.diff 356 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MIRYamlMapping.h b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
index 09a6ca936fe1f4..d4dc53fc0ed32c 100644
--- a/llvm/include/llvm/CodeGen/MIRYamlMapping.h
+++ b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
@@ -610,6 +610,24 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::MachineJumpTable::Entry)
namespace llvm {
namespace yaml {
+struct SRPEntry {
+ StringValue Point;
+ std::vector<StringValue> Registers;
+
+ bool operator==(const SRPEntry &Other) const {
+ return Point == Other.Point && Registers == Other.Registers;
+ }
+};
+
+using SaveRestorePoints = std::vector<SRPEntry>;
+
+template <> struct MappingTraits<SRPEntry> {
+ static void mapping(IO &YamlIO, SRPEntry &Entry) {
+ YamlIO.mapRequired("point", Entry.Point);
+ YamlIO.mapRequired("registers", Entry.Registers);
+ }
+};
+
template <> struct MappingTraits<MachineJumpTable> {
static void mapping(IO &YamlIO, MachineJumpTable &JT) {
YamlIO.mapRequired("kind", JT.Kind);
@@ -618,6 +636,14 @@ template <> struct MappingTraits<MachineJumpTable> {
}
};
+} // namespace yaml
+} // namespace llvm
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::SRPEntry)
+
+namespace llvm {
+namespace yaml {
+
/// Serializable representation of MachineFrameInfo.
///
/// Doesn't serialize attributes like 'StackAlignment', 'IsStackRealignable' and
@@ -645,8 +671,8 @@ struct MachineFrameInfo {
bool HasTailCall = false;
bool IsCalleeSavedInfoValid = false;
unsigned LocalFrameSize = 0;
- StringValue SavePoint;
- StringValue RestorePoint;
+ SaveRestorePoints SavePoints;
+ SaveRestorePoints RestorePoints;
bool operator==(const MachineFrameInfo &Other) const {
return IsFrameAddressTaken == Other.IsFrameAddressTaken &&
@@ -667,7 +693,8 @@ struct MachineFrameInfo {
HasMustTailInVarArgFunc == Other.HasMustTailInVarArgFunc &&
HasTailCall == Other.HasTailCall &&
LocalFrameSize == Other.LocalFrameSize &&
- SavePoint == Other.SavePoint && RestorePoint == Other.RestorePoint &&
+ SavePoints == Other.SavePoints &&
+ RestorePoints == Other.RestorePoints &&
IsCalleeSavedInfoValid == Other.IsCalleeSavedInfoValid;
}
};
@@ -699,10 +726,12 @@ template <> struct MappingTraits<MachineFrameInfo> {
YamlIO.mapOptional("isCalleeSavedInfoValid", MFI.IsCalleeSavedInfoValid,
false);
YamlIO.mapOptional("localFrameSize", MFI.LocalFrameSize, (unsigned)0);
- YamlIO.mapOptional("savePoint", MFI.SavePoint,
- StringValue()); // Don't print it out when it's empty.
- YamlIO.mapOptional("restorePoint", MFI.RestorePoint,
- StringValue()); // Don't print it out when it's empty.
+ YamlIO.mapOptional(
+ "savePoints", MFI.SavePoints,
+ SaveRestorePoints()); // Don't print it out when it's empty.
+ YamlIO.mapOptional(
+ "restorePoints", MFI.RestorePoints,
+ SaveRestorePoints()); // Don't print it out when it's empty.
}
};
diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 74cf94398736dd..88800d91ef51a9 100644
--- a/llvm/include/llvm/CodeGen/MachineDominators.h
+++ b/llvm/include/llvm/CodeGen/MachineDominators.h
@@ -185,6 +185,11 @@ class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
return Base::findNearestCommonDominator(A, B);
}
+ /// Returns the nearest common dominator of the given blocks.
+ /// If that tree node is a virtual root, a nullptr will be returned.
+ MachineBasicBlock *
+ findNearestCommonDominator(ArrayRef<MachineBasicBlock *> Blocks) const;
+
MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
applySplitCriticalEdges();
return Base::getNode(BB);
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 213b7ec6b3fbfb..d746466d41c3e2 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -27,6 +27,21 @@ class MachineBasicBlock;
class BitVector;
class AllocaInst;
+using SaveRestorePoints = DenseMap<MachineBasicBlock *, std::vector<Register>>;
+
+class CalleeSavedInfoPerBB {
+ DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> Map;
+
+public:
+ std::vector<CalleeSavedInfo> get(MachineBasicBlock *MBB) const {
+ return Map.lookup(MBB);
+ }
+
+ void set(DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> CSI) {
+ Map = std::move(CSI);
+ }
+};
+
/// The CalleeSavedInfo class tracks the information need to locate where a
/// callee saved register is in the current frame.
/// Callee saved reg can also be saved to a different register rather than
@@ -37,6 +52,8 @@ class CalleeSavedInfo {
int FrameIdx;
unsigned DstReg;
};
+ std::vector<MachineBasicBlock *> SpilledIn;
+ std::vector<MachineBasicBlock *> RestoredIn;
/// Flag indicating whether the register is actually restored in the epilog.
/// In most cases, if a register is saved, it is also restored. There are
/// some situations, though, when this is not the case. For example, the
@@ -58,9 +75,9 @@ class CalleeSavedInfo {
explicit CalleeSavedInfo(unsigned R, int FI = 0) : Reg(R), FrameIdx(FI) {}
// Accessors.
- Register getReg() const { return Reg; }
- int getFrameIdx() const { return FrameIdx; }
- unsigned getDstReg() const { return DstReg; }
+ Register getReg() const { return Reg; }
+ int getFrameIdx() const { return FrameIdx; }
+ unsigned getDstReg() const { return DstReg; }
void setFrameIdx(int FI) {
FrameIdx = FI;
SpilledToReg = false;
@@ -72,6 +89,16 @@ class CalleeSavedInfo {
bool isRestored() const { return Restored; }
void setRestored(bool R) { Restored = R; }
bool isSpilledToReg() const { return SpilledToReg; }
+ ArrayRef<MachineBasicBlock *> spilledIn() const { return SpilledIn; }
+ ArrayRef<MachineBasicBlock *> restoredIn() const { return RestoredIn; }
+ void addSpilledIn(MachineBasicBlock *MBB) { SpilledIn.push_back(MBB); }
+ void addRestoredIn(MachineBasicBlock *MBB) { RestoredIn.push_back(MBB); }
+ void setSpilledIn(std::vector<MachineBasicBlock *> BBV) {
+ SpilledIn = std::move(BBV);
+ }
+ void setRestoredIn(std::vector<MachineBasicBlock *> BBV) {
+ RestoredIn = std::move(BBV);
+ }
};
/// The MachineFrameInfo class represents an abstract stack frame until
@@ -295,6 +322,10 @@ class MachineFrameInfo {
/// Has CSInfo been set yet?
bool CSIValid = false;
+ CalleeSavedInfoPerBB CSInfoPerSave;
+
+ CalleeSavedInfoPerBB CSInfoPerRestore;
+
/// References to frame indices which are mapped
/// into the local frame allocation block. <FrameIdx, LocalOffset>
SmallVector<std::pair<int, int64_t>, 32> LocalFrameObjects;
@@ -331,9 +362,16 @@ class MachineFrameInfo {
bool HasTailCall = false;
/// Not null, if shrink-wrapping found a better place for the prologue.
- MachineBasicBlock *Save = nullptr;
+ MachineBasicBlock *Prolog = nullptr;
/// Not null, if shrink-wrapping found a better place for the epilogue.
- MachineBasicBlock *Restore = nullptr;
+ MachineBasicBlock *Epilog = nullptr;
+
+ /// Not empty, if shrink-wrapping found a better place for saving callee
+ /// saves.
+ SaveRestorePoints SavePoints;
+ /// Not empty, if shrink-wrapping found a better place for restoring callee
+ /// saves.
+ SaveRestorePoints RestorePoints;
/// Size of the UnsafeStack Frame
uint64_t UnsafeStackSize = 0;
@@ -809,21 +847,105 @@ class MachineFrameInfo {
/// \copydoc getCalleeSavedInfo()
std::vector<CalleeSavedInfo> &getCalleeSavedInfo() { return CSInfo; }
+ /// Returns callee saved info vector for provided save point in
+ /// the current function.
+ std::vector<CalleeSavedInfo> getCSInfoPerSave(MachineBasicBlock *MBB) const {
+ return CSInfoPerSave.get(MBB);
+ }
+
+ /// Returns callee saved info vector for provided restore point
+ /// in the current function.
+ std::vector<CalleeSavedInfo>
+ getCSInfoPerRestore(MachineBasicBlock *MBB) const {
+ return CSInfoPerRestore.get(MBB);
+ }
+
/// Used by prolog/epilog inserter to set the function's callee saved
/// information.
void setCalleeSavedInfo(std::vector<CalleeSavedInfo> CSI) {
CSInfo = std::move(CSI);
}
+ /// Used by prolog/epilog inserter to set the function's callee saved
+ /// information for particular save point.
+ void setCSInfoPerSave(
+ DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> CSI) {
+ CSInfoPerSave.set(CSI);
+ }
+
+ /// Used by prolog/epilog inserter to set the function's callee saved
+ /// information for particular restore point.
+ void setCSInfoPerRestore(
+ DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> CSI) {
+ CSInfoPerRestore.set(CSI);
+ }
+
/// Has the callee saved info been calculated yet?
bool isCalleeSavedInfoValid() const { return CSIValid; }
void setCalleeSavedInfoValid(bool v) { CSIValid = v; }
- MachineBasicBlock *getSavePoint() const { return Save; }
- void setSavePoint(MachineBasicBlock *NewSave) { Save = NewSave; }
- MachineBasicBlock *getRestorePoint() const { return Restore; }
- void setRestorePoint(MachineBasicBlock *NewRestore) { Restore = NewRestore; }
+ const SaveRestorePoints &getRestorePoints() const { return RestorePoints; }
+
+ const SaveRestorePoints &getSavePoints() const { return SavePoints; }
+
+ std::pair<MachineBasicBlock *, std::vector<Register>>
+ getRestorePoint(MachineBasicBlock *MBB) const {
+ if (auto It = RestorePoints.find(MBB); It != RestorePoints.end())
+ return *It;
+
+ std::vector<Register> Regs = {};
+ return std::make_pair(nullptr, Regs);
+ }
+
+ std::pair<MachineBasicBlock *, std::vector<Register>>
+ getSavePoint(MachineBasicBlock *MBB) const {
+ if (auto It = SavePoints.find(MBB); It != SavePoints.end())
+ return *It;
+
+ std::vector<Register> Regs = {};
+ return std::make_pair(nullptr, Regs);
+ }
+
+ void setSavePoints(SaveRestorePoints NewSavePoints) {
+ SavePoints = std::move(NewSavePoints);
+ }
+
+ void setRestorePoints(SaveRestorePoints NewRestorePoints) {
+ RestorePoints = std::move(NewRestorePoints);
+ }
+
+ void setSavePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) {
+ if (SavePoints.contains(MBB))
+ SavePoints[MBB] = Regs;
+ else
+ SavePoints.insert(std::make_pair(MBB, Regs));
+ }
+
+ static const SaveRestorePoints constructSaveRestorePoints(
+ const SaveRestorePoints &SRP,
+ const DenseMap<MachineBasicBlock *, MachineBasicBlock *> &BBMap) {
+ SaveRestorePoints Pts{};
+ for (auto &Src : SRP) {
+ Pts.insert(std::make_pair(BBMap.find(Src.first)->second, Src.second));
+ }
+ return Pts;
+ }
+
+ void setRestorePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) {
+ if (RestorePoints.contains(MBB))
+ RestorePoints[MBB] = Regs;
+ else
+ RestorePoints.insert(std::make_pair(MBB, Regs));
+ }
+
+ MachineBasicBlock *getProlog() const { return Prolog; }
+ void setProlog(MachineBasicBlock *BB) { Prolog = BB; }
+ MachineBasicBlock *getEpilog() const { return Epilog; }
+ void setEpilog(MachineBasicBlock *BB) { Epilog = BB; }
+
+ void clearSavePoints() { SavePoints.clear(); }
+ void clearRestorePoints() { RestorePoints.clear(); }
uint64_t getUnsafeStackSize() const { return UnsafeStackSize; }
void setUnsafeStackSize(uint64_t Size) { UnsafeStackSize = Size; }
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index e2543f883f91ce..835eeb2362ceb1 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -124,6 +124,10 @@ class MIRParserImpl {
bool initializeFrameInfo(PerFunctionMIParsingState &PFS,
const yaml::MachineFunction &YamlMF);
+ bool initializeSaveRestorePoints(PerFunctionMIParsingState &PFS,
+ const yaml::SaveRestorePoints &YamlSRP,
+ bool IsSavePoints);
+
bool initializeCallSiteInfo(PerFunctionMIParsingState &PFS,
const yaml::MachineFunction &YamlMF);
@@ -832,18 +836,9 @@ bool MIRParserImpl::initializeFrameInfo(PerFunctionMIParsingState &PFS,
MFI.setHasTailCall(YamlMFI.HasTailCall);
MFI.setCalleeSavedInfoValid(YamlMFI.IsCalleeSavedInfoValid);
MFI.setLocalFrameSize(YamlMFI.LocalFrameSize);
- if (!YamlMFI.SavePoint.Value.empty()) {
- MachineBasicBlock *MBB = nullptr;
- if (parseMBBReference(PFS, MBB, YamlMFI.SavePoint))
- return true;
- MFI.setSavePoint(MBB);
- }
- if (!YamlMFI.RestorePoint.Value.empty()) {
- MachineBasicBlock *MBB = nullptr;
- if (parseMBBReference(PFS, MBB, YamlMFI.RestorePoint))
- return true;
- MFI.setRestorePoint(MBB);
- }
+ initializeSaveRestorePoints(PFS, YamlMFI.SavePoints, true /*IsSavePoints*/);
+ initializeSaveRestorePoints(PFS, YamlMFI.RestorePoints,
+ false /*IsSavePoints*/);
std::vector<CalleeSavedInfo> CSIInfo;
// Initialize the fixed frame objects.
@@ -1058,8 +1053,40 @@ bool MIRParserImpl::initializeConstantPool(PerFunctionMIParsingState &PFS,
return false;
}
-bool MIRParserImpl::initializeJumpTableInfo(PerFunctionMIParsingState &PFS,
- const yaml::MachineJumpTable &YamlJTI) {
+bool MIRParserImpl::initializeSaveRestorePoints(
+ PerFunctionMIParsingState &PFS, const yaml::SaveRestorePoints &YamlSRP,
+ bool IsSavePoints) {
+ SMDiagnostic Error;
+ MachineFunction &MF = PFS.MF;
+ MachineFrameInfo &MFI = MF.getFrameInfo();
+ llvm::SaveRestorePoints SRPoints;
+
+ for (const auto &Entry : YamlSRP) {
+ const auto &MBBSource = Entry.Point;
+ MachineBasicBlock *MBB = nullptr;
+ if (parseMBBReference(PFS, MBB, MBBSource.Value))
+ return true;
+
+ std::vector<Register> Registers{};
+ for (auto &RegStr : Entry.Registers) {
+ Register Reg;
+ if (parseNamedRegisterReference(PFS, Reg, RegStr.Value, Error))
+ return error(Error, RegStr.SourceRange);
+
+ Registers.push_back(Reg);
+ }
+ SRPoints.insert(std::make_pair(MBB, Registers));
+ }
+
+ if (IsSavePoints)
+ MFI.setSavePoints(SRPoints);
+ else
+ MFI.setRestorePoints(SRPoints);
+ return false;
+}
+
+bool MIRParserImpl::initializeJumpTableInfo(
+ PerFunctionMIParsingState &PFS, const yaml::MachineJumpTable &YamlJTI) {
MachineJumpTableInfo *JTI = PFS.MF.getOrCreateJumpTableInfo(YamlJTI.Kind);
for (const auto &Entry : YamlJTI.Entries) {
std::vector<MachineBasicBlock *> Blocks;
diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index c8f6341c1224d2..ea7c504d355c19 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/lib/CodeGen/MIRPrinter.cpp
@@ -117,7 +117,10 @@ class MIRPrinter {
const MachineRegisterInfo &RegInfo,
const TargetRegisterInfo *TRI);
void convert(ModuleSlotTracker &MST, yaml::MachineFrameInfo &YamlMFI,
- const MachineFrameInfo &MFI);
+ const MachineFrameInfo &MFI, const TargetRegisterInfo *TRI);
+ void convert(ModuleSlotTracker &MST, yaml::SaveRestorePoints &YamlSRP,
+ const DenseMap<MachineBasicBlock *, std::vector<Register>> &SRP,
+ const TargetRegisterInfo *TRI);
void convert(yaml::MachineFunction &MF,
const MachineConstantPool &ConstantPool);
void convert(ModuleSlotTracker &MST, yaml::MachineJumpTable &YamlJTI,
@@ -235,7 +238,8 @@ void MIRPrinter::print(const MachineFunction &MF) {
convert(YamlMF, MF, MF.getRegInfo(), MF.getSubtarget().getRegisterInfo());
MachineModuleSlotTracker MST(MMI, &MF);
MST.incorporateFunction(MF.getFunction());
- convert(MST, YamlMF.FrameInfo, MF.getFrameInfo());
+ convert(MST, YamlMF.FrameInfo, MF.getFrameInfo(),
+ MF.getSubtarget().getRegisterInfo());
convertStackObjects(YamlMF, MF, MST);
convertEntryValueObjects(YamlMF, MF, MST);
convertCallSiteObjects(YamlMF, MF, MST);
@@ -372,7 +376,8 @@ void MIRPrinter::convert(yaml::MachineFunction &YamlMF,
void MIRPrinter::convert(ModuleSlotTracker &MST,
yaml::MachineFrameInfo &YamlMFI,
- const MachineFrameInfo &MFI) {
+ const MachineFrameInfo &MFI,
+ const TargetRegisterInfo *TRI) {
YamlMFI.IsFrameAddressTaken = MFI.isFrameAddressTaken();
YamlMFI.IsReturnAddressTaken = MFI.isReturnAddressTaken();
YamlMFI.HasStackMap = MFI.hasStackMap();
@@ -392,14 +397,10 @@ void MIRPrinter::convert(ModuleSlotTracker &MST,
YamlMFI.HasTailCall = MFI.hasTailCall();
YamlMFI.IsCalleeSavedInfoValid = MFI.isCalleeSavedInfoValid();
YamlMFI.LocalFrameSize = MFI.getLocalFrameSize();
- if (MFI.getSavePoint()) {
- raw_string_ostream StrOS(YamlMFI.SavePoint.Value);
- StrOS << printMBBReference(*MFI.getSavePoint());
- }
- if (MFI.getRestorePoint()) {
- raw_string_ostream StrOS(YamlMFI.RestorePoint.Value);
- StrOS << printMBBReference(*MFI.getRestorePoint());
- }
+ if (!MFI.getSavePoints().empty())
+ convert(MST, YamlMFI.SavePoints, MFI.getSavePoints(), TRI);
+ if (!MFI.getRestorePoints().empty())
+ convert(MST, YamlMFI.RestorePoints, MFI.getRestorePoints(), TRI);
}
void MIRPrinter::convertEntryValueObjects(yaml::MachineFunction &YMF,
@@ -618,6 +619,28 @@ void MIRPrinter::convert(yaml::MachineFunction &MF,
}
}
+void MIRPrinter::convert(ModuleSlotTracker &MST,
+ yaml::SaveRestorePoints &YamlSRP,
+ const SaveRestorePoints &SRP,
+ const TargetRegisterInfo *TRI) {
+ for (const auto &MBBEntry : SRP) {
+ std::string Str;
+ yaml::SRPEntry Entry;
+ raw_string_ostream StrOS(Str);
+ StrOS << printMBBReference(*MBBEntry.first);
+ Entry.Point = StrOS.str();
+ Str.clear();
+ for (auto &Reg : MBBEntry.second) {
+ if (Reg != MCRegister::NoRegister) {
+ StrOS << printReg(Reg, TRI);
+ Entry.Registers.push_back(StrOS.str());
+ Str.clear();
+ }
+ }
+ YamlSRP.push_back(Entry);
+ }
+}
+
void MIRPrinter::convert(ModuleSlotTracker &MST,
yaml::MachineJumpTable &YamlJTI,
const MachineJumpTableInfo &JTI) {
diff --git a/llvm/lib/CodeGen/MachineDominators.cpp b/llvm/lib/CodeGen/MachineDominators.cpp
index a2cc8fdfa7c9f9..384f90c6da66c0 100644
--- a/llvm/lib/CodeGen/MachineDominators.cpp
+++ b/llvm/lib/CodeGen/MachineDominators.cpp
@@ -189,3 +189,19 @@ void MachineDominatorTree::applySplitCriticalEdges() const {
NewBBs.clear();
CriticalEdgesToSplit.clear();
}
+
+MachineBasicBlock *MachineDominatorTree::findNearestCommonDominator(
+ ArrayRef<MachineBasicBlock *> Blocks) const {
+ assert(!Blocks.empty());
+
+ MachineBasicBlock *NCD = Blocks.front();
+ for (MachineBasicBlock *BB : Blocks.drop_front()) {
+ NCD = Base::findNearestCommonDominator(NCD, BB);
+
+ // Stop when the root is reached.
+ if (Base::isVirtualRoot(Base::getNode(NCD)))
+ return nullptr;
+ }
+
+ return NCD;
+}
diff --git a/llvm/lib/CodeGen/MachineFrameInfo.cpp b/llvm/lib/CodeGen/MachineFrameInfo.cpp
index e4b993850f73dc..c6658d2e9eba88 100644
--- a/llvm/lib/CodeGen/MachineFrameInfo.cpp
+++ b/llvm/lib/CodeGen/MachineFrameInfo.cpp
@@ -244,6 +244,23 @@ void MachineFrameInfo::print(const MachineFunction &MF, raw_ostream &OS) const{
}
OS << "\n";
}
+
+ OS << "save/restore points:\n";
+
+ if (!SavePoints.empty()) {
+ OS << "save points:\n";
+
+ for (auto &item : SavePoints)
+ OS << printMBBReference(*item.first) << "\n";
+ } else
+ OS << "save points are empty\n";
+
+ if (!RestorePoints.empty()) {
+ OS << "restore points:\n";
+ for (auto &item : RestorePoints)
+ OS << printMBBReference(*item.first) << "\n";
+ } else
+ OS << "restore points are empty\n";
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index 34dd79c7b6184f..d7dee80d4292ba 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -99,8 +99,12 @@ class PEI : public Machin...
[truncated]
|
@llvm/pr-subscribers-backend-arm Author: Elizaveta Noskova (enoskova-sc) ChangesWith this patch the possibility to store multiple Save and Restore points in MachineFrameInfo appears. As we assume to have multiple Save and Restore points we need to know the list of registers, The full support for operating with multiple Save / Restore points is supported only in RISCV backend. Shrink-Wrap points split Part 4. Part 1: #117862 Patch is 352.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119358.diff 356 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MIRYamlMapping.h b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
index 09a6ca936fe1f4..d4dc53fc0ed32c 100644
--- a/llvm/include/llvm/CodeGen/MIRYamlMapping.h
+++ b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
@@ -610,6 +610,24 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::MachineJumpTable::Entry)
namespace llvm {
namespace yaml {
+struct SRPEntry {
+ StringValue Point;
+ std::vector<StringValue> Registers;
+
+ bool operator==(const SRPEntry &Other) const {
+ return Point == Other.Point && Registers == Other.Registers;
+ }
+};
+
+using SaveRestorePoints = std::vector<SRPEntry>;
+
+template <> struct MappingTraits<SRPEntry> {
+ static void mapping(IO &YamlIO, SRPEntry &Entry) {
+ YamlIO.mapRequired("point", Entry.Point);
+ YamlIO.mapRequired("registers", Entry.Registers);
+ }
+};
+
template <> struct MappingTraits<MachineJumpTable> {
static void mapping(IO &YamlIO, MachineJumpTable &JT) {
YamlIO.mapRequired("kind", JT.Kind);
@@ -618,6 +636,14 @@ template <> struct MappingTraits<MachineJumpTable> {
}
};
+} // namespace yaml
+} // namespace llvm
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::SRPEntry)
+
+namespace llvm {
+namespace yaml {
+
/// Serializable representation of MachineFrameInfo.
///
/// Doesn't serialize attributes like 'StackAlignment', 'IsStackRealignable' and
@@ -645,8 +671,8 @@ struct MachineFrameInfo {
bool HasTailCall = false;
bool IsCalleeSavedInfoValid = false;
unsigned LocalFrameSize = 0;
- StringValue SavePoint;
- StringValue RestorePoint;
+ SaveRestorePoints SavePoints;
+ SaveRestorePoints RestorePoints;
bool operator==(const MachineFrameInfo &Other) const {
return IsFrameAddressTaken == Other.IsFrameAddressTaken &&
@@ -667,7 +693,8 @@ struct MachineFrameInfo {
HasMustTailInVarArgFunc == Other.HasMustTailInVarArgFunc &&
HasTailCall == Other.HasTailCall &&
LocalFrameSize == Other.LocalFrameSize &&
- SavePoint == Other.SavePoint && RestorePoint == Other.RestorePoint &&
+ SavePoints == Other.SavePoints &&
+ RestorePoints == Other.RestorePoints &&
IsCalleeSavedInfoValid == Other.IsCalleeSavedInfoValid;
}
};
@@ -699,10 +726,12 @@ template <> struct MappingTraits<MachineFrameInfo> {
YamlIO.mapOptional("isCalleeSavedInfoValid", MFI.IsCalleeSavedInfoValid,
false);
YamlIO.mapOptional("localFrameSize", MFI.LocalFrameSize, (unsigned)0);
- YamlIO.mapOptional("savePoint", MFI.SavePoint,
- StringValue()); // Don't print it out when it's empty.
- YamlIO.mapOptional("restorePoint", MFI.RestorePoint,
- StringValue()); // Don't print it out when it's empty.
+ YamlIO.mapOptional(
+ "savePoints", MFI.SavePoints,
+ SaveRestorePoints()); // Don't print it out when it's empty.
+ YamlIO.mapOptional(
+ "restorePoints", MFI.RestorePoints,
+ SaveRestorePoints()); // Don't print it out when it's empty.
}
};
diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 74cf94398736dd..88800d91ef51a9 100644
--- a/llvm/include/llvm/CodeGen/MachineDominators.h
+++ b/llvm/include/llvm/CodeGen/MachineDominators.h
@@ -185,6 +185,11 @@ class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
return Base::findNearestCommonDominator(A, B);
}
+ /// Returns the nearest common dominator of the given blocks.
+ /// If that tree node is a virtual root, a nullptr will be returned.
+ MachineBasicBlock *
+ findNearestCommonDominator(ArrayRef<MachineBasicBlock *> Blocks) const;
+
MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
applySplitCriticalEdges();
return Base::getNode(BB);
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 213b7ec6b3fbfb..d746466d41c3e2 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -27,6 +27,21 @@ class MachineBasicBlock;
class BitVector;
class AllocaInst;
+using SaveRestorePoints = DenseMap<MachineBasicBlock *, std::vector<Register>>;
+
+class CalleeSavedInfoPerBB {
+ DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> Map;
+
+public:
+ std::vector<CalleeSavedInfo> get(MachineBasicBlock *MBB) const {
+ return Map.lookup(MBB);
+ }
+
+ void set(DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> CSI) {
+ Map = std::move(CSI);
+ }
+};
+
/// The CalleeSavedInfo class tracks the information need to locate where a
/// callee saved register is in the current frame.
/// Callee saved reg can also be saved to a different register rather than
@@ -37,6 +52,8 @@ class CalleeSavedInfo {
int FrameIdx;
unsigned DstReg;
};
+ std::vector<MachineBasicBlock *> SpilledIn;
+ std::vector<MachineBasicBlock *> RestoredIn;
/// Flag indicating whether the register is actually restored in the epilog.
/// In most cases, if a register is saved, it is also restored. There are
/// some situations, though, when this is not the case. For example, the
@@ -58,9 +75,9 @@ class CalleeSavedInfo {
explicit CalleeSavedInfo(unsigned R, int FI = 0) : Reg(R), FrameIdx(FI) {}
// Accessors.
- Register getReg() const { return Reg; }
- int getFrameIdx() const { return FrameIdx; }
- unsigned getDstReg() const { return DstReg; }
+ Register getReg() const { return Reg; }
+ int getFrameIdx() const { return FrameIdx; }
+ unsigned getDstReg() const { return DstReg; }
void setFrameIdx(int FI) {
FrameIdx = FI;
SpilledToReg = false;
@@ -72,6 +89,16 @@ class CalleeSavedInfo {
bool isRestored() const { return Restored; }
void setRestored(bool R) { Restored = R; }
bool isSpilledToReg() const { return SpilledToReg; }
+ ArrayRef<MachineBasicBlock *> spilledIn() const { return SpilledIn; }
+ ArrayRef<MachineBasicBlock *> restoredIn() const { return RestoredIn; }
+ void addSpilledIn(MachineBasicBlock *MBB) { SpilledIn.push_back(MBB); }
+ void addRestoredIn(MachineBasicBlock *MBB) { RestoredIn.push_back(MBB); }
+ void setSpilledIn(std::vector<MachineBasicBlock *> BBV) {
+ SpilledIn = std::move(BBV);
+ }
+ void setRestoredIn(std::vector<MachineBasicBlock *> BBV) {
+ RestoredIn = std::move(BBV);
+ }
};
/// The MachineFrameInfo class represents an abstract stack frame until
@@ -295,6 +322,10 @@ class MachineFrameInfo {
/// Has CSInfo been set yet?
bool CSIValid = false;
+ CalleeSavedInfoPerBB CSInfoPerSave;
+
+ CalleeSavedInfoPerBB CSInfoPerRestore;
+
/// References to frame indices which are mapped
/// into the local frame allocation block. <FrameIdx, LocalOffset>
SmallVector<std::pair<int, int64_t>, 32> LocalFrameObjects;
@@ -331,9 +362,16 @@ class MachineFrameInfo {
bool HasTailCall = false;
/// Not null, if shrink-wrapping found a better place for the prologue.
- MachineBasicBlock *Save = nullptr;
+ MachineBasicBlock *Prolog = nullptr;
/// Not null, if shrink-wrapping found a better place for the epilogue.
- MachineBasicBlock *Restore = nullptr;
+ MachineBasicBlock *Epilog = nullptr;
+
+ /// Not empty, if shrink-wrapping found a better place for saving callee
+ /// saves.
+ SaveRestorePoints SavePoints;
+ /// Not empty, if shrink-wrapping found a better place for restoring callee
+ /// saves.
+ SaveRestorePoints RestorePoints;
/// Size of the UnsafeStack Frame
uint64_t UnsafeStackSize = 0;
@@ -809,21 +847,105 @@ class MachineFrameInfo {
/// \copydoc getCalleeSavedInfo()
std::vector<CalleeSavedInfo> &getCalleeSavedInfo() { return CSInfo; }
+ /// Returns callee saved info vector for provided save point in
+ /// the current function.
+ std::vector<CalleeSavedInfo> getCSInfoPerSave(MachineBasicBlock *MBB) const {
+ return CSInfoPerSave.get(MBB);
+ }
+
+ /// Returns callee saved info vector for provided restore point
+ /// in the current function.
+ std::vector<CalleeSavedInfo>
+ getCSInfoPerRestore(MachineBasicBlock *MBB) const {
+ return CSInfoPerRestore.get(MBB);
+ }
+
/// Used by prolog/epilog inserter to set the function's callee saved
/// information.
void setCalleeSavedInfo(std::vector<CalleeSavedInfo> CSI) {
CSInfo = std::move(CSI);
}
+ /// Used by prolog/epilog inserter to set the function's callee saved
+ /// information for particular save point.
+ void setCSInfoPerSave(
+ DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> CSI) {
+ CSInfoPerSave.set(CSI);
+ }
+
+ /// Used by prolog/epilog inserter to set the function's callee saved
+ /// information for particular restore point.
+ void setCSInfoPerRestore(
+ DenseMap<MachineBasicBlock *, std::vector<CalleeSavedInfo>> CSI) {
+ CSInfoPerRestore.set(CSI);
+ }
+
/// Has the callee saved info been calculated yet?
bool isCalleeSavedInfoValid() const { return CSIValid; }
void setCalleeSavedInfoValid(bool v) { CSIValid = v; }
- MachineBasicBlock *getSavePoint() const { return Save; }
- void setSavePoint(MachineBasicBlock *NewSave) { Save = NewSave; }
- MachineBasicBlock *getRestorePoint() const { return Restore; }
- void setRestorePoint(MachineBasicBlock *NewRestore) { Restore = NewRestore; }
+ const SaveRestorePoints &getRestorePoints() const { return RestorePoints; }
+
+ const SaveRestorePoints &getSavePoints() const { return SavePoints; }
+
+ std::pair<MachineBasicBlock *, std::vector<Register>>
+ getRestorePoint(MachineBasicBlock *MBB) const {
+ if (auto It = RestorePoints.find(MBB); It != RestorePoints.end())
+ return *It;
+
+ std::vector<Register> Regs = {};
+ return std::make_pair(nullptr, Regs);
+ }
+
+ std::pair<MachineBasicBlock *, std::vector<Register>>
+ getSavePoint(MachineBasicBlock *MBB) const {
+ if (auto It = SavePoints.find(MBB); It != SavePoints.end())
+ return *It;
+
+ std::vector<Register> Regs = {};
+ return std::make_pair(nullptr, Regs);
+ }
+
+ void setSavePoints(SaveRestorePoints NewSavePoints) {
+ SavePoints = std::move(NewSavePoints);
+ }
+
+ void setRestorePoints(SaveRestorePoints NewRestorePoints) {
+ RestorePoints = std::move(NewRestorePoints);
+ }
+
+ void setSavePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) {
+ if (SavePoints.contains(MBB))
+ SavePoints[MBB] = Regs;
+ else
+ SavePoints.insert(std::make_pair(MBB, Regs));
+ }
+
+ static const SaveRestorePoints constructSaveRestorePoints(
+ const SaveRestorePoints &SRP,
+ const DenseMap<MachineBasicBlock *, MachineBasicBlock *> &BBMap) {
+ SaveRestorePoints Pts{};
+ for (auto &Src : SRP) {
+ Pts.insert(std::make_pair(BBMap.find(Src.first)->second, Src.second));
+ }
+ return Pts;
+ }
+
+ void setRestorePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) {
+ if (RestorePoints.contains(MBB))
+ RestorePoints[MBB] = Regs;
+ else
+ RestorePoints.insert(std::make_pair(MBB, Regs));
+ }
+
+ MachineBasicBlock *getProlog() const { return Prolog; }
+ void setProlog(MachineBasicBlock *BB) { Prolog = BB; }
+ MachineBasicBlock *getEpilog() const { return Epilog; }
+ void setEpilog(MachineBasicBlock *BB) { Epilog = BB; }
+
+ void clearSavePoints() { SavePoints.clear(); }
+ void clearRestorePoints() { RestorePoints.clear(); }
uint64_t getUnsafeStackSize() const { return UnsafeStackSize; }
void setUnsafeStackSize(uint64_t Size) { UnsafeStackSize = Size; }
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index e2543f883f91ce..835eeb2362ceb1 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -124,6 +124,10 @@ class MIRParserImpl {
bool initializeFrameInfo(PerFunctionMIParsingState &PFS,
const yaml::MachineFunction &YamlMF);
+ bool initializeSaveRestorePoints(PerFunctionMIParsingState &PFS,
+ const yaml::SaveRestorePoints &YamlSRP,
+ bool IsSavePoints);
+
bool initializeCallSiteInfo(PerFunctionMIParsingState &PFS,
const yaml::MachineFunction &YamlMF);
@@ -832,18 +836,9 @@ bool MIRParserImpl::initializeFrameInfo(PerFunctionMIParsingState &PFS,
MFI.setHasTailCall(YamlMFI.HasTailCall);
MFI.setCalleeSavedInfoValid(YamlMFI.IsCalleeSavedInfoValid);
MFI.setLocalFrameSize(YamlMFI.LocalFrameSize);
- if (!YamlMFI.SavePoint.Value.empty()) {
- MachineBasicBlock *MBB = nullptr;
- if (parseMBBReference(PFS, MBB, YamlMFI.SavePoint))
- return true;
- MFI.setSavePoint(MBB);
- }
- if (!YamlMFI.RestorePoint.Value.empty()) {
- MachineBasicBlock *MBB = nullptr;
- if (parseMBBReference(PFS, MBB, YamlMFI.RestorePoint))
- return true;
- MFI.setRestorePoint(MBB);
- }
+ initializeSaveRestorePoints(PFS, YamlMFI.SavePoints, true /*IsSavePoints*/);
+ initializeSaveRestorePoints(PFS, YamlMFI.RestorePoints,
+ false /*IsSavePoints*/);
std::vector<CalleeSavedInfo> CSIInfo;
// Initialize the fixed frame objects.
@@ -1058,8 +1053,40 @@ bool MIRParserImpl::initializeConstantPool(PerFunctionMIParsingState &PFS,
return false;
}
-bool MIRParserImpl::initializeJumpTableInfo(PerFunctionMIParsingState &PFS,
- const yaml::MachineJumpTable &YamlJTI) {
+bool MIRParserImpl::initializeSaveRestorePoints(
+ PerFunctionMIParsingState &PFS, const yaml::SaveRestorePoints &YamlSRP,
+ bool IsSavePoints) {
+ SMDiagnostic Error;
+ MachineFunction &MF = PFS.MF;
+ MachineFrameInfo &MFI = MF.getFrameInfo();
+ llvm::SaveRestorePoints SRPoints;
+
+ for (const auto &Entry : YamlSRP) {
+ const auto &MBBSource = Entry.Point;
+ MachineBasicBlock *MBB = nullptr;
+ if (parseMBBReference(PFS, MBB, MBBSource.Value))
+ return true;
+
+ std::vector<Register> Registers{};
+ for (auto &RegStr : Entry.Registers) {
+ Register Reg;
+ if (parseNamedRegisterReference(PFS, Reg, RegStr.Value, Error))
+ return error(Error, RegStr.SourceRange);
+
+ Registers.push_back(Reg);
+ }
+ SRPoints.insert(std::make_pair(MBB, Registers));
+ }
+
+ if (IsSavePoints)
+ MFI.setSavePoints(SRPoints);
+ else
+ MFI.setRestorePoints(SRPoints);
+ return false;
+}
+
+bool MIRParserImpl::initializeJumpTableInfo(
+ PerFunctionMIParsingState &PFS, const yaml::MachineJumpTable &YamlJTI) {
MachineJumpTableInfo *JTI = PFS.MF.getOrCreateJumpTableInfo(YamlJTI.Kind);
for (const auto &Entry : YamlJTI.Entries) {
std::vector<MachineBasicBlock *> Blocks;
diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index c8f6341c1224d2..ea7c504d355c19 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/lib/CodeGen/MIRPrinter.cpp
@@ -117,7 +117,10 @@ class MIRPrinter {
const MachineRegisterInfo &RegInfo,
const TargetRegisterInfo *TRI);
void convert(ModuleSlotTracker &MST, yaml::MachineFrameInfo &YamlMFI,
- const MachineFrameInfo &MFI);
+ const MachineFrameInfo &MFI, const TargetRegisterInfo *TRI);
+ void convert(ModuleSlotTracker &MST, yaml::SaveRestorePoints &YamlSRP,
+ const DenseMap<MachineBasicBlock *, std::vector<Register>> &SRP,
+ const TargetRegisterInfo *TRI);
void convert(yaml::MachineFunction &MF,
const MachineConstantPool &ConstantPool);
void convert(ModuleSlotTracker &MST, yaml::MachineJumpTable &YamlJTI,
@@ -235,7 +238,8 @@ void MIRPrinter::print(const MachineFunction &MF) {
convert(YamlMF, MF, MF.getRegInfo(), MF.getSubtarget().getRegisterInfo());
MachineModuleSlotTracker MST(MMI, &MF);
MST.incorporateFunction(MF.getFunction());
- convert(MST, YamlMF.FrameInfo, MF.getFrameInfo());
+ convert(MST, YamlMF.FrameInfo, MF.getFrameInfo(),
+ MF.getSubtarget().getRegisterInfo());
convertStackObjects(YamlMF, MF, MST);
convertEntryValueObjects(YamlMF, MF, MST);
convertCallSiteObjects(YamlMF, MF, MST);
@@ -372,7 +376,8 @@ void MIRPrinter::convert(yaml::MachineFunction &YamlMF,
void MIRPrinter::convert(ModuleSlotTracker &MST,
yaml::MachineFrameInfo &YamlMFI,
- const MachineFrameInfo &MFI) {
+ const MachineFrameInfo &MFI,
+ const TargetRegisterInfo *TRI) {
YamlMFI.IsFrameAddressTaken = MFI.isFrameAddressTaken();
YamlMFI.IsReturnAddressTaken = MFI.isReturnAddressTaken();
YamlMFI.HasStackMap = MFI.hasStackMap();
@@ -392,14 +397,10 @@ void MIRPrinter::convert(ModuleSlotTracker &MST,
YamlMFI.HasTailCall = MFI.hasTailCall();
YamlMFI.IsCalleeSavedInfoValid = MFI.isCalleeSavedInfoValid();
YamlMFI.LocalFrameSize = MFI.getLocalFrameSize();
- if (MFI.getSavePoint()) {
- raw_string_ostream StrOS(YamlMFI.SavePoint.Value);
- StrOS << printMBBReference(*MFI.getSavePoint());
- }
- if (MFI.getRestorePoint()) {
- raw_string_ostream StrOS(YamlMFI.RestorePoint.Value);
- StrOS << printMBBReference(*MFI.getRestorePoint());
- }
+ if (!MFI.getSavePoints().empty())
+ convert(MST, YamlMFI.SavePoints, MFI.getSavePoints(), TRI);
+ if (!MFI.getRestorePoints().empty())
+ convert(MST, YamlMFI.RestorePoints, MFI.getRestorePoints(), TRI);
}
void MIRPrinter::convertEntryValueObjects(yaml::MachineFunction &YMF,
@@ -618,6 +619,28 @@ void MIRPrinter::convert(yaml::MachineFunction &MF,
}
}
+void MIRPrinter::convert(ModuleSlotTracker &MST,
+ yaml::SaveRestorePoints &YamlSRP,
+ const SaveRestorePoints &SRP,
+ const TargetRegisterInfo *TRI) {
+ for (const auto &MBBEntry : SRP) {
+ std::string Str;
+ yaml::SRPEntry Entry;
+ raw_string_ostream StrOS(Str);
+ StrOS << printMBBReference(*MBBEntry.first);
+ Entry.Point = StrOS.str();
+ Str.clear();
+ for (auto &Reg : MBBEntry.second) {
+ if (Reg != MCRegister::NoRegister) {
+ StrOS << printReg(Reg, TRI);
+ Entry.Registers.push_back(StrOS.str());
+ Str.clear();
+ }
+ }
+ YamlSRP.push_back(Entry);
+ }
+}
+
void MIRPrinter::convert(ModuleSlotTracker &MST,
yaml::MachineJumpTable &YamlJTI,
const MachineJumpTableInfo &JTI) {
diff --git a/llvm/lib/CodeGen/MachineDominators.cpp b/llvm/lib/CodeGen/MachineDominators.cpp
index a2cc8fdfa7c9f9..384f90c6da66c0 100644
--- a/llvm/lib/CodeGen/MachineDominators.cpp
+++ b/llvm/lib/CodeGen/MachineDominators.cpp
@@ -189,3 +189,19 @@ void MachineDominatorTree::applySplitCriticalEdges() const {
NewBBs.clear();
CriticalEdgesToSplit.clear();
}
+
+MachineBasicBlock *MachineDominatorTree::findNearestCommonDominator(
+ ArrayRef<MachineBasicBlock *> Blocks) const {
+ assert(!Blocks.empty());
+
+ MachineBasicBlock *NCD = Blocks.front();
+ for (MachineBasicBlock *BB : Blocks.drop_front()) {
+ NCD = Base::findNearestCommonDominator(NCD, BB);
+
+ // Stop when the root is reached.
+ if (Base::isVirtualRoot(Base::getNode(NCD)))
+ return nullptr;
+ }
+
+ return NCD;
+}
diff --git a/llvm/lib/CodeGen/MachineFrameInfo.cpp b/llvm/lib/CodeGen/MachineFrameInfo.cpp
index e4b993850f73dc..c6658d2e9eba88 100644
--- a/llvm/lib/CodeGen/MachineFrameInfo.cpp
+++ b/llvm/lib/CodeGen/MachineFrameInfo.cpp
@@ -244,6 +244,23 @@ void MachineFrameInfo::print(const MachineFunction &MF, raw_ostream &OS) const{
}
OS << "\n";
}
+
+ OS << "save/restore points:\n";
+
+ if (!SavePoints.empty()) {
+ OS << "save points:\n";
+
+ for (auto &item : SavePoints)
+ OS << printMBBReference(*item.first) << "\n";
+ } else
+ OS << "save points are empty\n";
+
+ if (!RestorePoints.empty()) {
+ OS << "restore points:\n";
+ for (auto &item : RestorePoints)
+ OS << printMBBReference(*item.first) << "\n";
+ } else
+ OS << "restore points are empty\n";
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index 34dd79c7b6184f..d7dee80d4292ba 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -99,8 +99,12 @@ class PEI : public Machin...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ddb2a49
to
72194c2
Compare
|
||
void setSavePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) { | ||
if (SavePoints.contains(MBB)) | ||
SavePoints[MBB] = Regs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to check contains
? Can't we just always use SavePoints[MBB] = Regs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, no need to check, thanks.
Addressed.
RestorePoints = std::move(NewRestorePoints); | ||
} | ||
|
||
void setSavePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const std::vector<Register> &Regs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
} | ||
|
||
void setRestorePoint(MachineBasicBlock *MBB, std::vector<Register> &Regs) { | ||
if (RestorePoints.contains(MBB)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to check contains?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
if (parseMBBReference(PFS, MBB, MBBSource.Value)) | ||
return true; | ||
|
||
std::vector<Register> Registers{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the curly braces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
Shrink-Wrap points split Part 2. RFC: https://discourse.llvm.org/t/shrink-wrap-save-restore-points-splitting/83581 Part 1: #117862 Part 3: #119357 Part 4: #119358 Part 5: #119359
…355) Shrink-Wrap points split Part 2. RFC: https://discourse.llvm.org/t/shrink-wrap-save-restore-points-splitting/83581 Part 1: llvm/llvm-project#117862 Part 3: llvm/llvm-project#119357 Part 4: llvm/llvm-project#119358 Part 5: llvm/llvm-project#119359
0f13b56
to
3ee8d04
Compare
Currently mir supports only one save and one restore point specification: ``` savePoint: '%bb.1' restorePoint: '%bb.2' ``` This patch provide possibility to have multiple save and multiple restore points in mir: ``` savePoints: - point: '%bb.1' restorePoints: - point: '%bb.2' ``` Shrink-Wrap points split Part 3. RFC: https://discourse.llvm.org/t/shrink-wrap-save-restore-points-splitting/83581 Part 1: #117862 Part 2: #119355 Part 4: #119358 Part 5: #119359
Currently mir supports only one save and one restore point specification: ``` savePoint: '%bb.1' restorePoint: '%bb.2' ``` This patch provide possibility to have multiple save and multiple restore points in mir: ``` savePoints: - point: '%bb.1' restorePoints: - point: '%bb.2' ``` Shrink-Wrap points split Part 3. RFC: https://discourse.llvm.org/t/shrink-wrap-save-restore-points-splitting/83581 Part 1: llvm/llvm-project#117862 Part 2: llvm/llvm-project#119355 Part 4: llvm/llvm-project#119358 Part 5: llvm/llvm-project#119359
@enoskova-sc Please merge main into this review, and remove all the contents of the changes which have already landed. Once you've done that, please consider whether this the next logical stepping stone. In particular, can you write a patch which does the multiple save/restore points (with the same list of registers), and do anything useful? If so, that might make a reasonable stepping stone in between. |
7bfd4a8
to
a112f35
Compare
8cbdd27
to
d691cb1
Compare
@preames, I split the 4th patch, and this part introduces information about registers spilled/restored in save/restore points (as you suggested), but it still does not provide the possibility to have multiple save/restore points. |
@preames, do you still have any doubts, concerning shrink-wrap improvement, proposed by me? |
bool isSpilledToReg() const { return SpilledToReg; } | ||
}; | ||
|
||
class SaveRestorePoints { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wrapper class does not seem to serve any purpose beyond adding a layer of abstraction, please remove it. It looks like you could reasonable use a typedef if you wanted to give this partial map type a short name.
"MFI can't contain multiple restore points!"); | ||
MachineBasicBlock *RestoreBlock = MFI.getRestorePoints().front(); | ||
const auto &RestorePoint = *MFI.getRestorePoints().begin(); | ||
MachineBasicBlock *RestoreBlock = RestorePoint.first; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable appears unused.
This looks like it's close to landing once comments are addressed. Please note that I rewrote the description to be more clear about what the patch is actually doing. |
I wouldn't call them "doubts", more questions about the path. After this, is your intention to post a patch which goes from MIR to correct codegen? And then after that a patch which heuristically inserts the distinct save/restore? (I see that your final patch contains all of this, but it's hard to parse apart.) |
This patch adds the MIR parsing and serialization support for save and restore points with subsets of callee saved registers. It syntactically allows a function to contain two or more distinct sub-regions with distinct subsets of registers spilled/filled as callee save. It can be useful, when one of the CSRs isn't modified in one of the sub-regions, but is in the other(s). Shrink-Wrap points split. Part 4. RFC: https://discourse.llvm.org/t/shrink-wrap-save-restore-points-splitting/83581
9d366b5
to
c61332c
Compare
After this patch I plan to support multiple points in RISC-V backend (may be in two patches, if I manage to split). And then, yes, I'll move to shrink-wrap pass and will modify its algorithm so that it can produce multiple points. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM conditional on answer to inline question.
|
||
// List of CalleeSavedInfo for registers will be added during prologepilog | ||
// pass | ||
SaveRestorePoints SavePoints({{Save, {}}}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, is the invariant here that Save was always non-null along this path? If so, the change looks okay, otherwise you're adding a null block.
(To be clear, my LGTM is conditional on the answer to this question being always non-null.)
This patch adds the MIR parsing and serialization support for save and restore points with subsets of callee saved registers. That is, it syntactically allows a function to contain two or more distinct sub-regions in which distinct subsets of registers are spilled/filled as callee save. This is useful if e.g. one of the CSRs isn't modified in one of the sub-regions, but is in the other(s).
Support for actually using this capability in code generation is still forthcoming. This patch is the next logical step for multiple save/restore points support.
All points are now stored in DenseMap from MBB to vector of CalleeSavedInfo.
Shrink-Wrap points split Part 4.
RFC: https://discourse.llvm.org/t/shrink-wrap-save-restore-points-splitting/83581
Part 1: #117862 (landed)
Part 2: #119355 (landed)
Part 3: #119357 (landed)
Part 5: #119359 (likely to be further split)