Skip to content

[CI] Install cm-compiler in drivers image #5128

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 1 commit into from
Dec 15, 2021
Merged

Conversation

alexbatashev
Copy link
Contributor

At least one test in llvm-test-suite uses C for metal. Deploy required components to run it.

@alexbatashev alexbatashev requested a review from bader as a code owner December 12, 2021 11:22
RUN python3 /get_release.py intel/intel-graphics-compiler $igc_tag \
| grep ".*deb" \
| wget -qi - && \
python3 /get_release.py intel/compute-runtime $compute_runtime_tag \
| grep -E ".*((deb)|(sum))" \
| wget -qi - && \
sha256sum -c *.sum &&\
python3 /get_release.py intel/cm-compiler $cm_tag \
Copy link
Contributor

Choose a reason for hiding this comment

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

https://intel.github.io/llvm-docs/GetStartedGuide.html - I don't see cm-compiler in the list of prerequisites in our documentation.
@dongkyunahn-intel, @kbobrovs, is cm-compiler required? If so, please, update the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that cm-compiler might be needed to test all DPC++ features, but tests should not fail if it's not installed on the system.
LIT config must have corresponding feature and skip the test if dependencies are not installed. The same way it's done for OpenCL runtime in https://github.com/intel/llvm-test-suite/blob/74e49b34513f3a24c939be3b8aada17aa83b5d53/SYCL/OnlineCompiler/online_compiler_OpenCL.cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, added a new feature to llvm-test-suite here intel/llvm-test-suite#611

Copy link
Contributor

Choose a reason for hiding this comment

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

@bader, the only DPC++ feature CM compiler is required for is online compilation from CM source. CM compilation should be available in driver/ocloc. Maybe something changed since this feature was committed.

  -options <options>            Optional OpenCL C compilation options
                                as defined by OpenCL specification.
                                Special options for Vector Compute:
                                -vc-codegen <vc options> compile from SPIRV
                                -cmc <cm-options> compile from CM sources

Copy link
Contributor

@v-klochkov v-klochkov Dec 14, 2021

Choose a reason for hiding this comment

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

@v-klochkov, can you please comment?

ocloc calls corresponding CM dynamic library if -cmc is passed. So, CM compiler is needed for that LIT test verifying online_compiler feature.
I also agree that the test should not fail if CM is not installed and the patch intel/llvm-test-suite#611 is a good move. I added my approval there as well - it is ready to be merged now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bader, I had an impression that driver + ocloc encompasses needed headers and compiler itself and that there is no dependence on stand-alone CM compiler. @v-klochkov - is this not correct?

Copy link
Contributor

@v-klochkov v-klochkov Dec 14, 2021

Choose a reason for hiding this comment

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

@bader, I had an impression that driver + ocloc encompasses needed headers and compiler itself and that there is no dependence on stand-alone CM compiler. @v-klochkov - is this not correct?

as soon as you give -cmc to libocloc, it starts requiring another lib* cm* library doing the compilation of CM source.

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like driver+ocloc is not enough and there is a dependency on https://github.com/intel/cm-compiler to compile CM sources.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. I checked recent distros and I don't see the libcm* there anymore.

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

@kbobrovs, @dongkyunahn-intel, ping. Any thoughts on this change?

@bader bader merged commit 510b243 into intel:sycl Dec 15, 2021
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Dec 15, 2021
* upstream/sycl: (5961 commits)
  [SYCL] Implement discard_events extension (intel#5026)
  [SYCL][NFC] Fix unused parameter warning in piQueueFlush (intel#5139)
  [SYCL][XPTI] Fix static analysis tool warnings (intel#5040)
  [CI] Switch post-commit jobs to self-hosted runners (intel#5147)
  [SYCL] Fix support for classes implicitly converted from items in parallel_for (intel#5118)
  [SYCL][HIP] Fix platform query in USM alloc info (intel#5140)
  [Docker] Add workarounds for two SYCL issues (intel#5143)
  [CI] Install cm-compiler in drivers image (intel#5128)
  [ESIMD] Add support for an arbitrary number of elements to simd::copy_from/to (intel#5135)
  [SYCL] Add number HW threads per EU query (intel#4901)
  [CI] Refactor workflow files (intel#5134)
  [CI] Enable HIP and CUDA plugins in GitHub Actions builds (intel#5087)
  [SYCL] Implement queue flushing (intel#5052)
  Disable issue labeler in LLVM forks
  Modify translation for disable_loop_pipelining metadata
  Add SPIR-V friendly translation for OpLoad and OpStore
  Fix return type postfix for SPIR-V Friendly IR
  Restrict special handling of sampler OpVariable only to UniformConstant
  Add lowering for llvm.bswap intrinsic
  Fix translation of OpVariable with OpSamplerType
  ...
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.

4 participants