Skip to content

[AMDGPU] Handle unset/max flat workgroup size in waves/EU #139955

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 1 commit into from
Closed

[AMDGPU] Handle unset/max flat workgroup size in waves/EU #139955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,13 +1170,19 @@ struct AAAMDWavesPerEU : public AAAMDSizeRangeAttribute {
!AssumedGroupSize->isValidState())
return false;

unsigned MinFWGSize =
AssumedGroupSize->getAssumed().getLower().getZExtValue();
unsigned MaxFWGSize =
AssumedGroupSize->getAssumed().getUpper().getZExtValue();
if (MinFWGSize == 0 && MaxFWGSize == 0)
std::tie(MinFWGSize, MaxFWGSize) =
InfoCache.getDefaultFlatWorkGroupSize(*Func);
Copy link
Contributor

Choose a reason for hiding this comment

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

And it is also wrong to use default flat work group size here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you elaborate why ? Isn't it always safe (correctness-wise) to use max that function allows ?

Copy link
Contributor

@shiltian shiltian May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If both of them are zero, it means the flat workgroup size AA still doesn't reach fixed point since its state is still valid here.

Isn't it always safe (correctness-wise) to use max that function allows ?

Yes, it is always safe to use max, but then there is no point to have this waves per eu AA. Using the default/max value here basically pushes waves per eu to its worst state, and it can't be optimized later on, even if a more optimal flat work group size is encountered. That is because the growth of flat workgroup size and waves per eu is in different direction, and the value gets from waves per eu always dominates the values propagated here.

To fix this properly, we should not use flat workgroup size here at all, which is exactly what #123995 is doing. The intermediate value of work group size should not be used to define waves per eu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is because the growth of flat workgroup size and waves per eu is in different direction, and the value gets from waves per eu always dominates the values propagated here.

Thanks for explanation.

Yes, it is always safe to use max, but then there is no point to have this waves per eu AA. Using the default/max value here basically pushes waves per eu to its worst state, and it can't be optimized later on, even if a more optimal flat work group size is encountered

That's what I sort-of get from the comment in AAAMDFlatWorkGroupSize. However, the issue we're seeing is internal compiler error, so compiling with bad perf is always better than having crashing compiler.

To fix this properly, we should not use flat workgroup size here at all, which is exactly what #123995 is doing. The intermediate value of work group size should not be used to define waves per eu.

@arsenm can #123995 be merged ?

unsigned Min, Max;
std::tie(Min, Max) = InfoCache.getEffectiveWavesPerEU(
*Caller,
{CallerInfo->getAssumed().getLower().getZExtValue(),
CallerInfo->getAssumed().getUpper().getZExtValue() - 1},
{AssumedGroupSize->getAssumed().getLower().getZExtValue(),
AssumedGroupSize->getAssumed().getUpper().getZExtValue() - 1});
{MinFWGSize, MaxFWGSize - 1});
ConstantRange CallerRange(APInt(32, Min), APInt(32, Max + 1));
IntegerRangeState CallerRangeState(CallerRange);
Change |= clampStateAndIndicateChange(this->getState(), CallerRangeState);
Expand Down
35 changes: 35 additions & 0 deletions llvm/test/CodeGen/AMDGPU/amdgpu-attributor-max-flat-wgs.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
; RUN: opt -S -mtriple=amdgcn-unknown-amdhsa -mcpu=gfx942 -passes=amdgpu-attributor %s | FileCheck %s

; CHECK-LABEL: define internal fastcc void @call1(
; CHECK-SAME: ) #[[ATTR0:[0-9]+]]
define internal fastcc void @call1() #0 {
tail call fastcc void @call2()
ret void
}

; CHECK-LABEL: define internal fastcc void @call2(
; CHECK-SAME: ) #[[ATTR0]]
define internal fastcc void @call2() #1 {
tail call fastcc void @call5()
ret void
}

; CHECK-LABEL: define { ptr addrspace(1), ptr } @call3(
; CHECK-SAME:) #[[ATTR0]]
define { ptr addrspace(1), ptr } @call3() #2 {
tail call fastcc void @call5()
ret { ptr addrspace(1), ptr } zeroinitializer
}

; CHECK-LABEL: define internal fastcc void @call5(
; CHECK-SAME: ) #[[ATTR0]]
define internal fastcc void @call5() {
tail call fastcc void @call1()
ret void
}

attributes #0 = {"amdgpu-flat-work-group-size"="1, 1024" "target-cpu"="gfx942" }
attributes #1 = {"amdgpu-flat-work-group-size"="1, 1024" "target-cpu"="gfx942" }
attributes #2 = {"amdgpu-flat-work-group-size"="1, 256" "target-cpu"="gfx942" }

; CHECK: attributes #[[ATTR0]] = { "amdgpu-agpr-alloc"="0" "amdgpu-flat-work-group-size"="1,256" "amdgpu-no-completion-action" "amdgpu-no-default-queue" "amdgpu-no-dispatch-id" "amdgpu-no-dispatch-ptr" "amdgpu-no-flat-scratch-init" "amdgpu-no-heap-ptr" "amdgpu-no-hostcall-ptr" "amdgpu-no-implicitarg-ptr" "amdgpu-no-lds-kernel-id" "amdgpu-no-multigrid-sync-arg" "amdgpu-no-queue-ptr" "amdgpu-no-workgroup-id-x" "amdgpu-no-workgroup-id-y" "amdgpu-no-workgroup-id-z" "amdgpu-no-workitem-id-x" "amdgpu-no-workitem-id-y" "amdgpu-no-workitem-id-z" "target-cpu"="gfx942" "uniform-work-group-size"="false" }