unified-runtime icon indicating copy to clipboard operation
unified-runtime copied to clipboard

[UR] Improve handling of error cases in urProgramLink

Open RossBrunton opened this issue 1 year ago • 1 comments

Note that this change includes a specification change: urProgramLink now requires the output parameter to contain either nullptr or some unspecified binary on failure.

As well as this change, a number of bugs have been fixed:

  • The Level Zero adapter now correctly returns UR_RESULT_ERROR_PROGRAM_LINK_FAILURE when linking fails, rather than UR_RESULT_ERROR_UNKNOWN.
  • A workaround has been added for some OpenCL devices that return CL_INVALID_BINARY rather than CL_LINK_PROGRAM_FAILURE on linker failure.
  • The phProgram handle is wrapped in a loader handle by the loader even if an error would be returned. This is required by Level Zero, which outputs a "dummy" program to store the linker log.

Conformance tests have also been added.

RossBrunton avatar Mar 19 '24 17:03 RossBrunton

Codecov Report

Attention: Patch coverage is 0% with 59 lines in your changes are missing coverage. Please review.

Project coverage is 12.41%. Comparing base (78ef1ca) to head (da9d876). Report is 190 commits behind head on main.

Files Patch % Lines
test/conformance/program/urProgramLink.cpp 0.00% 33 Missing :warning:
source/loader/ur_ldrddi.cpp 0.00% 10 Missing :warning:
source/adapters/null/ur_nullddi.cpp 0.00% 4 Missing :warning:
source/loader/layers/tracing/ur_trcddi.cpp 0.00% 4 Missing :warning:
source/loader/layers/validation/ur_valddi.cpp 0.00% 4 Missing :warning:
source/loader/ur_libapi.cpp 0.00% 4 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1458      +/-   ##
==========================================
- Coverage   14.82%   12.41%   -2.41%     
==========================================
  Files         250      241       -9     
  Lines       36220    36293      +73     
  Branches     4094     4124      +30     
==========================================
- Hits         5369     4506     -863     
- Misses      30800    31783     +983     
+ Partials       51        4      -47     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Mar 20 '24 15:03 codecov-commenter

this has merged conflicts, please fix

kbenzie avatar Jul 08 '24 13:07 kbenzie

this has merged conflicts, please fix

Unfortunately the job in https://github.com/intel/llvm/pull/13085 is failing, so I'll need to look into that first when I get the chance.

RossBrunton avatar Jul 08 '24 14:07 RossBrunton

this has merged conflicts, please fix

Unfortunately the job in intel/llvm#13085 is failing, so I'll need to look into that first when I get the chance.

I don't see how rebasing this PR requires investigation into the intel/llvm failures. Sure, to get it merged they would need to not fail but that shouldn't be a blocker for rebasing this PR.

kbenzie avatar Jul 08 '24 14:07 kbenzie

@oneapi-src/unified-runtime-native-cpu-write please review

kbenzie avatar Jul 09 '24 15:07 kbenzie