Skip to content

Replace PROGRAM_INFO_SOURCE with PROGRAM_INFO_IL. #1196

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 10, 2024
Merged

Replace PROGRAM_INFO_SOURCE with PROGRAM_INFO_IL. #1196

merged 6 commits into from
Oct 10, 2024

Conversation

@aarongreig
Copy link
Contributor

@aarongreig aarongreig commented Dec 15, 2023 •
edited
Loading

Also make returns for this consistent across adapter implementations (i.e. actual IL if that's supported, otherwise return UNSUPPORTED_ENUMERATION).

resolves #1138

LLVM testing intel/llvm#12265

@aarongreig aarongreig force-pushed the aaron/replaceProgramInfoSourceWithIL branch from 64dc5d9 to 2621f1d Compare December 28, 2023 15:30
@aarongreig aarongreig marked this pull request as ready for review December 28, 2023 15:30
@aarongreig aarongreig requested review from a team as code owners December 28, 2023 15:30
@aarongreig aarongreig requested a review from hdelan December 28, 2023 15:30
@codecov-commenter
Copy link

codecov-commenter commented Dec 28, 2023 •
edited
Loading

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (40517d2) 15.44% compared to head (8398d0e) 15.39%.
Report is 221 commits behind head on main.

Files Patch % Lines
include/ur_print.hpp 0.00% 2 Missing ⚠️
test/conformance/program/urProgramGetInfo.cpp 0.00% 2 Missing ⚠️

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

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1196      +/-   ##
==========================================
- Coverage   15.44%   15.39%   -0.06%     
==========================================
  Files         239      240       +1     
  Lines       33961    34125     +164     
  Branches     3757     3780      +23     
==========================================
+ Hits         5246     5254       +8     
- Misses      28664    28820     +156     
  Partials       51       51

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

This was referenced Dec 29, 2023
Closed
Merged
fabiomestre
fabiomestre approved these changes Jan 1, 2024
View reviewed changes
Copy link
Contributor

@fabiomestre fabiomestre left a comment

Choose a reason for hiding this comment

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

LGTM

fabiomestre
fabiomestre reviewed Jan 1, 2024
View reviewed changes
source/adapters/cuda/program.cpp
UR_ASSERT(getKernelNames(hProgram), UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;
case UR_PROGRAM_INFO_NUM_KERNELS:
case UR_PROGRAM_INFO_IL:
Copy link
Contributor

@fabiomestre fabiomestre Jan 1, 2024

Choose a reason for hiding this comment

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

I'm wondering, if we wanted to implement this for CUDA, we would have to find way to figure out which binaries are PTX right?

Copy link
Contributor Author

@aarongreig aarongreig Jan 2, 2024

Choose a reason for hiding this comment

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

yeah, so it's doable but we should probably wait for a use case to come up before going to the trouble

@aarongreig aarongreig mentioned this pull request Jan 31, 2024
Closed
@aarongreig aarongreig force-pushed the aaron/replaceProgramInfoSourceWithIL branch 4 times, most recently from 8398d0e to ab6a4e3 Compare February 5, 2024 10:55
@aarongreig aarongreig mentioned this pull request Feb 12, 2024
Merged
@kbenzie kbenzie added conformance Conformance test suite issues. specification Changes or additions to the specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues opencl OpenCL adapter specific issues labels Apr 10, 2024
@aarongreig aarongreig force-pushed the aaron/replaceProgramInfoSourceWithIL branch from ab6a4e3 to 5bf0a91 Compare May 7, 2024 14:22
@github-actions github-actions bot added the native-cpu Native CPU adapter specific issues label May 7, 2024
@aarongreig aarongreig force-pushed the aaron/replaceProgramInfoSourceWithIL branch from 5bf0a91 to d9d819c Compare May 8, 2024 15:44
@aarongreig aarongreig force-pushed the aaron/replaceProgramInfoSourceWithIL branch from d9d819c to d251ddc Compare September 13, 2024 11:16
@aarongreig aarongreig requested a review from a team as a code owner September 13, 2024 11:26
@aarongreig
Replace PROGRAM_INFO_SOURCE with PROGRAM_INFO_IL.
6c98e93 
Also make returns for this consistent across adapter implementations
(i.e. actual IL if that's supported, otherwise return
UNSUPPORTED_ENUMERATION).

resolves oneapi-src#1138
@aarongreig
Verify queried out IL against input IL in CTS test
ace3fd0
@aarongreig
Clarify default return for INFO_IL in spec.
917448c
@aarongreig
Update INFO_SOURCE in l0 adapter -> INFO_IL.
e42415e
@aarongreig
Fix some match files and fix level zero handling of the query.
18c7d27 
Also add simple optional query handling to urProgramGetInfo cts test.
@aarongreig aarongreig force-pushed the aaron/replaceProgramInfoSourceWithIL branch from 8626319 to 18c7d27 Compare September 16, 2024 15:48
@aarongreig
Copy link
Contributor Author

aarongreig commented Sep 18, 2024

@oneapi-src/unified-runtime-native-cpu-write @oneapi-src/unified-runtime-level-zero-write please take a look when you can

@aarongreig
Copy link
Contributor Author

aarongreig commented Sep 24, 2024

ping @oneapi-src/unified-runtime-native-cpu-write @oneapi-src/unified-runtime-level-zero-write

PietroGhg
PietroGhg approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU lgtm, thank you

@aarongreig
Copy link
Contributor Author

aarongreig commented Oct 7, 2024

ping @oneapi-src/unified-runtime-level-zero-write

@aarongreig aarongreig added the ready to merge Added to PR's which are ready to merge label Oct 7, 2024
@aarongreig aarongreig merged commit ed5a20e into oneapi-src:main Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pbalcer pbalcer pbalcer approved these changes

+3 more reviewers

@fabiomestre fabiomestre fabiomestre approved these changes

@PietroGhg PietroGhg PietroGhg approved these changes

@hdelan hdelan hdelan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

conformance Conformance test suite issues. cuda CUDA adapter specific issues hip HIP adapter specific issues level-zero L0 adapter specific issues native-cpu Native CPU adapter specific issues opencl OpenCL adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider adding PROGRAM_INFO_IL

7 participants

@aarongreig @codecov-commenter @fabiomestre @pbalcer @PietroGhg @hdelan @kbenzie