[PATCH] D112492: [CUDA][HIP] Allow comdat for kernels

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4290-4293
-  // Do not set COMDAT attribute for CUDA/HIP stub functions to prevent
-  // them being "merged" by the COMDAT Folding linker optimization.
-  if (D.hasAttr<CUDAGlobalAttr>())
-    return false;
----------------
rnk wrote:
> tra wrote:
> > This was added in D63277 specifically to deal with comdat-related issue on windows. 
> > 
> > We do need to have unique addresses for each kernel stub. Placing stubs into comdat may allow them to be merged into one and that would be a problem.
> > https://docs.microsoft.com/en-us/cpp/build/reference/opt-optimizations?view=vs-2019 
> > 
> > @rnk, at kpyzhov -- how do we make sure that identical functions are placed in comdat (needed for templates) but do not get comdat-folded with `/OPT:ICF`? 
> > 
> These are readonly global variables. I believe MSVC's ICF implementation changed behavior here a few times over the years. I think the current state is that they don't merge such globals. If you are using LLD, you can use safe ICF to avoid merging such globals.
> 
> @zequanwu reviewed the state of ICF in LLD in the last year, so he may know more about the current status, and he may be more helpful if you have more questions.
> 
> Finally, you could mark these globals as mutable if you really want to block ICF. That will work reliably with any linker.
Broadly, I think this change is correct. If I had reviewed D63277, I probably would have objected to it and not approved it.


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

https://reviews.llvm.org/D112492