Skip to content

Change static variables to static pointers #29

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

Merged

Conversation

PokhodenkoSA
Copy link
Contributor

On Windows static variables construction and destruction cause crashes. I think it is because it should be called in specific order. With static variables the order is undefined.

Now the static variables are constructed at the first access to this variables. The first access happens after all necessary initializations.
Also I think destruction should happen in correct order. As we have no explicit destruction function it is hard to control where static global variables will be deleted. So the variables are changed to pointers and we do not call destruction for them at all. As they should exist to the end of the application it is ok to not destruct them and allow OS to clean its memory.

Copy link
Contributor

@diptorupd diptorupd left a comment

Choose a reason for hiding this comment

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

Except minor formatting nitpick, looks good. Thanks for fixing it (once again).

static std::vector<cl::sycl::queue> cpu_queues;
static std::vector<cl::sycl::queue> gpu_queues;
static thread_local std::vector<cl::sycl::queue> active_queues;
static std::vector<cl::sycl::queue>& cpu_queues_()
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick. Can you please add a space before the open paren? I usually ffollow the convention of adding a space before the open paren in function declaration. In the long term let us create a common coding style and formatter for VS Code so that everyone can be consistent.

@PokhodenkoSA PokhodenkoSA merged commit 78aba38 into IntelPython:master Sep 15, 2020
diptorupd added a commit to diptorupd/dpctl that referenced this pull request Sep 15, 2020
diptorupd added a commit to diptorupd/dpctl that referenced this pull request Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working windows Applies to Windows OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants