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.

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

[SYCL] Properly deploy SYCL components #509

Closed

Conversation

alexbatashev
Copy link
Contributor

Add deploy-sycl-toolchain target that installs SYCL compiler and its dependencies to CMAKE_INSTALL_PREFIX location.

Usage:

  1. Configure CMake project:
cmake -DCMAKE_BUILD_TYPE=Release \
-DLLVM_EXTERNAL_PROJECTS="llvm-spirv;sycl" \
-DLLVM_EXTERNAL_SYCL_SOURCE_DIR=$SYCL_HOME/sycl \
-DLLVM_EXTERNAL_LLVM_SPIRV_SOURCE_DIR=$SYCL_HOME/llvm-spirv \
-DLLVM_ENABLE_PROJECTS="clang;llvm-spirv;sycl" \
-DCMAKE_INSTALL_PREFIX=/usr/local \
-DSYCL_DEPLOY_OPENCL=ON \
$SYCL_HOME/llvm
  1. Build/test compiler as needed
  2. Install SYCL compiler:
cmake --build /path/to/build/ --target deploy-sycl-toolchain

SYCL compiler with all its dependencies will be installed to /usr/local.

New CMake options:

SYCL_DEPLOY_OPENCL - installs OpenCL to the same location as SYCL. Defaults to OFF.

Signed-off-by: Alexander Batashev <[email protected]>

Add deploy-sycl-toolchain target that installs SYCL compiler and its
dependencies to CMAKE_INSTALL_PREFIX location.

Usage:
1. Configure CMake project:
```
cmake -DCMAKE_BUILD_TYPE=Release \
-DLLVM_EXTERNAL_PROJECTS="llvm-spirv;sycl" \
-DLLVM_EXTERNAL_SYCL_SOURCE_DIR=$SYCL_HOME/sycl \
-DLLVM_EXTERNAL_LLVM_SPIRV_SOURCE_DIR=$SYCL_HOME/llvm-spirv \
-DLLVM_ENABLE_PROJECTS="clang;llvm-spirv;sycl" \
-DCMAKE_INSTALL_PREFIX=/usr/local \
-DSYCL_DEPLOY_OPENCL=ON \
$SYCL_HOME/llvm
```

2. Build/test compiler as needed
3. Install SYCL compiler:
```
cmake --build /path/to/build/ --target deploy-sycl-toolchain
```
SYCL compiler with all its dependencies will be installed to /usr/local.

New CMake options:

`SYCL_DEPLOY_OPENCL` - installs OpenCL to the same location as SYCL.
Defaults to `OFF`.

Signed-off-by: Alexander Batashev <[email protected]>
ARGS -E copy ${CMAKE_BINARY_DIR}/bin/llvm-lit${LLVM_LIT_EXT}
${CMAKE_INSTALL_PREFIX}/bin/llvm-lit${LLVM_LIT_EXT})

# For some reason clang-cl is being created on Linux. Remove it so users
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest fix this on clang side rather than add such a post build adjustment.
Is it reproduced consistently? Do you have steps to reproduce?

Copy link
Contributor Author

@alexbatashev alexbatashev Aug 15, 2019

Choose a reason for hiding this comment

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

@valerydmit when I build sycl-toolchain target, I get clang-cl in bin directory. And at the time I'm not sure it is a bug in clang. There are tests that call clang-cl, and they seem to run and pass on Linux. On the other hand, all clang-cl can do on Linux is show help and crash, complaining about missing MSVC. So, it worths double checking.


set(SYCL_BUNDLE_DEPS ${SYCL_TOOLCHAIN_DEPS} opencl clang-resource-headers)

foreach(comp ${SYCL_BUNDLE_DEPS})
Copy link
Contributor

Choose a reason for hiding this comment

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

This code relies on assumption that target names do match names of associated components.

# Configure OpenCL header and ICD loader
include_directories(AFTER "${sycl_inc_dir}" "${OpenCL_INCLUDE_DIRS}")
link_libraries(${OpenCL_LIBRARIES})

# SYCL runtime library
add_subdirectory(source)

# SYCL toolchain builds all components: compiler, libraries, headers, etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to have separate variable per component.

llvm-spirv
llvm-link
llvm-objcopy
opt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these tools are not part of the SYCL toolchain and actually dependency of check-sycl target. If possible, please, fix with this patch, if not, I'm okay to resolve it separately.

add_custom_command(TARGET deploy-sycl-toolchain PRE_BUILD
COMMAND ${CMAKE_COMMAND}
ARGS -E copy ${CMAKE_BINARY_DIR}/bin/llvm-lit${LLVM_LIT_EXT}
${CMAKE_INSTALL_PREFIX}/bin/llvm-lit${LLVM_LIT_EXT})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why llvm-lit is deployed with the SYCL toolchain?

endif()

if (SYCL_DEPLOY_OPENCL)
install(TARGETS OpenCL EXPORT SYCL
Copy link
Contributor

Choose a reason for hiding this comment

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

Deploying OpenCL to the same directory as SYCL is probably bad idea. It complicates the configuration.
Please, add TODO to make OpenCL deploy path configurable.

add_custom_command(TARGET deploy-sycl-toolchain POST_BUILD
COMMAND ${CMAKE_COMMAND}
ARGS -E rename ${CMAKE_INSTALL_PREFIX}/lib/libOpenCL.so.1.0.0
${CMAKE_INSTALL_PREFIX}/lib/libOpenCL.so)
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel something is wrong with this WA.
I suggest removing it...
I think we should start with deploying only latest Khronos ICD, if it's downloaded and build from sources by this script.
I'm not sure there is a reason to deploy anything else.

@valerydmit
Copy link
Contributor

Replaced with #567.

@valerydmit valerydmit closed this Aug 28, 2019
@alexbatashev alexbatashev deleted the private/abatashe/deploy branch December 19, 2019 15:13
vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request May 19, 2020
Add --spirv-fp-contract={on|off|fast} option

In SPIR-V, floating point contraction (fused multiply add) can only be
controlled per entry point using ContractionOFF execution mode: it can
either allow any floating point operation to contract across a call
graph (with an entry point as a root), or disallow contraction for a
call graph.

SPIR-V translator currently have an algorithm to determine if
ContractionOFF should be set, but this algorithm is supposed to be
very pessimistic. A single non-contracted fmul + fadd or a single
undefined function must force ContractionOFF for the entire call
graph. This behavior is used by default, or when --spirv-fp-contract=on is
set.

Two more modes are added:
    - `off' disables contraction for all entry points
    - `fast' unconditionally enables contraction for all entry points
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request May 20, 2020
* sycl: (65 commits)
  [SYCL] Host task implementation (intel#1471)
  [SYCL] Update getting dependencies documentation (intel#1699)
  [SYCL] Fix types and transparent functors recognition in reduction (intel#1709)
  [SYCL][Doc] Get started guide clean-up (intel#1697)
  Add --spirv-fp-contract={on|off|fast} option (intel#509)
  [SYCL][Doc] Fix tbb target path in Get Started Guide. (intel#1695)
  [SYCL] Add support for kernel name types templated using enums. (intel#1675)
  [Driver][SYCL] Make -std=c++17 the default for DPC++ (intel#1662)
  AllocaInst should store Align instead of MaybeAlign.
  [X86] Replace selectScalarSSELoad ComplexPattern with PatFrags to handle the 3 types of loads we currently match.
  Harden IR and bitcode parsers against infinite size types.
  Revert "[nfc] test commit"
  [nfc] test commit
  Expose IRGen API to add the default IR attributes to a function definition.
  The release notes for ObjCBreakBeforeNestedBlockParam was placed between the release note for IndentCaseBlocks and its example code
  [VectorCombine] forward walk through instructions to improve chaining of transforms
  [PhaseOrdering] add vector reduction tests; NFC
  [InstCombine] Clean up alignment handling (NFC)
  [ARM] Patterns for VQSHRN
  [VectorCombine] add reduction-like patterns; NFC
  ...
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.

3 participants