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

Move Unified Runtime code into intel/llvm #17043

Merged
merged 5,520 commits into from
Feb 18, 2025

Conversation

kbenzie
Copy link
Contributor

@kbenzie kbenzie commented Feb 18, 2025

This PR pulls in the entire git history of the https://github.com/oneapi-src/unified-runtime project.

The following fixup commits are also included:

  • bbc1f73 [UR] Cleanup GitHub directory
  • 8f0d7e3 [UR] Ensure UNIFIED_RUNTIME_SOURCE_DIR is set
  • eed4250 [UR] Ensure UMF options are set
  • 422551e [LIBDEVICE][UR] Ensure UR_SANITIZER_INCLUDE_DIR is set

dyniols and others added 30 commits January 17, 2025 11:28
Until now we've had multiple styles throught the project. This doesn't
buy us anything for contributors as each source file might be different
- totally negating the point of having a clang-format style in the first
place - indeed I started debugging clang-format since it was apparently
not formatting my adapter code as 4 spaces after my editor had loaded
the editconfig file specifying 4 spaces. I wasted an hour on that!

Here we codify everything as LLVM style, which seems to be used by the
majority of the project.

We also bump the clang-format version requirement to v18.1 to align with
dpc++.
How comments are laid out can have a significant effect on readability.
If we're to have 80column headers, then placing documentation comments
before a declaration line - rather than on the end of the same line -
can enable clang format to generate more readable headers without a
great change in the overall line length..

This might use more or less vertical space than the existing version,
since lines aren't off the right hand side. As an example:

```cpp
urLoaderConfigGetInfo(
    ur_loader_config_handle_t hLoaderConfig, ///< [in] handle of the loader config object
    ur_loader_config_info_t propName,        ///< [in] type of the info to retrieve
    size_t propSize,                         ///< [in] the number of bytes pointed to by pPropValue.
    void *pPropValue,                        ///< [out][optional][typename(propName, propSize)] array of bytes holding
                                             ///< the info.
                                             ///< If propSize is not equal to or greater than the real number of bytes
                                             ///< needed to return the info
                                             ///< then the ::UR_RESULT_ERROR_INVALID_SIZE error is returned and
                                             ///< pPropValue is not used.
    size_t *pPropSizeRet                     ///< [out][optional] pointer to the actual size in bytes of the queried propName.
);
```
->

```cpp
urLoaderConfigGetInfo(
  /// [in] handle of the loader config object
  ur_loader_config_handle_t hLoaderConfig,
  /// [in] type of the info to retrieve
  ur_loader_config_info_t propName,
  /// [in] the number of bytes pointed to by pPropValue.
  size_t propSize,
  /// [out][optional][typename(propName, propSize)] array of bytes holding the
  /// info. If propSize is not equal to or greater than the real number of bytes
  /// needed to return the info then the ::UR_RESULT_ERROR_INVALID_SIZE error is
  /// returned and pPropValue is not used.
  void *pPropValue,
  /// [out][optional] pointer to the actual size in bytes of the queried propName.
  size_t *pPropSizeRet
);
```
The comment and declaration for pPropValue takes fewer lines here, but
the shorter comments occupy more space.

Here is another example, where certain sequences can end up the same
length, but with a slightly more consistent appearance : enumerations
where the enumerator itself is long, and explicit values for the
enumerator declaration:

```cpp
    UR_FUNCTION_CONTEXT_SET_EXTENDED_DELETER =
      7,                      ///< Enumerator for ::urContextSetExtendedDeleter
```
->
```cpp
  /// Enumerator for ::urContextSetExtendedDeleter
  UR_FUNCTION_CONTEXT_SET_EXTENDED_DELETER = 7,
```

```
The total line length change for those files touched by the generator:
before:
```
  12411 include/ur_api.h
   1832 include/ur_ddi.h
   1564 scripts/templates/helper.py
   6818 source/adapters/null/ur_nullddi.cpp
   7650 source/loader/layers/tracing/ur_trcddi.cpp
  10658 source/loader/layers/validation/ur_valddi.cpp
   9173 source/loader/ur_ldrddi.cpp
   8706 source/loader/ur_libapi.cpp
   7390 source/ur_api.cpp
  66202 total
```
After:
```
  13051 include/ur_api.h
   1832 include/ur_ddi.h
   1564 scripts/templates/helper.py
   6986 source/adapters/null/ur_nullddi.cpp
   7671 source/loader/layers/tracing/ur_trcddi.cpp
  10153 source/loader/layers/validation/ur_valddi.cpp
   8986 source/loader/ur_ldrddi.cpp
   8770 source/loader/ur_libapi.cpp
   7637 source/ur_api.cpp
  66650 total
```

That's a less than 1% line length increase for generated code, so I
think this is a win.
clang-format and generation rules were updated. This is simply a run of
the generator on that code.

If you need to rebase as a result of the churn, all that should be
needed is to run the generate target on "theirs".
The comments are now more left-aligned leaving more space for prose.
The SPDX-license header must be on its own line according to the spec
add building compute-runtime UMD in benchmarks jobs
The check is already done in validation layer
to match SYCL expectations. When pHost is set in buffer
properties, urEnqueueMemBufferMap should map memory to
that pointer (instead of creating a new allocation).
event should always be signaled (if provided by the user).
as they can be allocated by the user using L0 API directly
(and then UMF will not know about those pointers and umfFree
will do nothing).
To support both legacy and v2 adapter in SYCL tests
@github-actions github-actions bot mentioned this pull request Feb 26, 2025
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.

None yet