Skip to content

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

Merged
oleksandr-pavlyk merged 1 commit intomasterfrom
issue-480
May 27, 2021
Merged

Implemented support for non-standard installation of oneAPI compiler by dpctl#481
oleksandr-pavlyk merged 1 commit intomasterfrom
issue-480

Conversation

@oleksandr-pavlyk
Copy link
Copy Markdown
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented May 27, 2021

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
Copy Markdown
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
Copy Markdown
Contributor

diptorupd commented May 27, 2021

@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
Copy Markdown
Contributor Author

oleksandr-pavlyk commented May 27, 2021

@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
Copy Markdown
Contributor Author

oleksandr-pavlyk commented May 27, 2021

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
diptorupd reviewed May 27, 2021
View reviewed changes
Comment thread dpctl-capi/CMakeLists.txt Outdated
diptorupd
diptorupd reviewed May 27, 2021
View reviewed changes
Comment thread dpctl-capi/cmake/modules/FindIntelSycl.cmake Outdated
@diptorupd
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

oleksandr-pavlyk commented May 27, 2021

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
Copy Markdown
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
Copy Markdown
Contributor

diptorupd commented May 27, 2021

@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

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

1 more reviewer

@diptorupd diptorupd diptorupd approved these changes

Reviewers whose approvals may not affect merge requirements

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