Skip to content

Conversation

@jchlanda
Copy link
Contributor

@jchlanda jchlanda commented Dec 6, 2021

Follow up to #5062

This PR ensures that libclc is built and tested against:

  • amdgcn--;amdgcn--amdhsa
  • nvptx64--;nvptx64--nvidiacl

bader
bader previously approved these changes Dec 6, 2021
@bader bader added cuda CUDA back-end hip Issues related to execution on HIP backend. labels Dec 6, 2021
@bader bader added the libclc libclc project related issues label Dec 6, 2021
@alexbatashev
Copy link
Contributor

LGTM, but will we need it after #5087 is merged?

if build_libclc:
llvm_enable_projects += ';libclc'
# CI Default conditionally appends to options, keep it at the bottom of
# args handling
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexbatashev, @vladimirlaz, FYI.
When we originally discussed --ci-defaults option, I thought that these defaults can be overridden by additional options. E.g. --ci-defaults --option-X=val, so that if --ci-defaults sets --option-X, users can replace the default value.
Unfortunately, I don't see this requirement to be documented in the code and it seems to be not true for options modified inside this if-statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bader it’s a good point, we shall fix it in the future

@bader
Copy link
Contributor

bader commented Dec 6, 2021

LGTM, but will we need it after #5087 is merged?

I think so. This patch enables libclc validation only (note, it doesn't require external SDKs).
#5087 adds validation for CUDA and HIP runtime plug-ins, which require corresponding SDKs.

@bader bader merged commit 80cf149 into intel:sycl Dec 6, 2021
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Dec 11, 2021
* upstream/sycl: (725 commits)
  [SYCL] Translate ZE_RESULT_ERROR_INVALID_ARGUMENT error code from L0 RT (intel#5122)
  [SYCL][L0][Plugin] Call ZeCommandQueueCreate on demand (intel#5109)
  [SYCL] Switch to using blocking USM free for OpenCL GPU (intel#4928)
  [CI] Disable pack and upload steps (intel#5119)
  [SYCL] Disable submission of AssertInfoCopier for FPGA (intel#4780)
  [SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead (intel#5076)
  [SYCL] Fix typo in the name of the host-visible pool (intel#5073)
  [SYCL] Only call shutdown when DLL is being unloaded, not when process is terminating (intel#4983)
  [SYCL][CUDA][PI] Fix infinite loop when parallel_for range exceeds INT_MAX (intel#5095)
  [SYCL] Translate out-of-memory error codes from L0 RT (intel#5107)
  [SYCL] Fix a few warnings during build scripts configuration (intel#5082)
  [SYCL] Fix amdgpu openmp test (intel#5103)
  [SYCL] [FPGA] Create experimental headers for FPGA latency control (intel#5066)
  [SYCL][CUDA] Don't enqueue an event wait on same CUDA stream (intel#5099)
  Remove PR disable template (intel#5102)
  [BuildBot]Uplift CPU/FPGAEMU RT version (intel#5078)
  [SYCL] Fix the test to not depend on a specific line. (intel#5092)
  [CI] Provide libclc targets to build and test (intel#5091)
  Fix build of `check-llvm-spirv` target after 8f8001a
  Force opt to use new pass manager in pr52289 test after c34d157
  ...
Comment on lines +115 to +116
if libclc_amd_target_names not in libclc_targets_to_build:
libclc_targets_to_build += libclc_amd_target_names
Copy link
Contributor

@bader bader Mar 3, 2023

Choose a reason for hiding this comment

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

I think to build libclc for AMD target, LLVM's AMDGPU target back-end must be built. @jchlanda, am I right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my guess as well, but I've just tried and LIBCLC_TARGETS_TO_BUILD:STRING=;amdgcn--;amdgcn--amdhsa builds fine with only LLVM_TARGETS_TO_BUILD:STRING=X86. Saying that I'm worried that if in future libclc uses more AMD specific code (for example builtins) it will not be possible. Enforcing AMDGPU backend seems like a wise thing to do moving forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda CUDA back-end hip Issues related to execution on HIP backend. libclc libclc project related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants