Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implemented support for non-standard installation of oneAPI compiler by dpctl #481

Merged
merged 1 commit into from
May 27, 2021
Merged

Implemented support for non-standard installation of oneAPI compiler by dpctl #481

merged 1 commit into from
May 27, 2021

Conversation

oleksandr-pavlyk
Copy link
Collaborator

Closes #480

@AndresGuzman-Ballen Please check to see if this changes unblocks the use of DPC++ installation in Python environment.

@oleksandr-pavlyk oleksandr-pavlyk mentioned this pull request May 27, 2021
Closed
@coveralls
Copy link
Collaborator

coveralls commented May 27, 2021 •
edited
Loading

Coverage Status

Coverage remained the same at 60.847% when pulling 5c138ad on issue-480 into 48794f7 on master.

@diptorupd
Copy link
Contributor

@oleksandr-pavlyk I do not like the approach of checking the compiler string. Just checking dpcpp is not a good practice. One can use oneAPI and use -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXXFLAGS=-fsycl and not specify dpcpp. Please see my proposed solution in #480.

@oleksandr-pavlyk
Copy link
Collaborator Author

@oleksandr-pavlyk I do not like the approach of checking the compiler string. Just checking dpcpp is not a good practice. One can use oneAPI and use -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXXFLAGS=-fsycl and not specify dpcpp. Please see my proposed solution in #480.

Ok, I will implement that, but the only difference would be that we would not insist on 2021.2 minimum version requirement.

@oleksandr-pavlyk
Copy link
Collaborator Author

Confirmed that python setup.py develop works with default one API, and with custom-toolchain, using python setup.py develop --sycl-compiler-prefix=${DPCPP_HOME}/llvm/build

@diptorupd
Copy link
Contributor

diptorupd commented May 27, 2021 •
edited
Loading

LGTM. In @AndresGuzman-Ballen's case all he needs to do is pass in --sycl_compiler_path=${BUILD_DIR} in the build.sh. Right?

@oleksandr-pavlyk
Copy link
Collaborator Author

Yes, and he already got this working on Linux, and Windows (a test fails here, but building process works)

diptorupd
diptorupd previously approved these changes May 27, 2021
View reviewed changes
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.

Please squash and merge

@diptorupd
Copy link
Contributor

@oleksandr-pavlyk please update the docs in a follow up or as a separate commit in this PR.

@oleksandr-pavlyk
Improvements to logic for working with custom DPC++ toolchain
5c138ad 
Renamed DPCTL_CSTOME_DPCPP_INSATLL_DIR to DPCTL_DPCPP_HOME_DIR,
and added DPCTL_DPCPP_FROM_ONEAPI and improved their CMake
description strings.

We now always pass DPCTL_DPCPP_HOME_DIR to cmake when building backend.
We rely on user supplied `DPCTL_DPCPP_FROM_ONEAPI` to either require
minimum version of 2021.2.0, or not.

The logic for querying compiler for the version got simplified.
We always execute ``clang++ --version``
@oleksandr-pavlyk oleksandr-pavlyk merged commit 7ea23e5 into master May 27, 2021
@oleksandr-pavlyk oleksandr-pavlyk deleted the issue-480 branch May 27, 2021 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diptorupd diptorupd diptorupd approved these changes

@AndresGuzman-Ballen AndresGuzman-Ballen Awaiting requested review from AndresGuzman-Ballen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Building with oneAPI for non-standard compiler installation
3 participants
@oleksandr-pavlyk @coveralls @diptorupd