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

[HIP][CUDA][Command-Buffer] Fix Coverity issues in HIP/CUDA command-buffer code #1340

Merged
merged 3 commits into from
Mar 10, 2024

Conversation

EwanC
Copy link
Contributor

@EwanC EwanC commented Feb 14, 2024

Address issues in the CUDA adapter code added by #1089 which have been flagged by Coverity.

  • Uninitialized struct member of CUDA_KERNEL_NODE_PARAMS struct
  • copying instead of moving a shared pointer to a node when constructing a command-handle.

As well as issues flagged by coverity in the HIP adapter code added in #1254

  • Initialize NextSyncPoint
  • Move rather than copy node shared-pointers during sync-point registration
  • Don't throw exceptions from a class destructor

DPC++ PR intel/llvm#12723

Copy link
Contributor

@mfrancepillois mfrancepillois left a comment

Choose a reason for hiding this comment

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

LGTM

@EwanC EwanC added the ready to merge Added to PR's which are ready to merge label Feb 15, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 12.51%. Comparing base (78ef1ca) to head (be622e7).
Report is 96 commits behind head on main.

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1340      +/-   ##
==========================================
- Coverage   14.82%   12.51%   -2.32%     
==========================================
  Files         250      239      -11     
  Lines       36220    35949     -271     
  Branches     4094     4076      -18     
==========================================
- Hits         5369     4498     -871     
- Misses      30800    31447     +647     
+ Partials       51        4      -47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EwanC EwanC changed the title [EXP][Command-Buffer] Fix CUDA Coverity issues [HIP][CUDA][Command-Buffer] Fix Coverity issues in HIP/CUDA command-buffer code Feb 28, 2024
Address issues in the CUDA adapter code added by oneapi-src#1089
flagged by Coverity.

* Uninitalized struct member of `CUDA_KERNEL_NODE_PARAMS` struct
* copying instead of moving a shared pointer to a node when
  constructing a command-handle.
* Don't throw from constructor
* Move rather than copy nodes during sync-point registration
* Initalize `NextSyncPoint`
@EwanC EwanC force-pushed the ewan/coverity_cuda_update branch from 7c4a593 to be622e7 Compare March 4, 2024 08:30
EwanC added a commit to reble/llvm that referenced this pull request Mar 4, 2024
Test changes to the UR CUDA adapter in oneapi-src/unified-runtime#1340
that fix Coverity issues in the implementation of
kernel command update in a command-buffer.
@kbenzie kbenzie merged commit e2ee9a4 into oneapi-src:main Mar 10, 2024
52 checks passed
sommerlukas pushed a commit to intel/llvm that referenced this pull request Mar 11, 2024
… implementation (#12723)

Test changes to the UR CUDA/HIP adapters in
oneapi-src/unified-runtime#1340 that fix
Coverity issues in the implementation of kernel command update in a
command-buffer.

---------

Co-authored-by: Kenneth Benzie (Benie) <k.benzie@codeplay.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Added to PR's which are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants