Skip to content

[SYCL] Finally fix iostream_proxy.hpp #6517

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
merged 1 commit into from
Aug 3, 2022
Merged

Conversation

aelovikov-intel
Copy link
Contributor

@aelovikov-intel aelovikov-intel commented Aug 2, 2022

When _MT and _DLL are both defined, the user has passed /MD or /MDd and
is dynamically linking in the runtimes. Because std::cout etc are data,
they need to be marked dllimport in that case or else the linker cannot
find them.

@aelovikov-intel
Copy link
Contributor Author

Many thanks to @AaronBallman for helping with this!

Copy link
Contributor

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

FWIW, these changes LGTM.

For reference, when _MT and _DLL are both defined, the user has passed /MD or /MDd and is dynamically linking in the runtimes. Because std::cout etc are data, they need to be marked dllimport in that case or else the linker cannot find them.

@pvchupin
Copy link
Contributor

pvchupin commented Aug 2, 2022

when _MT and _DLL are both defined, the user has passed /MD or /MDd and is dynamically linking in the runtimes. Because std::cout etc are data, they need to be marked dllimport in that case or else the linker cannot find them.

This should be in PR description I think.

@aelovikov-intel
Copy link
Contributor Author

I believe

Failed Tests (2):
  SYCL :: Assert/assert_in_simultaneously_multiple_tus.cpp
  SYCL :: Assert/assert_in_simultaneously_multiple_tus_one_ndebug.cpp

are known and there was/were issue(s) submitted for that. @intel/llvm-gatekeepers , ready to merge?

@pvchupin
Copy link
Contributor

pvchupin commented Aug 3, 2022

I've restarted testing yesterday to check if it's really flaky and it didn't help. Pushed one more restart today... just for curiosity...
I don't think fail is related to the change because it's NFC on Linux it seems.

#6463 looks related.
There is also #6520 for HIP.

@aelovikov-intel
Copy link
Contributor Author

Failing test is being disabled at intel/llvm-test-suite#1127.

@pvchupin pvchupin merged commit 62c36e9 into intel:sycl Aug 3, 2022
raaiq1 pushed a commit to raaiq1/llvm that referenced this pull request Aug 4, 2022
When _MT and _DLL are both defined, the user has passed /MD or /MDd and
is dynamically linking in the runtimes. Because std::cout etc are data,
they need to be marked dllimport in that case or else the linker cannot
find them.
@aelovikov-intel aelovikov-intel deleted the io_fix branch November 8, 2022 20:53
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.

4 participants