[PATCH] D18340: AMDGPU: allow specifying a workgroup size that needs to fit in a compute unit
Nicolai Hähnle via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 23 09:12:27 PDT 2016
nhaehnle added a comment.
Thanks. And yes, I think you're right about the spills, I got confused there.
================
Comment at: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:487-492
@@ -485,8 +486,8 @@
// FIXME: This is the maximum work group size. We should try to get
// value from the reqd_work_group_size function attribute if it is
// available.
- unsigned WorkGroupSize = 256;
+ unsigned WorkGroupSize = AMDGPU::getMaximumWorkGroupSize(ContainingFunction);
int AllocaSize =
WorkGroupSize * Mod->getDataLayout().getTypeAllocSize(AllocaTy);
----------------
Fix the comment :)
================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.cpp:62
@@ +61,3 @@
+
+}
+
----------------
I was confused by that closing brace for a moment. I'm not sure what the LLVM coding style says if anything, but personally I find something like `// anonymous namespace` helpful.
================
Comment at: lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:132
@@ +131,3 @@
+unsigned getMaximumWorkGroupSize(const Function &F) {
+ return getIntegerAttribute(F, "maximum-work-group-size", 256);
+}
----------------
According to Matt, there should be an amdgpu- prefix here.
http://reviews.llvm.org/D18340
More information about the llvm-commits
mailing list