-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Reapply [AMDGPU] Avoid resource propagation for recursion through multiple functions #112251
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
Changes from 2 commits
1b88285
5463971
ba27cd9
f99b859
b18ed55
a72d1ef
a60c29f
8a38d54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,30 +91,83 @@ MCSymbol *MCResourceInfo::getMaxSGPRSymbol(MCContext &OutContext) { | |
| return OutContext.getOrCreateSymbol("amdgpu.max_num_sgpr"); | ||
| } | ||
|
|
||
| // The expression should have no recursion in it. Test a (sub-)expression to see | ||
| // if it needs to be further visited, or if a recursion has been found. Returns | ||
| // true if Sym is found within Expr (i.e., has a recurrance of Sym found), false | ||
| // otherwise. | ||
| static bool findSymbolInExpr(MCSymbol *Sym, const MCExpr *Expr, | ||
| SmallPtrSetImpl<const MCExpr *> &Exprs) { | ||
| switch (Expr->getKind()) { | ||
| default: | ||
| return false; | ||
| case MCExpr::ExprKind::SymbolRef: { | ||
| const MCSymbolRefExpr *SymRefExpr = cast<MCSymbolRefExpr>(Expr); | ||
| const MCSymbol &SymRef = SymRefExpr->getSymbol(); | ||
| if (Sym == &SymRef) | ||
| return true; | ||
| if (SymRef.isVariable()) | ||
| Exprs.insert(SymRef.getVariableValue(/*isUsed=*/false)); | ||
| return false; | ||
| } | ||
| case MCExpr::ExprKind::Binary: { | ||
| const MCBinaryExpr *BExpr = cast<MCBinaryExpr>(Expr); | ||
| Exprs.insert(BExpr->getLHS()); | ||
| Exprs.insert(BExpr->getRHS()); | ||
| return false; | ||
| } | ||
| case MCExpr::ExprKind::Unary: { | ||
| const MCUnaryExpr *UExpr = cast<MCUnaryExpr>(Expr); | ||
| Exprs.insert(UExpr->getSubExpr()); | ||
| return false; | ||
| } | ||
| case MCExpr::ExprKind::Target: { | ||
| const AMDGPUMCExpr *AGVK = cast<AMDGPUMCExpr>(Expr); | ||
| for (const MCExpr *E : AGVK->getArgs()) | ||
| Exprs.insert(E); | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void MCResourceInfo::assignResourceInfoExpr( | ||
| int64_t LocalValue, ResourceInfoKind RIK, AMDGPUMCExpr::VariantKind Kind, | ||
| const MachineFunction &MF, const SmallVectorImpl<const Function *> &Callees, | ||
| MCContext &OutContext) { | ||
| const MCConstantExpr *LocalConstExpr = | ||
| MCConstantExpr::create(LocalValue, OutContext); | ||
| const MCExpr *SymVal = LocalConstExpr; | ||
| MCSymbol *Sym = getSymbol(MF.getName(), RIK, OutContext); | ||
| if (!Callees.empty()) { | ||
| SmallVector<const MCExpr *, 8> ArgExprs; | ||
| // Avoid recursive symbol assignment. | ||
| SmallPtrSet<const Function *, 8> Seen; | ||
| ArgExprs.push_back(LocalConstExpr); | ||
| const Function &F = MF.getFunction(); | ||
| Seen.insert(&F); | ||
|
|
||
| for (const Function *Callee : Callees) { | ||
| if (!Seen.insert(Callee).second) | ||
| continue; | ||
|
|
||
| SmallPtrSet<const MCExpr *, 8> WorkSet; | ||
| MCSymbol *CalleeValSym = getSymbol(Callee->getName(), RIK, OutContext); | ||
| ArgExprs.push_back(MCSymbolRefExpr::create(CalleeValSym, OutContext)); | ||
| if (CalleeValSym->isVariable()) | ||
| WorkSet.insert(CalleeValSym->getVariableValue(/*IsUsed=*/false)); | ||
| else | ||
| ArgExprs.push_back(MCSymbolRefExpr::create(CalleeValSym, OutContext)); | ||
|
|
||
| bool FoundRecursion = false; | ||
| while (!WorkSet.empty() && !FoundRecursion) { | ||
| auto It = WorkSet.begin(); | ||
| const MCExpr *Expr = *It; | ||
| WorkSet.erase(Expr); | ||
|
|
||
| FoundRecursion = findSymbolInExpr(Sym, Expr, WorkSet); | ||
|
||
| } | ||
|
|
||
| if (CalleeValSym->isVariable() && !FoundRecursion) | ||
| ArgExprs.push_back(MCSymbolRefExpr::create(CalleeValSym, OutContext)); | ||
| } | ||
| SymVal = AMDGPUMCExpr::create(Kind, ArgExprs, OutContext); | ||
| if (ArgExprs.size() > 1) | ||
| SymVal = AMDGPUMCExpr::create(Kind, ArgExprs, OutContext); | ||
| } | ||
| MCSymbol *Sym = getSymbol(MF.getName(), RIK, OutContext); | ||
| Sym->setVariableValue(SymVal); | ||
| } | ||
|
|
||
|
|
@@ -155,6 +208,7 @@ void MCResourceInfo::gatherResourceInfo( | |
| // The expression for private segment size should be: FRI.PrivateSegmentSize | ||
| // + max(FRI.Callees, FRI.CalleeSegmentSize) | ||
| SmallVector<const MCExpr *, 8> ArgExprs; | ||
| MCSymbol *Sym = getSymbol(MF.getName(), RIK_PrivateSegSize, OutContext); | ||
| if (FRI.CalleeSegmentSize) | ||
| ArgExprs.push_back( | ||
| MCConstantExpr::create(FRI.CalleeSegmentSize, OutContext)); | ||
|
|
@@ -165,9 +219,24 @@ void MCResourceInfo::gatherResourceInfo( | |
| if (!Seen.insert(Callee).second) | ||
| continue; | ||
| if (!Callee->isDeclaration()) { | ||
| MCSymbol *calleeValSym = | ||
| SmallPtrSet<const MCExpr *, 8> WorkSet; | ||
| MCSymbol *CalleeValSym = | ||
| getSymbol(Callee->getName(), RIK_PrivateSegSize, OutContext); | ||
| ArgExprs.push_back(MCSymbolRefExpr::create(calleeValSym, OutContext)); | ||
| if (CalleeValSym->isVariable()) | ||
| WorkSet.insert(CalleeValSym->getVariableValue(/*IsUsed=*/false)); | ||
| else | ||
| ArgExprs.push_back(MCSymbolRefExpr::create(CalleeValSym, OutContext)); | ||
|
|
||
| bool FoundRecursion = false; | ||
| while (!WorkSet.empty() && !FoundRecursion) { | ||
| auto It = WorkSet.begin(); | ||
| const MCExpr *Expr = *It; | ||
| WorkSet.erase(Expr); | ||
|
|
||
| FoundRecursion = findSymbolInExpr(Sym, Expr, WorkSet); | ||
| } | ||
| if (CalleeValSym->isVariable() && !FoundRecursion) | ||
| ArgExprs.push_back(MCSymbolRefExpr::create(CalleeValSym, OutContext)); | ||
| } | ||
| } | ||
| const MCExpr *localConstExpr = | ||
|
|
@@ -178,8 +247,7 @@ void MCResourceInfo::gatherResourceInfo( | |
| localConstExpr = | ||
| MCBinaryExpr::createAdd(localConstExpr, transitiveExpr, OutContext); | ||
| } | ||
| getSymbol(MF.getName(), RIK_PrivateSegSize, OutContext) | ||
| ->setVariableValue(localConstExpr); | ||
| Sym->setVariableValue(localConstExpr); | ||
| } | ||
|
|
||
| auto SetToLocal = [&](int64_t LocalValue, ResourceInfoKind RIK) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a < %s | FileCheck %s | ||
|
|
||
| ; CHECK-LABEL: {{^}}qux | ||
| ; CHECK: .set qux.num_vgpr, 0 | ||
| ; CHECK: .set qux.num_agpr, 0 | ||
| ; CHECK: .set qux.numbered_sgpr, 32 | ||
| ; CHECK: .set qux.private_seg_size, 0 | ||
| ; CHECK: .set qux.uses_vcc, 0 | ||
| ; CHECK: .set qux.uses_flat_scratch, 0 | ||
| ; CHECK: .set qux.has_dyn_sized_stack, 0 | ||
| ; CHECK: .set qux.has_recursion, 0 | ||
| ; CHECK: .set qux.has_indirect_call, 0 | ||
| define void @qux() { | ||
| entry: | ||
JanekvO marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ret void | ||
| } | ||
|
|
||
| ; CHECK-LABEL: {{^}}baz | ||
| ; CHECK: .set baz.num_vgpr, max(32, qux.num_vgpr) | ||
| ; CHECK: .set baz.num_agpr, max(0, qux.num_agpr) | ||
| ; CHECK: .set baz.numbered_sgpr, max(34, qux.numbered_sgpr) | ||
| ; CHECK: .set baz.private_seg_size, 16+(max(qux.private_seg_size)) | ||
| ; CHECK: .set baz.uses_vcc, or(0, qux.uses_vcc) | ||
| ; CHECK: .set baz.uses_flat_scratch, or(0, qux.uses_flat_scratch) | ||
| ; CHECK: .set baz.has_dyn_sized_stack, or(0, qux.has_dyn_sized_stack) | ||
| ; CHECK: .set baz.has_recursion, or(1, qux.has_recursion) | ||
| ; CHECK: .set baz.has_indirect_call, or(0, qux.has_indirect_call) | ||
| define void @baz() { | ||
| entry: | ||
| call void @qux() | ||
| ret void | ||
| } | ||
|
|
||
| ; CHECK-LABEL: {{^}}bar | ||
| ; CHECK: .set bar.num_vgpr, max(32, baz.num_vgpr, qux.num_vgpr) | ||
| ; CHECK: .set bar.num_agpr, max(0, baz.num_agpr, qux.num_agpr) | ||
| ; CHECK: .set bar.numbered_sgpr, max(34, baz.numbered_sgpr, qux.numbered_sgpr) | ||
| ; CHECK: .set bar.private_seg_size, 16+(max(baz.private_seg_size, qux.private_seg_size)) | ||
| ; CHECK: .set bar.uses_vcc, or(0, baz.uses_vcc, qux.uses_vcc) | ||
| ; CHECK: .set bar.uses_flat_scratch, or(0, baz.uses_flat_scratch, qux.uses_flat_scratch) | ||
| ; CHECK: .set bar.has_dyn_sized_stack, or(0, baz.has_dyn_sized_stack, qux.has_dyn_sized_stack) | ||
| ; CHECK: .set bar.has_recursion, or(1, baz.has_recursion, qux.has_recursion) | ||
| ; CHECK: .set bar.has_indirect_call, or(0, baz.has_indirect_call, qux.has_indirect_call) | ||
| define void @bar() { | ||
| entry: | ||
| call void @baz() | ||
| call void @qux() | ||
| call void @baz() | ||
| ret void | ||
| } | ||
|
|
||
| ; CHECK-LABEL: {{^}}foo | ||
| ; CHECK: .set foo.num_vgpr, max(32, bar.num_vgpr) | ||
| ; CHECK: .set foo.num_agpr, max(0, bar.num_agpr) | ||
| ; CHECK: .set foo.numbered_sgpr, max(34, bar.numbered_sgpr) | ||
| ; CHECK: .set foo.private_seg_size, 16+(max(bar.private_seg_size)) | ||
| ; CHECK: .set foo.uses_vcc, or(0, bar.uses_vcc) | ||
| ; CHECK: .set foo.uses_flat_scratch, or(0, bar.uses_flat_scratch) | ||
| ; CHECK: .set foo.has_dyn_sized_stack, or(0, bar.has_dyn_sized_stack) | ||
| ; CHECK: .set foo.has_recursion, or(1, bar.has_recursion) | ||
| ; CHECK: .set foo.has_indirect_call, or(0, bar.has_indirect_call) | ||
| define void @foo() { | ||
| entry: | ||
| call void @bar() | ||
| ret void | ||
| } | ||
|
|
||
| ; CHECK-LABEL: {{^}}usefoo | ||
| ; CHECK: .set usefoo.num_vgpr, max(32, foo.num_vgpr) | ||
| ; CHECK: .set usefoo.num_agpr, max(0, foo.num_agpr) | ||
| ; CHECK: .set usefoo.numbered_sgpr, max(33, foo.numbered_sgpr) | ||
| ; CHECK: .set usefoo.private_seg_size, 0+(max(foo.private_seg_size)) | ||
| ; CHECK: .set usefoo.uses_vcc, or(0, foo.uses_vcc) | ||
| ; CHECK: .set usefoo.uses_flat_scratch, or(1, foo.uses_flat_scratch) | ||
| ; CHECK: .set usefoo.has_dyn_sized_stack, or(0, foo.has_dyn_sized_stack) | ||
| ; CHECK: .set usefoo.has_recursion, or(1, foo.has_recursion) | ||
| ; CHECK: .set usefoo.has_indirect_call, or(0, foo.has_indirect_call) | ||
| define amdgpu_kernel void @usefoo() { | ||
| call void @foo() | ||
| ret void | ||
| } | ||
|
|
||
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.
I expect to maintain the set for all callees to be visited, not redone on each one
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.
I tried so as well but I had a hard time re-retrieving the
MCSymbolRefExprfrom theWorkSetafter walking down it and concluding it should be added toArgExprs(I may be missing some low hanging fruit)