Skip to content

[clang][sema] Fix -Wunused-function on target_version'd file-scope Fn's #81167

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

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

jroelofs
Copy link
Contributor

@jroelofs jroelofs commented Feb 8, 2024

We should only warn if the default version is the one that is unused.

Fixes: #80227

…nctions

We should only warn if the default version is the one that is unused.

Fixes: llvm#80227
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-clang

Author: Jon Roelofs (jroelofs)

Changes

We should only warn if the default version is the one that is unused.

Fixes: #80227


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

4 Files Affected:

  • (modified) clang/include/clang/AST/Decl.h (+4)
  • (modified) clang/lib/AST/Decl.cpp (+5)
  • (modified) clang/lib/Sema/Sema.cpp (+1-1)
  • (modified) clang/test/SemaCXX/warn-unused-filescoped.cpp (+16)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index f26fb5ad5f133..854e466cfd6b0 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2615,6 +2615,10 @@ class FunctionDecl : public DeclaratorDecl,
   /// the target functionality.
   bool isTargetMultiVersion() const;
 
+  /// True if this function is the default version of a multiversioned dispatch
+  /// function as a part of the target functionality.
+  bool isTargetMultiVersionDefault() const;
+
   /// True if this function is a multiversioned dispatch function as a part of
   /// the target-clones functionality.
   bool isTargetClonesMultiVersion() const;
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 26fdfa040796e..8a79f5c0b04f7 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3537,6 +3537,11 @@ bool FunctionDecl::isTargetMultiVersion() const {
          (hasAttr<TargetAttr>() || hasAttr<TargetVersionAttr>());
 }
 
+bool FunctionDecl::isTargetMultiVersionDefault() const {
+  return isMultiVersion() && hasAttr<TargetVersionAttr>() &&
+         getAttr<TargetVersionAttr>()->isDefaultVersion();
+}
+
 bool FunctionDecl::isTargetClonesMultiVersion() const {
   return isMultiVersion() && hasAttr<TargetClonesAttr>();
 }
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 2d4e6d1d058cd..58cbcc30e098a 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1393,7 +1393,7 @@ void Sema::ActOnEndOfTranslationUnit() {
               Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
                   << /*function=*/0 << DiagD << DiagRange;
           }
-        } else {
+        } else if (!FD->isTargetMultiVersion() || FD->isTargetMultiVersionDefault()) {
           if (FD->getDescribedFunctionTemplate())
             Diag(DiagD->getLocation(), diag::warn_unused_template)
                 << /*function=*/0 << DiagD << DiagRange;
diff --git a/clang/test/SemaCXX/warn-unused-filescoped.cpp b/clang/test/SemaCXX/warn-unused-filescoped.cpp
index be8d350855c07..0c347e9e19c9d 100644
--- a/clang/test/SemaCXX/warn-unused-filescoped.cpp
+++ b/clang/test/SemaCXX/warn-unused-filescoped.cpp
@@ -236,4 +236,20 @@ constexpr int constexpr4() { return 2; }
 #endif
 }
 
+__attribute__((target_version("fp16")))
+static int not_used_fmv(void) { return 1; }
+__attribute__((target_version("fp16fml")))
+static int not_used_fmv(void) { return 2; }
+__attribute__((target_version("default")))
+static int not_used_fmv(void) { return 0; } // expected-warning {{unused function 'not_used_fmv'}}
+
+
+__attribute__((target_version("fp16")))
+static int definitely_used_fmv(void) { return 1; }
+__attribute__((target_version("fp16fml")))
+static int definitely_used_fmv(void) { return 2; }
+__attribute__((target_version("default")))
+static int definitely_used_fmv(void) { return 0; }
+int definite_user(void) { return definitely_used_fmv(); }
+
 #endif

Copy link

github-actions bot commented Feb 8, 2024

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

Copy link
Collaborator

@labrinea labrinea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Good catch, thanks for fixing the bug!

@jroelofs jroelofs merged commit 2095655 into llvm:main Feb 9, 2024
@jroelofs jroelofs deleted the jroelofs/local-fmv branch February 9, 2024 16:14
@jroelofs
Copy link
Contributor Author

jroelofs commented Feb 9, 2024

Moved the test to its own file to fix appease buildbots 9bb54b2

MaskRay added a commit that referenced this pull request Feb 9, 2024
…81302)

The spurious -Wunused-function warning issue for `target_version` #80227
also applied to `__attribute__((target(...)))` based FMV. #81167 removed
warnings for all `target`-based FMV. This patch restores the warnings
for `__attribute__((target("default")))`.
jroelofs added a commit to swiftlang/llvm-project that referenced this pull request Feb 9, 2024
…'s (llvm#81167)

We should only warn if the default version is the one that is unused.

Fixes: llvm#80227
jroelofs pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 9, 2024
…lvm#81302)

The spurious -Wunused-function warning issue for `target_version` llvm#80227
also applied to `__attribute__((target(...)))` based FMV. llvm#81167 removed
warnings for all `target`-based FMV. This patch restores the warnings
for `__attribute__((target("default")))`.
jroelofs added a commit to swiftlang/llvm-project that referenced this pull request Feb 11, 2024
…'s (llvm#81167)

We should only warn if the default version is the one that is unused.

Fixes: llvm#80227
jroelofs pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 11, 2024
…lvm#81302)

The spurious -Wunused-function warning issue for `target_version` llvm#80227
also applied to `__attribute__((target(...)))` based FMV. llvm#81167 removed
warnings for all `target`-based FMV. This patch restores the warnings
for `__attribute__((target("default")))`.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FMV] -Wunused-function incorrectly complains about versioned static functions
5 participants