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

[clang][CUDA] Assume unknown emission status for skipped function definitions #100124

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

kadircet
Copy link
Member

Emission status seems to be only used by cuda/openmp/hip compiles, to figure out
when to emit diagnostics. Current logic emits "uknown" when definition is
missing, so i extended that to skipped-function-bodies as well.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-openmp

@llvm/pr-subscribers-clang

Author: kadir çetinkaya (kadircet)

Changes

Emission status seems to be only used by cuda/openmp/hip compiles, to figure out
when to emit diagnostics. Current logic emits "uknown" when definition is
missing, so i extended that to skipped-function-bodies as well.


Full diff: https://github.com/llvm/llvm-project/pull/100124.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaDecl.cpp (+2-1)
  • (modified) clang/unittests/Tooling/ToolingTest.cpp (+5)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index bb25a0b3a45ae..c04c66d8c445a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -20148,7 +20148,8 @@ Sema::FunctionEmissionStatus Sema::getEmissionStatus(const FunctionDecl *FD,
     // be emitted, because (say) the definition could include "inline".
     const FunctionDecl *Def = FD->getDefinition();
 
-    return Def && !isDiscardableGVALinkage(
+    // We can't compute linkage when we skip function bodies.
+    return Def && !Def->hasSkippedBody() && !isDiscardableGVALinkage(
                       getASTContext().GetGVALinkageForFunction(Def));
   };
 
diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp
index 2e3da2cd2a701..f41a44fa0922a 100644
--- a/clang/unittests/Tooling/ToolingTest.cpp
+++ b/clang/unittests/Tooling/ToolingTest.cpp
@@ -586,6 +586,11 @@ TEST(runToolOnCode, TestSkipFunctionBody) {
   EXPECT_FALSE(runToolOnCodeWithArgs(
       std::make_unique<SkipBodyAction>(),
       "template<typename T> int skipMeNot() { an_error_here }", Args2));
+
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+      std::make_unique<SkipBodyAction>(),
+      "__inline __attribute__((__gnu_inline__)) void skipMe() {}",
+      {"--cuda-host-only", "-nocudainc", "-xcuda"}));
 }
 
 TEST(runToolOnCodeWithArgs, TestNoDepFile) {

Copy link

github-actions bot commented Jul 23, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@kadircet
Copy link
Member Author

this is fixing crashes when using clangd on cuda files. as clangd skips function bodies inside headers to speed up parsing times.

@ilya-biryukov
Copy link
Contributor

Could you reformat the code so that the clang-format presubmit is happy?
Otherwise LG.

@ilya-biryukov ilya-biryukov self-requested a review July 23, 2024 14:56
@kadircet kadircet merged commit 4ca1a90 into llvm:main Jul 25, 2024
7 checks passed
@kadircet kadircet deleted the cuda_inline_skipped branch July 25, 2024 09:21
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…initions (#100124)

Summary:
Emission status seems to be only used by cuda/openmp/hip compiles, to
figure out
when to emit diagnostics. Current logic emits "uknown" when definition
is
missing, so i extended that to skipped-function-bodies as well.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250704
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category cuda openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants