Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR for llvm/llvm-project#54560 #185

Merged
merged 2 commits into from
May 24, 2022
Merged

PR for llvm/llvm-project#54560 #185

merged 2 commits into from
May 24, 2022

Conversation

llvmbot
Copy link
Owner

@llvmbot llvmbot commented Apr 26, 2022

resolves llvm#54560

@mkuron
Copy link

mkuron commented Apr 27, 2022

The API compatibility check on this manual cherry-pick failed because @yxsamliu renamed mayExternalizeStaticVar/shouldExternalizeStaticVar to mayExternalize/shouldExternalize to better reflect their expanded use. @tstellar, do you want me to restore these back to their old names for the backport?

@tstellar
Copy link

The API compatibility check on this manual cherry-pick failed because @yxsamliu renamed mayExternalizeStaticVar/shouldExternalizeStaticVar to mayExternalize/shouldExternalize to better reflect their expanded use. @tstellar, do you want me to restore these back to their old names for the backport?

Yes, we need to maintain the ABI in the release branch.

kernels in anonymous name space needs to have unique name
to avoid duplicate symbols.

Fixes: llvm#54560

Reviewed by: Artem Belevich

Differential Revision: https://reviews.llvm.org/D123353

(cherry picked from commit 4ea1d43)
This patch is a continuation of https://reviews.llvm.org/D123353.

Not only kernels in anonymous namespace, but also template
kernels with template arguments in anonymous namespace
need to be externalized.

To be more generic, this patch checks the linkage of a kernel
assuming the kernel does not have __global__ attribute. If
the linkage is internal then clang will externalize it.

This patch also fixes the postfix for externalized symbol
since nvptx does not allow '.' in symbol name.

Reviewed by: Artem Belevich

Differential Revision: https://reviews.llvm.org/D124189

Fixes: llvm#54560
(cherry picked from commit 04fb816)
@mkuron
Copy link

mkuron commented May 19, 2022

@tstellar, I've updated the branch accordingly. GitHub says I need "a maintainer to approve running workflows" -- could you please do that? I expect all tests should pass this time.

#187 is a duplicate of my pull request, opened by @yxsamliu. It still has the ABI incompatibility and can thus probably be closed.

@tstellar tstellar merged commit 29f1039 into llvmbot:release/14.x May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants