DO NOT MERGE: UR code move test #17040
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the function pointer passed into the function was null, then the wrapper is leaked. This change re-orders things so the wrapper is created later.
ShadowMemory should increment/decrement device handle
Fix Coverity COPY_INSTEAD_OF_MOVE defects.
[DevASAN] Only report warning if passing host ptr to kernel
- Track whether a command buffer was recently synchronized so we can skip unnecessary fence/event waits
Fix memory leak in loader urEventSetCallback
…_SUCCESS_OR_OPTIONAL_QUERY macro. Rework urVirtualMemGranularityGetInfoTest into separate tests. Various tidying up of the urXGetInfo tests for better consistency.
Move urUSMGetMemAlloc/PoolGetInfo success test from a switch to indivual tests
[L0 v2] implement deferred kernel deallocation
Redo various urXGetInfo query tests so the first call uses the ASSERT_SUCCESS_OR_OPTIONAL_QUERY macro
[L0][CMDBUF] Optimize fence/event waits during update
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
Unified clang format
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).
kbenzie
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
kbenzie
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
force-pushed
the
benie/move-unified-runtime-code-test
branch
from
8549a7f to
be1d490
Compare
kbenzie
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
kbenzie
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
kbenzie
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
kbenzie
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
kbenzie
force-pushed
the
benie/move-unified-runtime-code-test
branch
from
8aedf29 to
0538400
Compare
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
force-pushed
the
benie/move-unified-runtime-code-test
branch
from
0538400 to
bafcba3
Compare
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
kbenzie
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
I believe if you re-enable "UR Pre Commit" workflow it will run on this PR without any code changes. Once this PR is merged I'll prepare a first bunch of updates to run UR workflow only if UR changes are detected. |
Sounds good. Thanks. This PR is only for testing, it showed up a number of issues some of which will be merged to UR before I create the official move PR. |
|
Closing this now, #17043 is the official move PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.