Skip to content

Conversation

alexbatashev
Copy link
Contributor

This patch splits up environment variables into 4 categories, to make it clear, which variables are not supposed to be used in production and only serve for debugging purpose.

@alexbatashev alexbatashev marked this pull request as ready for review September 29, 2021 11:09
@alexbatashev alexbatashev requested a review from bader as a code owner September 29, 2021 11:09
| -------------------- | ------ | ----------- |
| `SYCL_ENABLE_PCI` | Integer | When set to 1, enables obtaining the GPU PCI address when using the Level Zero backend. The default is 0. |
| `SYCL_PI_LEVEL_ZERO_DISABLE_USM_ALLOCATOR` | Any(\*) | Disable USM allocator in Level Zero plugin (each memory request will go directly to Level Zero runtime) |
| `SYCL_PI_LEVEL_ZERO_TRACK_INDIRECT_ACCESS_MEMORY` | Any(\*) | Enable support of the kernels with indirect access and corresponding deferred release of memory allocations in the Level Zero plugin. |
Copy link
Contributor

Choose a reason for hiding this comment

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

@smaslov-intel
Could you please tell if we have tests for this variable(couldn't find such)? If no, probably we need to move it to "debugging" section until we have proper testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

@againull : did you add a test?


**Warning:** the environment variables described below are used for
development and debugging of DPC++ compiler and runtime. Their semantics are
subject to change. Do not rely on these variables in production code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would stop more people from using these vars if we say that they may have bugs that we may refuse to fix. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're not bugs, they're undefined behavior

romanovvlad
romanovvlad previously approved these changes Oct 7, 2021
Copy link
Contributor

@romanovvlad romanovvlad left a comment

Choose a reason for hiding this comment

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

LGTM, except the comment about L0 env var above.


## Debugging variables for DPC++ Runtime

**Warning:** the environment variables described below are used for
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we highlight this warning more? Red text or some warning icon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This blocks color painting on github: github/markup#1440 (but it'll work for rendered docs)
But github supports emojis, so I added one

Comment on lines 131 to 133
**Warning:** the environment variables described below are used for
development and debugging of DPC++ compiler and runtime. Their semantics are
subject to change. Do not rely on these variables in production code.
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@bader bader requested a review from pvchupin October 8, 2021 12:21
@pvchupin pvchupin merged commit d5ba0db into intel:sycl Oct 8, 2021
@alexbatashev alexbatashev deleted the env_vars_refactor branch October 9, 2021 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants