Skip to content

[SYCL] Don't emit @__cxa_pure_virtual into device code #16231

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.

Sign up for GitHub

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
Dec 3, 2024
Merged

[SYCL] Don't emit @__cxa_pure_virtual into device code #16231

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0eb128
[SYCL] Don't emit @__cxa_pure_virtual into device code
AlexeySachkov Dec 2, 2024
6eeb45f
Apply comments
AlexeySachkov Dec 3, 2024
414d05b
Fix typos in test comments
AlexeySachkov Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions clang/lib/CodeGen/CGVTables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,9 +831,16 @@ void CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,

// Pure virtual member functions.
if (cast<CXXMethodDecl>(GD.getDecl())->isPureVirtual()) {
if (!PureVirtualFn)
PureVirtualFn =
getSpecialVirtualFn(CGM.getCXXABI().GetPureVirtualCallName());
if (!PureVirtualFn) {
Copy link
Contributor

@elizabethandrews elizabethandrews Dec 2, 2024

Choose a reason for hiding this comment

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

Should we just add this to getSpecialVirtualFn ?

Copy link
Contributor Author

@AlexeySachkov AlexeySachkov Dec 3, 2024

Choose a reason for hiding this comment

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

So, on the one hand, it would also cover deleted virtual functions. On the other hand, the attributes (in form of SYCL properties) for virtual functions that are callable from device cannot be applied to deleted virtual functions so it won't bring any extra benefit.

I think that the code looks cleaner if we move this change. Even though it is now a little bit more complicated to trace what are the special functions which we are ignoring for SYCL device code (because the filtering is within a helper lambda and not directly within an if pure virtual), the code overall seems cleaner to me anyway (less new { }).

Moved it in 6eeb45f

// There is no guarantee that special function for handling pure virtual
// calls will be provided by a SYCL backend compiler and therefore we
// simply emit nullptr here.
if (CGM.getLangOpts().SYCLIsDevice)
PureVirtualFn = llvm::ConstantPointerNull::get(CGM.GlobalsInt8PtrTy);
else
PureVirtualFn =
getSpecialVirtualFn(CGM.getCXXABI().GetPureVirtualCallName());
}
fnPtr = PureVirtualFn;

// Deleted virtual member functions.
Expand Down
40 changes: 40 additions & 0 deletions clang/test/CodeGenSYCL/dont-emit-cxx-pure-virtual.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// There ios no guarantee that special symbol @__cxa_pure_virtual is suppored
// by SYCL backend compiler, so we need to make sure that we don't emit it
// during SYCL device compilation.
//
// RUN: %clang_cc1 -triple spir64 -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
//
// CHECK-NOT: @__cxa_pure_virtual

SYCL_EXTERNAL bool rand();

class Base {
public:
[[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "a")]]
virtual void display() {}

virtual void pure_host() = 0;

[[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "a")]]
virtual void pure_device() = 0;
};

class Derived1 : public Base {
public:
[[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "a")]]
void display() override {}

void pure_host() override {}

[[__sycl_detail__::add_ir_attributes_function("indirectly-callable", "a")]]
void pure_device() override {}
};

SYCL_EXTERNAL void test() {
Derived1 d1;
Base *b = nullptr;
if (rand())
b = &d1;
b->display();
}

Loading