[PATCH] D71221: [HIP] Add option --gpu-max-threads-per-block=n

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:8067
+    unsigned MaxThreadsPerBlock =
+        IsHIPKernel ? M.getLangOpts().GPUMaxThreadsPerBlock : 256;
+    std::string AttrVal = std::string("1,") + llvm::utostr(MaxThreadsPerBlock);
----------------
yaxunl wrote:
> tra wrote:
> > The magic value of 256 should be defined as a constant or macro somewhere -- you're using it in multiple places.
> > Alternatively, always set LangOpts.GPUMaxThreadsPerBlock to something and skip figuring out the default everywhere else.
> For the default value of LangOpts.GPUMaxThreadsPerBlock, it tends to be target dependent. I am thinking probably should add TargetInfo.getDefaultMaxThreadsPerBlock() and use it to set the default value for LangOpts.GPUMaxThreadsPerBlock.
That could be an option. I just want to have an unambiguous source for that number.
BTW, does the value need to be hardcoded for OpenCL?

I think it would make sense for --gpu-max-threads-per-block=n to control the value for OpenCL, too.
Then you would always get the value from LangOpts.GPUMaxThreadsPerBlock  and will have only one place where you set the default, which would be OK until we make the default target-specific.


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

https://reviews.llvm.org/D71221