Skip to content

Conversation

@nrspruit
Copy link
Contributor

@nrspruit nrspruit commented Oct 25, 2024

  • Enable ZES_ENABLE_SYSMAN=1 by default unless
    UR_L0_ENABLE_SYSMAN_ENV_DEFAULT==0.
  • Allows for Sysman support thru zeInit() without user input.
  • Added UR_L0_ENABLE_ZESINIT_DEFAULT to allow for a customer to enable
    zesInit over sysman env init if the env allows it.
  • NOTE: UR_L0_ENABLE_ZESINIT_DEFAULT=1 requires
    UR_L0_ENABLE_SYSMAN_ENV_DEFAULT==0 due to the env overwriting the
    zesInit support.

@github-actions github-actions bot added the level-zero L0 adapter specific issues label Oct 25, 2024
@nrspruit nrspruit changed the title [L0] Add Debug Env Var to Disable Sysman Default [L0] Enable Sysman Thru Env by default and have zesInit be optional Oct 25, 2024
nrspruit added a commit to nrspruit/llvm that referenced this pull request Oct 25, 2024
@nrspruit nrspruit marked this pull request as ready for review October 25, 2024 16:02
@nrspruit nrspruit requested a review from a team as a code owner October 25, 2024 16:02
@nrspruit nrspruit added the v0.10.x Include in the v0.10.x release label Oct 25, 2024
nrspruit added a commit to nrspruit/llvm that referenced this pull request Oct 25, 2024
nrspruit added a commit to intel/llvm that referenced this pull request Oct 25, 2024
- Enable ZES_ENABLE_SYSMAN=1 by default unless
  UR_L0_ENABLE_SYSMAN_ENV_DEFAULT==0.
- Allows for Sysman support thru zeInit() without user input.
- Added UR_L0_ENABLE_ZESINIT_DEFAULT to allow for a customer to enable
  zesInit over sysman env init if the env allows it.
- NOTE: UR_L0_ENABLE_ZESINIT_DEFAULT=1 requires
  UR_L0_ENABLE_SYSMAN_ENV_DEFAULT==0 due to the env overwriting the
zesInit support.

Signed-off-by: Neil R. Spruit <[email protected]>
Comment on lines +154 to +173
| Environment Variable | Value | Behavior |
|--------------------------------|-------|----------------------------|
| UR_L0_ENABLE_SYSMAN_ENV_DEFAULT| 1 | Enables the default SysMan |
| | | environment initialization |
| | | by setting |
| | | `ZES_ENABLE_SYSMAN` to "1".|
| | 0 | Disables the default SysMan|
| | | environment initialization.|
| | unset | Defaults to 1, enabling the|
| | | SysMan environment |
| | | initialization. |
| UR_L0_ENABLE_ZESINIT_DEFAULT | 1 | Enables the default SysMan |
| | | initialization by loading |
| | | SysMan-related functions |
| | | and calling `zesInit`. |
| | 0 | Disables the default SysMan|
| | | initialization with zesInit|
| | unset | Defaults to 0, disabling |
| | | the SysMan initialization |
| | | thru zesInit. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like these should be described in the env var docs but I don't see any of the other L0 adapter env vars their either.

I think the best place for these would be an L0 reference doc, similar to the CUDA ref doc and HIP ref doc describing all the L0 adapter specific env vars. This could be done in a separate PR, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point and agreed, let me handle that in a separate PR for all the existing UR_L0_ENV vars.

Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

I think I'd preferred a single env variable with three states, but this is also fine.

@pbalcer pbalcer added the ready to merge Added to PR's which are ready to merge label Oct 28, 2024
@pbalcer pbalcer merged commit dbd168c into oneapi-src:main Oct 28, 2024
76 of 77 checks passed
nrspruit added a commit to nrspruit/llvm that referenced this pull request Oct 28, 2024
nrspruit pushed a commit to nrspruit/unified-runtime that referenced this pull request Oct 28, 2024
[L0] Enable Sysman Thru Env by default and have zesInit be optional
againull pushed a commit to intel/llvm that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.10.x Include in the v0.10.x release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants