Skip to content

Conversation

againull
Copy link
Contributor

@againull againull commented Aug 22, 2025

Add --offload option to configure.py which allows to build UR offload adapter.
Because offload API is under active development in llvm-project, we have to fetch specific version of LLVM and build offload runtime (and dependent openmp) using those sources.
Necessary to mention that offload/openmp runtimes are built in standalone mode using main compiler build, i.e. we don't build llvm twice.

@againull againull requested review from a team as code owners August 22, 2025 20:49
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

configure.py LGTM

UPDATE_COMMAND ""
)
# Build OpenMP runtime (required dependency for offload's libomptarget) from the cloned source
ExternalProject_Add(openmp_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 are we not just using LLVM_ENABLE_RUNTIMES="offload;openmp" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We’re not using LLVM_ENABLE_RUNTIMES="offload;openmp" here because that mode requires configuring from the top-level llvm directory. In that flow, clang/llvm would be built as part of the runtimes build, which means clang/llvm ends up being built twice.
Instead, I build OpenMP and offload in standalone mode, reusing the existing clang/llvm build from intel/llvm.

Also, in the latest commit I improved this further: rather than cloning llvm-project again, I now create a git worktree from the already cloned intel/llvm repository, pulling in only the directories needed for the runtime build.

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked into this a while ago and figured out how to build openmp without a dependency on clang. It required a few patches, which I should really figure out who to harass about.

rather than cloning llvm-project again, I now create a git worktree from the already cloned intel/llvm repository

What if the runtimes are using a newer version of llvm-project that hasn't been pulled down into intel/llvm yet?

Copy link
Contributor Author

@againull againull Aug 26, 2025

Choose a reason for hiding this comment

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

Sorry if the statement was confusing, worktree is created from fresh enough hash specified in LLVM_PROJECT_TAG, i.e. assumption is that llvm-project remote and its main branch is visible in the cloned intel/llvm repo, i.e. just trying to avoid re-cloning what's already available.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue I'm thinking of is with, for example the commit ee5022d63549032235247269903f6ccf7e70bd02 (which doesn't affect liboffload, but pretend it does). That commit exists on llvm/llvm-project, but not on intel/llvm at the time of writing (at the time of writing).

Although, there's a commit from 13 minutes ago on the main branch, so it might get mirrored frequently enough that that's not an issue. How often does intel/llvm pull down new llvm/llvm-project changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pulldowns approximately once per week/2weeks.
How dependent are openmp and offload from other components in llvm? I wonder why an option of standalone building mode even exists if those runtimes have to be built in-tree with entire llvm project of exact same version. If you believe openmp and offload are tightly coupled with the rest of llvm and has to be of exactly same version, then I can switch to what you propose - using LLVM_ENABLE_RUNTIMES="offload;openmp" i,e, re-building clang/llvm entirely from llvm-project.
Or we can try the current approach first for some time and switch to LLVM_ENABLE_RUNTIMES="offload;openmp" if there will be cases when build breaks because of such dependency.

@againull againull requested a review from a team as a code owner August 26, 2025 04:57
@againull againull requested a review from uditagarwal97 August 26, 2025 04:57
Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the sparse checkout.

set(UR_OFFLOAD_INSTALL_DIR ${CMAKE_BINARY_DIR}/offload-install)
set(UR_OFFLOAD_INCLUDE_DIR ${UR_OFFLOAD_INSTALL_DIR}/include)

execute_process(COMMAND git -C "${CMAKE_SOURCE_DIR}" worktree prune)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I tried this, but the helper’s shallow checkout approach seems to work only with tags, and it becomes slow when used with commit hashes. So I am leaving this part as is for now if that's ok.

Copy link
Contributor

@lplewa lplewa left a comment

Choose a reason for hiding this comment

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

looks correct

$ ./sycl-ls 
[level_zero:gpu][level_zero:0] Intel(R) oneAPI Unified Runtime over Level-Zero, Intel(R) Graphics 12.70.4 [1.6.33944+13]
[offload:gpu][offload:0] CUDA, NVIDIA GeForce GTX 1060 6GB  [12090]
[opencl:gpu][opencl:0] Intel(R) OpenCL Graphics, Intel(R) Graphics OpenCL 3.0 NEO  [25.22.33944]

@againull
Copy link
Contributor Author

againull commented Sep 2, 2025

Failures are unrelated.

********************
Unexpectedly Passed Tests (2):
  SYCL :: Matrix/joint_matrix_bf16_fill_k_cache_arg_dim.cpp
  SYCL :: Matrix/joint_matrix_bf16_fill_k_cache_runtime_dim.cpp

is supposed to be fixed by #19933

  ********************
Failed Tests (2):
  Unified Runtime Conformance :: exp_usm_context_memcpy//exp_usm_context_memcpy-test/urUSMContextMemcpyExpTestDevice/Success/UR_BACKEND_CUDA__NVIDIA_CUDA_BACKEND__NVIDIA_A2_ID0ID__e___6_______W______
  Unified Runtime Conformance :: exp_usm_context_memcpy//exp_usm_context_memcpy-test/urUSMContextMemcpyExpTestDevice/Success/UR_BACKEND_CUDA__NVIDIA_CUDA_BACKEND__NVIDIA_A2_ID1ID_______z__3_Gb_______

#19688

CMake Error: install(EXPORT "unified-runtime-targets" ...) includes target "xptifw" which requires target "emhash" that is not in any export set.
#19944

SYCL :: Assert/assert_in_multiple_tus.cpp
#19951

@againull againull merged commit bb6dcfb into intel:sycl Sep 2, 2025
32 of 36 checks passed
@againull againull deleted the ur_offload_build_option branch September 29, 2025 21:29
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.

6 participants