[PATCH] D102134: [docs]Updated the AMD GPU Attributes documentation

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 11 11:50:36 PDT 2021


aaron.ballman added a comment.

In D102134#2751184 <https://reviews.llvm.org/D102134#2751184>, @pooja2299 wrote:

> In D102134#2747649 <https://reviews.llvm.org/D102134#2747649>, @aaron.ballman wrote:
>
>> Minor wordsmithing on the documentation changes, but more importantly: why is the correct fix to the documentation as opposed to changing the default max working group size?
>
> Hi @aaron.ballman. Thanks for your feedback! I am an outreachy applicant and totally new to this project. I am currently trying to understand the code base.

Welcome!

> So thought to update the documentation meanwhile. Later on we can change the default max working group size with your suggestion. What do you say, should we directly change the default max working group size and not the documentation?

I'm not an AMD person and so I'm not certain I'm the *best* person to answer this, but my feeling is that this is a case where the implementation should be updated rather than the docs. Otherwise, we're effectively encouraging users to churn their code (add the attribute to places they didn't use it before) with the intention of undoing that in the future. However, I'm hoping someone more familiar with AMDGPU can pipe up with their opinions. @arsenm?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102134/new/

https://reviews.llvm.org/D102134



More information about the cfe-commits mailing list