[SYCL] Fix Lambda Mangling in Namespace-Scope Variable Initializers.

[zahiraam]

The community counterpart to this PR can be found here: llvm/llvm-project#159115.
In that PR, we propose a fix for the lambda mangling issue in namespace-scope variable initializers. This fix has uncovered a number of related problems. @tahonermann has identified several scenarios where the lambda mangling is either incorrect or diverges from GCC’s behavior.
While the current patch addresses a high-priority issue flagged in our downstream compiler, it requires additional time and effort to thoroughly address the full range of scenarios—some of which are partially listed in llvm/llvm-project#159115. The current change successfully fixes the issue affecting our downstream compiler.
Therefore, we propose to “cherry-pick” the community PR now, while continuing work in the community repository to complete the fix.

For convenience here is the description of llvm/llvm-project#159115:
This PR addresses an issue with the mangling of lambdas used as initializers for global variables within namespaces. According to the Itanium C++ ABI, lambdas should be uniquely mangled based on their context. GCC correctly includes the name of the declared variable in the mangling context for such lambdas, avoiding the need for discriminators since each lambda is scoped to its respective variable (see https://godbolt.org/z/38Y8qvvj3).

When C++ code is compiled without CUDA, HIP, or SYCL enabled, lambdas that don't require external linkage are given internal linkage and mangled with a $ in their name. When CUDA, HIP, or SYCL support is enabled, separate compilation for host and device requires that the same mangled names are observed, even for symbols that have internal linkage. Code to use external mangling was already present, but the mangled names for lambdas were incorrectly generated in some cases:

Lambdas in the initializers of global variables used the enclosing namespace as the mangling context rather than the global variable.
When such global variables were declared in different partial namespace definitions, discriminators were allocated in the context of a specific partial namespace definition rather than in the primary namespace definition leading to mangled name clashes, some of which provoked an error, some of which resulted in an additional . disambiguator being silently added to the symbol.
This PR ensures that lambdas used as global variable initializers are mangled in the context of the declared variable and that a canonical namespace is used for allocation of mangling discriminators.

[tahonermann]

I'm not a code owner, so I can't lend a formal review approval. I've been participating in the review of the upstream PR where, as @zahiraam mentioned, more work is needed to fully address the root issues. In the meantime, I think this PR is a strict improvement over the status quo. As best I can tell, it fixes at least one known problem without exacerbating other existing issues, so I recommend applying this change while we continue to iterate on a more complete fix upstream.

[elizabethandrews]

Can you update the PR description to also explain the current fix in this PR so we have context in logs without needing to refer to the community PR.

[elizabethandrews]

Am I understanding correctly that the variable names are now included in the mangling while it was not previously?

[zahiraam]

The issues occur when using the -fsycl flag or internal OpenMP flags. Several problems can arise: depending on the scenario, either the lambda name mangling does not follow the Itanium ABI, or there are name collisions. For examples, see this https://godbolt.org/z/Yvvrjnnjv

[Fznamznon]

I'm not a code owner

@tahonermann given your involvement in SYCL I strongly recommend you to become one.

[Fznamznon]

Can it break ABI?

[zahiraam]

Can it break ABI?

As @tahonermann commented in the community PR there might be potential ABI breakage for CUDA/HIP.

[tahonermann]

@tahonermann given your involvement in SYCL I strongly recommend you to become one.

@Fznamznon, the last thing I need is more obligations and responsibilities! :)

[tahonermann]

As @tahonermann commented in the community PR there might be potential ABI breakage for CUDA/HIP.

I'm still trying to understand exactly what the ABI implications are; a discussion thread can be found here. I believe the ABI implications are the same for CUDA/HIP/SYCL and oneAPI OpenMP. The status quo is that there appears to be a number of cases (like the one this PR addresses) where GCC, Clang, and Clang+CUDA/HIP/SYCL generate different mangled names contrary to what the ABI requires.

I'm pretty convinced that the change in this PR doesn't make anything worse, that it does make some things better (as evidenced by tests), and that there are other identified issues that this PR doesn't address (see linked thread above). We should address these other cases as part of the SYCL upstreaming effort.

[zahiraam]

@intel/llvm-gatekeepers can you please let me know what's the issue with the pre commit testing? It looks like an infrastructure issue.

[sarnex]

I think so too but I reran the failed jobs just to be sure.