Skip to content

Commit ba40a63

Browse files
committed
[CUDA][HIP] Fix overriding of constexpr virtual function (llvm#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 Change-Id: Iebd472bfe94a5f447748ec63171678cca151c5a9
1 parent b0b93d6 commit ba40a63

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,7 @@ RISC-V Support
12571257

12581258
CUDA/HIP Language Changes
12591259
^^^^^^^^^^^^^^^^^^^^^^^^^
1260+
- Fixed a bug about overriding a constexpr pure-virtual member function with a non-constexpr virtual member function which causes compilation failure when including standard C++ header `format`.
12601261

12611262
- [HIP Only] add experimental support for offloading select C++ Algorithms,
12621263
which can be toggled via the ``--hipstdpar`` flag. This is available only for

clang/lib/Sema/SemaOverload.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,13 @@ Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
12821282
return Ovl_Overload;
12831283
}
12841284

1285+
template <typename AttrT> static bool hasExplicitAttr(const FunctionDecl *D) {
1286+
assert(D && "function decl should not be null");
1287+
if (auto *A = D->getAttr<AttrT>())
1288+
return !A->isImplicit();
1289+
return false;
1290+
}
1291+
12851292
static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
12861293
FunctionDecl *Old,
12871294
bool UseMemberUsingDeclRules,
@@ -1543,6 +1550,7 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
15431550
return true;
15441551
}
15451552

1553+
// At this point, it is known that the two functions have the same signature.
15461554
if (SemaRef.getLangOpts().CUDA && ConsiderCudaAttrs) {
15471555
// Don't allow overloading of destructors. (In theory we could, but it
15481556
// would be a giant change to clang.)
@@ -1555,8 +1563,19 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
15551563

15561564
// Allow overloading of functions with same signature and different CUDA
15571565
// target attributes.
1558-
if (NewTarget != OldTarget)
1566+
if (NewTarget != OldTarget) {
1567+
// Special case: non-constexpr function is allowed to override
1568+
// constexpr virtual function
1569+
if (OldMethod && NewMethod && OldMethod->isVirtual() &&
1570+
OldMethod->isConstexpr() && !NewMethod->isConstexpr() &&
1571+
!hasExplicitAttr<CUDAHostAttr>(Old) &&
1572+
!hasExplicitAttr<CUDADeviceAttr>(Old) &&
1573+
!hasExplicitAttr<CUDAHostAttr>(New) &&
1574+
!hasExplicitAttr<CUDADeviceAttr>(New)) {
1575+
return false;
1576+
}
15591577
return true;
1578+
}
15601579
}
15611580
}
15621581
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only \
2+
// RUN: -fcuda-is-device %s
3+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only %s
4+
5+
// expected-no-diagnostics
6+
7+
#include "Inputs/cuda.h"
8+
9+
class A
10+
{
11+
public:
12+
constexpr virtual int f() = 0;
13+
};
14+
15+
class B : public A
16+
{
17+
public:
18+
int f() override
19+
{
20+
return 42;
21+
}
22+
};
23+
24+
int test()
25+
{
26+
B b;
27+
return b.f();
28+
}

0 commit comments

Comments
 (0)