Skip to content

[SYCL] [NFC] Make instance variable in scheduler singleton #2286

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

Conversation

s-kanaev
Copy link
Contributor

This may be needed for testing purposes in unit-tests.

Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
@s-kanaev s-kanaev requested a review from romanovvlad August 10, 2020 06:38
@@ -122,14 +122,35 @@ EventImplPtr Scheduler::addCopyBack(Requirement *Req) {
// The init_priority here causes the constructor for scheduler to run relatively
// early, and therefore the destructor to run relatively late (after anything
// else that has no priority set, or has a priority higher than 2000).
Scheduler Scheduler::instance __attribute__((init_priority(2000)));
std::atomic<Scheduler *> Scheduler::instance(nullptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a more c++ way?

Suggested change
std::atomic<Scheduler *> Scheduler::instance(nullptr);
std::atomic<Scheduler *> Scheduler::instance{nullptr};

Copy link
Contributor

Choose a reason for hiding this comment

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

An even more C++ way?

std::atomic<Scheduler *> Scheduler::instance {};

@alexbatashev
Copy link
Contributor

@s-kanaev isn't it going to produce a memory leak? I don't see any code for Scheduler destruction.

@romanovvlad
Copy link
Contributor

romanovvlad commented Aug 10, 2020

Why not make it to be just a unique_ptr?:
__attribute__((init_priority(2000))) std::unique_ptr<Scheduler> Scheduler::instance(new Scheduler) ;

@bader
Copy link
Contributor

bader commented Oct 14, 2020

@s-kanaev, do you plan to merge this?

@s-kanaev
Copy link
Contributor Author

do you plan to merge this?

I think not.
When there will be a need for this change it will appear in another patch.

@s-kanaev s-kanaev closed this Oct 14, 2020
vladimirlaz added a commit to vladimirlaz/llvm that referenced this pull request Jan 12, 2022
Align installation rules for XPTI libraries with the ones used for
OpenCL ICD loader to avoid installing to lib64 directory.

That causes missing of the libraries during E2E testing:
 - llvm_test_suite_sycl/xpti_buffer* tests are impacted.
vladimirlaz added a commit that referenced this pull request Jan 13, 2022
Align installation rules for XPTI libraries with the ones used for
OpenCL ICD loader to avoid installing to lib64 directory.

That causes missing of the libraries during E2E testing:
 - llvm_test_suite_sycl/xpti_buffer* tests are impacted.
jsji pushed a commit that referenced this pull request Jan 19, 2024
This PR preserves unsigned return type of image read and unsigned texel
type of image write builtins as ZeroExtend image operand in SPIRV.

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@0aab124
Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
[Common] fix parseDisjointPoolConfig
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.

5 participants