Skip to content

[AMDGPU] Avoid resource propagation for recursion through multiple functions #111004

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

Merged
merged 6 commits into from
Oct 11, 2024
Merged

[AMDGPU] Avoid resource propagation for recursion through multiple functions #111004

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ded865a
[AMDGPU] Avoid metadata propagation for recursion through multiple fu…
JanekvO Oct 3, 2024
e217863
Different approach, now walks over the sub-symbols and searches for u…
JanekvO Oct 4, 2024
2dc357c
Comments, Visited set with assert to make sure no recursion exists in…
JanekvO Oct 9, 2024
10382ea
Feedback, use WorkList instead of recurse, llvm_unreachable instead of
JanekvO Oct 10, 2024
454c317
Feedback, comments, merge nested conditional
JanekvO Oct 10, 2024
6d1ff45
Formatting
JanekvO Oct 11, 2024
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
22 changes: 16 additions & 6 deletions llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,17 @@ MCSymbol *MCResourceInfo::getMaxSGPRSymbol(MCContext &OutContext) {
return OutContext.getOrCreateSymbol("amdgpu.max_num_sgpr");
}

// The (partially complete) expression should have no recursion in it. After
// all, we're trying to avoid recursion using this codepath.
Copy link
Contributor

@arsenm arsenm Oct 10, 2024

Choose a reason for hiding this comment

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

Document return value

static bool findSymbolInExpr(MCSymbol *Sym, const MCExpr *Expr,
SmallVectorImpl<const MCExpr *> &Exprs) {
SmallVectorImpl<const MCExpr *> &Exprs,
SmallPtrSetImpl<const MCExpr *> &Visited) {
// Assert if any of the expressions is already visited (i.e., there is
// existing recursion).
assert(!Visited.contains(Expr) &&
"Expr should not exist in Visited as we're avoiding recursion");
Visited.insert(Expr);

switch (Expr->getKind()) {
default:
return false;
Expand All @@ -107,17 +116,17 @@ static bool findSymbolInExpr(MCSymbol *Sym, const MCExpr *Expr,
}
case MCExpr::ExprKind::Binary: {
const MCBinaryExpr *BExpr = cast<MCBinaryExpr>(Expr);
return findSymbolInExpr(Sym, BExpr->getLHS(), Exprs) ||
findSymbolInExpr(Sym, BExpr->getRHS(), Exprs);
return findSymbolInExpr(Sym, BExpr->getLHS(), Exprs, Visited) ||
findSymbolInExpr(Sym, BExpr->getRHS(), Exprs, Visited);
}
case MCExpr::ExprKind::Unary: {
const MCUnaryExpr *UExpr = cast<MCUnaryExpr>(Expr);
return findSymbolInExpr(Sym, UExpr->getSubExpr(), Exprs);
return findSymbolInExpr(Sym, UExpr->getSubExpr(), Exprs, Visited);
}
case MCExpr::ExprKind::Target: {
const AMDGPUMCExpr *AGVK = cast<AMDGPUMCExpr>(Expr);
for (const MCExpr *E : AGVK->getArgs()) {
if (findSymbolInExpr(Sym, E, Exprs))
if (findSymbolInExpr(Sym, E, Exprs, Visited))
return true;
}
return false;
Expand All @@ -132,11 +141,12 @@ static bool findSymbolInExpr(MCSymbol *Sym, const MCExpr *Expr,
// contains the symbol Expr is associated with.
static bool foundRecursiveSymbolDef(MCSymbol *Sym, const MCExpr *Expr) {
SmallVector<const MCExpr *, 8> WorkList;
SmallPtrSet<const MCExpr *, 8> Visited;
WorkList.push_back(Expr);

while (!WorkList.empty()) {
const MCExpr *CurExpr = WorkList.pop_back_val();
if (findSymbolInExpr(Sym, CurExpr, WorkList))
if (findSymbolInExpr(Sym, CurExpr, WorkList, Visited))
return true;
}

Expand Down
Loading