Skip to content

Refactoring changes to support emitting call graph section #83176

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Refactoring changes to support emitting call graph section #83176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5fbd39
[MIPS][CallSiteInfo][NFC] Fill CallSiteInfo only when needed
necipfazil Nov 7, 2023
86d9cf7
[CallSiteInfo][NFC] CallSiteInfo -> CallSiteInfo.ArgRegPairs
necipfazil Nov 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions llvm/include/llvm/CodeGen/MachineFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,9 +481,11 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
assert(Arg < (1 << 16) && "Arg out of range");
}
};
/// Vector of call argument and its forwarding register.
using CallSiteInfo = SmallVector<ArgRegPair, 1>;
using CallSiteInfoImpl = SmallVectorImpl<ArgRegPair>;

struct CallSiteInfo {
/// Vector of call argument and its forwarding register.
SmallVector<ArgRegPair, 1> ArgRegPairs;
};

private:
Delegate *TheDelegate = nullptr;
Expand Down Expand Up @@ -1323,8 +1325,7 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
}

/// Start tracking the arguments passed to the call \p CallI.
void addCallArgsForwardingRegs(const MachineInstr *CallI,
CallSiteInfoImpl &&CallInfo) {
void addCallSiteInfo(const MachineInstr *CallI, CallSiteInfo &&CallInfo) {
assert(CallI->isCandidateForCallSiteEntry());
bool Inserted =
CallSitesInfo.try_emplace(CallI, std::move(CallInfo)).second;
Expand Down
3 changes: 1 addition & 2 deletions llvm/include/llvm/CodeGen/SelectionDAG.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ class SelectionDAG {
SDDbgInfo *DbgInfo;

using CallSiteInfo = MachineFunction::CallSiteInfo;
using CallSiteInfoImpl = MachineFunction::CallSiteInfoImpl;

struct NodeExtraInfo {
CallSiteInfo CSInfo;
Expand Down Expand Up @@ -2265,7 +2264,7 @@ class SelectionDAG {
}

/// Set CallSiteInfo to be associated with Node.
void addCallSiteInfo(const SDNode *Node, CallSiteInfoImpl &&CallInfo) {
void addCallSiteInfo(const SDNode *Node, CallSiteInfo &&CallInfo) {
SDEI[Node].CSInfo = std::move(CallInfo);
}
/// Return CallSiteInfo associated with Node, or a default if none exists.
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,10 +798,10 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
ParamSet &Params) {
const MachineFunction *MF = CallMI->getMF();
const auto &CalleesMap = MF->getCallSitesInfo();
auto CallFwdRegsInfo = CalleesMap.find(CallMI);
auto CSInfo = CalleesMap.find(CallMI);

// There is no information for the call instruction.
if (CallFwdRegsInfo == CalleesMap.end())
if (CSInfo == CalleesMap.end())
return;

const MachineBasicBlock *MBB = CallMI->getParent();
Expand All @@ -815,7 +815,7 @@ static void collectCallSiteParameters(const MachineInstr *CallMI,
DIExpression::get(MF->getFunction().getContext(), {});

// Add all the forwarding registers into the ForwardedRegWorklist.
for (const auto &ArgReg : CallFwdRegsInfo->second) {
for (const auto &ArgReg : CSInfo->second.ArgRegPairs) {
bool InsertedReg =
ForwardedRegWorklist.insert({ArgReg.Reg, {{ArgReg.Reg, EmptyExpr}}})
.second;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/MIRParser/MIRParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,11 @@ bool MIRParserImpl::initializeCallSiteInfo(
Register Reg;
if (parseNamedRegisterReference(PFS, Reg, ArgRegPair.Reg.Value, Error))
return error(Error, ArgRegPair.Reg.SourceRange);
CSInfo.emplace_back(Reg, ArgRegPair.ArgNo);
CSInfo.ArgRegPairs.emplace_back(Reg, ArgRegPair.ArgNo);
}

if (TM.Options.EmitCallSiteInfo)
MF.addCallArgsForwardingRegs(&*CallI, std::move(CSInfo));
MF.addCallSiteInfo(&*CallI, std::move(CSInfo));
}

if (YamlMF.CallSitesInfo.size() && !TM.Options.EmitCallSiteInfo)
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/MIRPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ void MIRPrinter::convertCallSiteObjects(yaml::MachineFunction &YMF,
std::distance(CallI->getParent()->instr_begin(), CallI);
YmlCS.CallLocation = CallLocation;
// Construct call arguments and theirs forwarding register info.
for (auto ArgReg : CSInfo.second) {
for (auto ArgReg : CSInfo.second.ArgRegPairs) {
yaml::CallSiteInfo::ArgRegPair YmlArgReg;
YmlArgReg.ArgNo = ArgReg.ArgNo;
printRegMIR(ArgReg.Reg, YmlArgReg.Reg, TRI);
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,8 +888,10 @@ EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
}

if (MI->isCandidateForCallSiteEntry() &&
DAG->getTarget().Options.EmitCallSiteInfo)
MF.addCallArgsForwardingRegs(MI, DAG->getCallSiteInfo(Node));
DAG->getTarget().Options.EmitCallSiteInfo) {
// MF.addCallArgsForwardingRegs(MI, DAG->getCallSiteInfo(Node));
MF.addCallSiteInfo(MI, DAG->getCallSiteInfo(Node));
}
Comment on lines +891 to +894
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you don't need the braces. It will be clear if you just remove the commented out line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, more generally we should not commit commented out code.

I also agree this deserves to be split as two PRs and merged independently.


if (DAG->getNoMergeSiteInfo(Node)) {
MI->setFlag(MachineInstr::MIFlag::NoMerge);
Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7927,9 +7927,10 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
// Call site info is used for function's parameter entry value
// tracking. For now we track only simple cases when parameter
// is transferred through whole register.
llvm::erase_if(CSInfo, [&VA](MachineFunction::ArgRegPair ArgReg) {
return ArgReg.Reg == VA.getLocReg();
});
llvm::erase_if(CSInfo.ArgRegPairs,
[&VA](MachineFunction::ArgRegPair ArgReg) {
return ArgReg.Reg == VA.getLocReg();
});
} else {
// Add an extra level of indirection for streaming mode changes by
// using a pseudo copy node that cannot be rematerialised between a
Expand All @@ -7941,7 +7942,7 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
RegsUsed.insert(VA.getLocReg());
const TargetOptions &Options = DAG.getTarget().Options;
if (Options.EmitCallSiteInfo)
CSInfo.emplace_back(VA.getLocReg(), i);
CSInfo.ArgRegPairs.emplace_back(VA.getLocReg(), i);
}
} else {
assert(VA.isMemLoc());
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/ARMISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2571,7 +2571,7 @@ ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
}
const TargetOptions &Options = DAG.getTarget().Options;
if (Options.EmitCallSiteInfo)
CSInfo.emplace_back(VA.getLocReg(), i);
CSInfo.ArgRegPairs.emplace_back(VA.getLocReg(), i);
RegsToPass.push_back(std::make_pair(VA.getLocReg(), Arg));
} else if (isByVal) {
assert(VA.isMemLoc());
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/Mips/MipsISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3369,8 +3369,8 @@ MipsTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,

// Collect CSInfo about which register passes which parameter.
const TargetOptions &Options = DAG.getTarget().Options;
if (Options.SupportsDebugEntryValues)
CSInfo.emplace_back(VA.getLocReg(), i);
if (Options.EmitCallSiteInfo && Options.SupportsDebugEntryValues)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not affect any tests?

CSInfo.ArgRegPairs.emplace_back(VA.getLocReg(), i);

continue;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/X86/X86ISelLoweringCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2226,7 +2226,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
RegsToPass.push_back(std::make_pair(VA.getLocReg(), Arg));
const TargetOptions &Options = DAG.getTarget().Options;
if (Options.EmitCallSiteInfo)
CSInfo.emplace_back(VA.getLocReg(), I);
CSInfo.ArgRegPairs.emplace_back(VA.getLocReg(), I);
if (isVarArg && IsWin64) {
// Win64 ABI requires argument XMM reg to be copied to the corresponding
// shadow reg if callee is a varargs function.
Expand Down