Skip to content

[SYCL] Remove <iostream> from header and source files #6469

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 19 commits into from
Jul 29, 2022

Conversation

raaiq1
Copy link
Contributor

@raaiq1 raaiq1 commented Jul 22, 2022

-Removed <iostream> headers from header and source files to best conform to LLVM Coding Standards : https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden .

-Substituted the removed headers with a proxy header which uses external linkage to resolve symbols in <iostream>

@aelovikov-intel
Copy link
Contributor

Can you please add the following tests:

  1. Compile just #include <sycl/sycl.hpp>, ensure that global objects aren't created (i.e. resulting obj file is empty).
  2. The following compiles cleanly
#include <iostream>
#include <sycl/sycl.hpp>
void foo() { std::cout << 42; }
  1. The following compiles cleanly
#include <sycl/sycl.hpp>
#include <iostream>
void foo() { std::cout << 42; }

@raaiq1
Copy link
Contributor Author

raaiq1 commented Jul 22, 2022

Can you please add the following tests:

  1. Compile just #include <sycl/sycl.hpp>, ensure that global objects aren't created (i.e. resulting obj file is empty).
  2. The following compiles cleanly
#include <iostream>
#include <sycl/sycl.hpp>
void foo() { std::cout << 42; }
  1. The following compiles cleanly
#include <sycl/sycl.hpp>
#include <iostream>
void foo() { std::cout << 42; }

I'm sorry, I don't understand what you mean by 'compile cleanly'

@aelovikov-intel
Copy link
Contributor

That clang -fsycl -c foo.cpp finishes without warnings/errors.

Signed-off-by: Rauf, Rana <[email protected]>
Signed-off-by: Rauf, Rana <[email protected]>
@raaiq1 raaiq1 marked this pull request as ready for review July 25, 2022 20:46
@raaiq1 raaiq1 requested review from a team as code owners July 25, 2022 20:46
@raaiq1 raaiq1 requested a review from smaslov-intel July 25, 2022 20:46
@bader bader changed the title [SYCL] Removed <iostream> from header and source files [SYCL] Remove <iostream> from header and source files Jul 26, 2022
@cperkinsintel cperkinsintel self-requested a review July 27, 2022 17:45
Signed-off-by: Rauf, Rana <[email protected]>
cperkinsintel
cperkinsintel previously approved these changes Jul 28, 2022
Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

LGTM!!

@cperkinsintel
Copy link
Contributor

failing CUDA test is known to be flaky. ( #8800 )

@raaiq1
Copy link
Contributor Author

raaiq1 commented Jul 29, 2022

@intel/llvm-gatekeepers slight ping to merge commit. Thanks

@pvchupin pvchupin merged commit b5705a3 into intel:sycl Jul 29, 2022
pvchupin pushed a commit to intel/llvm-test-suite that referenced this pull request Jul 30, 2022
…uld have it included (#1084)

The purpose of this PR is support the changes made in PR intel/llvm#6469 , which gets rid of #include <iostream> from sycl headers and is assumed to be included in the "sycl/sycl.hpp" header for some of the tests present in this repository
@pvchupin
Copy link
Contributor

@raaiq1, please follow up on windows post commit issue: https://github.com/intel/llvm/runs/7587694915?check_suite_focus=true

@aelovikov-intel
Copy link
Contributor

aelovikov-intel commented Aug 1, 2022

I'm working on the build failure as @raaiq1 is on holiday today.

Edit: #6506

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Aug 1, 2022
againull pushed a commit that referenced this pull request Aug 1, 2022
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Aug 1, 2022
Just a workaround in sycl/test/regression/fsycl-host-compiler-win.cpp
while the proper fix is being investigated/prepared.
againull pushed a commit that referenced this pull request Aug 2, 2022
Just a workaround in sycl/test/regression/fsycl-host-compiler-win.cpp
while the proper fix is being investigated/prepared.
@raaiq1 raaiq1 deleted the iostream branch August 2, 2022 17:20
bader pushed a commit that referenced this pull request Sep 27, 2022
…ostream> (#6786)

This is to complement this [PR](#6469)
which removes \<iostream\> headers to comply with LLVM Coding Standards.

Signed-off-by: Rauf, Rana <[email protected]>

Signed-off-by: Rauf, Rana <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Feb 23, 2023
…uld have it included (intel#1084)

The purpose of this PR is support the changes made in PR intel#6469 , which gets rid of #include <iostream> from sycl headers and is assumed to be included in the "sycl/sycl.hpp" header for some of the tests present in this repository
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…uld have it included (intel/llvm-test-suite#1084)

The purpose of this PR is support the changes made in PR intel#6469 , which gets rid of #include <iostream> from sycl headers and is assumed to be included in the "sycl/sycl.hpp" header for some of the tests present in this repository
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.

6 participants