[clang] [CUDA][HIP] Fix overriding of constexpr virtual function (PR #121986)
Yaxun Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 7 11:38:30 PST 2025
https://github.com/yxsamliu created https://github.com/llvm/llvm-project/pull/121986
In C++20 constexpr virtual function is allowed. In C++17 although non-pure virtual function is not allowed to be constexpr, pure virtual function is allowed to be constexpr and is allowed to be overriden by non-constexpr virtual function in the derived class.
The following code compiles as C++:
```
class A
{
public:
constexpr virtual int f() = 0;
};
class B : public A
{
public:
int f() override
{
return 42;
}
};
```
However, it fails to compile as CUDA or HIP code. The reason: A::f() is implicitly host device function whereas B::f() is a host function. Since they have different targets, clang does not treat B::f() as an override of A::f(). Instead, it treats B::f() as a name-hiding non-virtual function for A::f(), and diagnoses it.
This causes any CUDA/HIP program using C++ standard header file <format> from g++-13 to fail to compile since such usage patten show up there:
```
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/format:3564:34: error: non-virtual member function marked 'override' hides virtual member function
3564 | _M_format_arg(size_t __id) override
| ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/format:3538:30: note: hidden overloaded virtual function 'std::__format::_Scanner<char>::_M_format_arg' declared here
3538 | constexpr virtual void _M_format_arg(size_t __id) = 0;
| ^
```
This is a serious issue and there is no workaround.
This patch allows non-constexpr function to override constexpr virtual function for CUDA and HIP. This should be OK since non-constexpr function without explicit host or device attribute can only be called in host functions.
Fixes: SWDEV-507350
>From cd7ada405dbc075075e6e6a61a322694b737208d Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" <yaxun.liu at amd.com>
Date: Tue, 7 Jan 2025 13:52:09 -0500
Subject: [PATCH] [CUDA][HIP] Fix overriding of constexpr virtual function
In C++20 constexpr virtual function is allowed. In C++17 although non-pure virtual function
is not allowed to be constexpr, pure virtual function is allowed to be constexpr and is
allowed to be overriden by non-constexpr virtual function in the derived class.
The following code compiles as C++:
```
class A
{
public:
constexpr virtual int f() = 0;
};
class B : public A
{
public:
int f() override
{
return 42;
}
};
```
However, it fails to compile as CUDA or HIP code. The reason: A::f() is implicitly
host device function whereas B::f() is a host function. Since they have different
targets, clang does not treat B::f() as an override of A::f(). Instead, it treats
B::f() as a name-hiding non-virtual function for A::f(), and diagnoses it.
This causes any CUDA/HIP program using C++ standard header file <format>
from g++-13 to fail to compile since such usage patten show up there:
```
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/format:3564:34: error: non-virtual member function marked 'override' hides virtual member function
3564 | _M_format_arg(size_t __id) override
| ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/format:3538:30: note: hidden overloaded virtual function 'std::__format::_Scanner<char>::_M_format_arg' declared here
3538 | constexpr virtual void _M_format_arg(size_t __id) = 0;
| ^
```
This is a serious issue and there is no workaround.
This patch allows non-constexpr function to override constexpr virtual function
for CUDA and HIP. This should be OK since non-constexpr function without explicit
host or device attribute can only be called in host functions.
Fixes: SWDEV-507350
---
clang/lib/Sema/SemaOverload.cpp | 26 ++++++++++++++++-
clang/test/SemaCUDA/constexpr-virtual-func.cu | 28 +++++++++++++++++++
2 files changed, 53 insertions(+), 1 deletion(-)
create mode 100644 clang/test/SemaCUDA/constexpr-virtual-func.cu
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7589701fb81de9..ed44a7666b0e28 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1309,6 +1309,16 @@ Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
return Ovl_Overload;
}
+template <typename AttrT> static bool hasExplicitAttr(const FunctionDecl *D) {
+ if (!D)
+ return false;
+ if (auto *A = D->getAttr<AttrT>())
+ return !A->isImplicit();
+ return false;
+}
+/// Assuming \p New is either an overload or override of \p Old.
+/// \returns true if \p New is an overload of \p Old,
+/// false if \p New is an override of \p Old.
static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
FunctionDecl *Old,
bool UseMemberUsingDeclRules,
@@ -1583,6 +1593,7 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
return true;
}
+ // At this point, it is known that the two functions have the same signature.
if (SemaRef.getLangOpts().CUDA && ConsiderCudaAttrs) {
// Don't allow overloading of destructors. (In theory we could, but it
// would be a giant change to clang.)
@@ -1595,8 +1606,21 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
// Allow overloading of functions with same signature and different CUDA
// target attributes.
- if (NewTarget != OldTarget)
+ if (NewTarget != OldTarget) {
+ // Special case: non-constexpr function is allowed to override
+ // constexpr virtual function
+ const auto *OldMethod = dyn_cast<CXXMethodDecl>(Old);
+ const auto *NewMethod = dyn_cast<CXXMethodDecl>(New);
+ if (OldMethod && NewMethod && OldMethod->isVirtual() &&
+ OldMethod->isConstexpr() && !NewMethod->isConstexpr() &&
+ !hasExplicitAttr<CUDAHostAttr>(Old) &&
+ !hasExplicitAttr<CUDADeviceAttr>(Old) &&
+ !hasExplicitAttr<CUDAHostAttr>(New) &&
+ !hasExplicitAttr<CUDADeviceAttr>(New)) {
+ return false;
+ }
return true;
+ }
}
}
}
diff --git a/clang/test/SemaCUDA/constexpr-virtual-func.cu b/clang/test/SemaCUDA/constexpr-virtual-func.cu
new file mode 100644
index 00000000000000..89d909181cd94d
--- /dev/null
+++ b/clang/test/SemaCUDA/constexpr-virtual-func.cu
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN: -fcuda-is-device %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only %s
+
+// expected-no-diagnostics
+
+#include "Inputs/cuda.h"
+
+class A
+{
+public:
+ constexpr virtual int f() = 0;
+};
+
+class B : public A
+{
+public:
+ int f() override
+ {
+ return 42;
+ }
+};
+
+int test()
+{
+ B b;
+ return b.f();
+}
More information about the cfe-commits
mailing list