[PATCH] D68818: [hip][cuda] Fix the extended lambda name mangling issue.

Wed Oct 16 13:49:41 PDT 2019

hliao marked an inline comment as done.
hliao added a comment.

In D68818#1711698 <https://reviews.llvm.org/D68818#1711698>, @rsmith wrote:

> Broadly, I think it's reasonable to number additional lambda expressions in CUDA compilations. However:
>
> - This is (in theory) an ABI break on the host side, as it changes the lambda numbering in inline functions and function templates and the like. That could be mitigated by using a different numbering sequence for the lambdas that are only numbered for this reason.
> - Depending on whether the call operator is a device function is unstable. If I understand the CUDA rules correctly, then in practice, because `constexpr` functions are implicitly `host device`, all lambdas will get numbered in CUDA on C++14 onwards but not in CUDA on C++11, and we generally want those modes to be ABI-compatible. I'd suggest you simplify and stabilize this by simply numbering all lambdas in CUDA mode.

I vote for the numbering of all lambdas in the CUDA mode once addressed in (D63164 <https://reviews.llvm.org/D63164> as long as we decouple the numbering from the linkage. As long as all lambdas are numbered, the first one is also mitigated. I will prepare the changes forcing all lambdas to be numbered under CUDA/HIP mode.

================
Comment at: clang/lib/Sema/SemaLambda.cpp:477
+  // mangled name. But, the mangler needs informing those re-numbered lambdas
+  // still have `internal` linkage.
+  if (getLangOpts().CUDA && Method->hasAttr<CUDADeviceAttr>()) {
----------------
rsmith wrote:
> What happens if there are other enclosing constructs that would give the lambda internal linkage (eg, an anonymous namespace or static function that might collide with one in another translation unit, or a declaration involving a type with internal linkage)? Presumably you can still suffer from mangling collisions in those cases, at least if you link together multiple translation units containing device code. Do we need (something like) unique identifiers for device code TUs to use in manglings of ostensibly internal linkage entities?
Those unnamed ones will be addressed in late patches as, currently, we only have lambda numbering issues from real workloads. I am preparing changes to address other unnamed types, namespaces, and etc.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68818