Skip to content

Conversation

@AlexeySachkov
Copy link
Contributor

This special symbol is used to handling calls to pure virtual functions in regular C++ program. However, for SYCL device code we do not have a guarantee that this symbol will be provided by SYCL backend compilers.

Considering that pure virtual function call is an undefined behavior anyway, this PR replaces @__cxa_pure_virtual uses with null during SYCL device compilation to avoid issues with unresolved symbols.

This special symbol is used to handling calls to pure virtual functions
in regular C++ program. However, for SYCL device code we do not have a
guarantee that this symbol will be provided by SYCL backend compilers.

Considering that pure virtual function call is an undefined behavior
anyway, this PR replaces `@__cxa_pure_virtual` uses with `null` during
SYCL device compilation to avoid issues with unresolved symbols.
if (!PureVirtualFn)
PureVirtualFn =
getSpecialVirtualFn(CGM.getCXXABI().GetPureVirtualCallName());
if (!PureVirtualFn) {
Copy link
Contributor

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

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

@AlexeySachkov
Copy link
Contributor Author

I cancelled the latest pre-commit, because I had only changed comments in a test in the latest commit and previous commit had successfully passed pre-commit. I will proceed with merge

@AlexeySachkov AlexeySachkov merged commit 45c33e9 into intel:sycl Dec 3, 2024
2 of 9 checks passed
@AlexeySachkov AlexeySachkov deleted the private/asachkov/dont-emit-special-pure-virtual-function branch December 3, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants