Skip to content

[SYCL][UR][L0] Deprecate SYCL_ENABLE_PCI #10681

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

Merged
merged 4 commits into from
Aug 10, 2023
Merged

[SYCL][UR][L0] Deprecate SYCL_ENABLE_PCI #10681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d17852b
[SYCL][UR][L0] Deprecate SYCL_ENABLE_PCI
Aug 3, 2023
8f7ae2f
Add deprecate note for SYCL_ENABLE_PCI
Aug 3, 2023
a78ab04
Add warning on deprecated variable
Aug 7, 2023
c64c5b9
Address comment on documentation
Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sycl/doc/EnvironmentVariables.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Note that conflicting configuration tuples in the same list will favor the last

| Environment variable | Values | Description |
| -------------------- | ------ | ----------- |
| `SYCL_ENABLE_PCI` | Integer | When set to 1, enables obtaining the GPU PCI address when using the Level Zero backend. The default is 0. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is described not in experimental env vars section, I think that we keep it for now, say it is deprecated, and is ignored, and will be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool, changing @smaslov-intel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smaslov-intel please check latest commit, 5937848, to confirm statement added is correct.

| `SYCL_ENABLE_PCI` (Deprecated) | Integer | When set to 1, enables obtaining the GPU PCI address when using the Level Zero backend. The default is 1. This option is kept for compatibility reasons and is immediately deprecated. |
| `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. |

Expand Down
6 changes: 5 additions & 1 deletion sycl/plugins/unified_runtime/ur/adapters/level_zero/common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ ze_structure_type_t getZeStructureType<ze_memory_allocation_properties_t>() {
return ZE_STRUCTURE_TYPE_MEMORY_ALLOCATION_PROPERTIES;
}

template <> zes_structure_type_t getZesStructureType<zes_pci_properties_t>() {
template <> ze_structure_type_t getZeStructureType<ze_pci_ext_properties_t>() {
return ZE_STRUCTURE_TYPE_PCI_EXT_PROPERTIES;
}

template <> zes_structure_type_t getZesStructureType<ze_pci_address_ext_t>() {
return ZES_STRUCTURE_TYPE_PCI_PROPERTIES;
}

Expand Down
10 changes: 4 additions & 6 deletions sycl/plugins/unified_runtime/ur/adapters/level_zero/device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,12 +612,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(
case UR_DEVICE_INFO_DEVICE_ID:
return ReturnValue(uint32_t{Device->ZeDeviceProperties->deviceId});
case UR_DEVICE_INFO_PCI_ADDRESS: {
if (getenv("ZES_ENABLE_SYSMAN") == nullptr) {
urPrint("Set SYCL_ENABLE_PCI=1 to obtain PCI data.\n");
return UR_RESULT_ERROR_INVALID_VALUE;
}
ZesStruct<zes_pci_properties_t> ZeDevicePciProperties;
ZE2UR_CALL(zesDevicePciGetProperties, (ZeDevice, &ZeDevicePciProperties));
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need SYSMAN for anything? perhaps some header includes can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smaslov-intel : yes, still for free memory.

ze_pci_address_ext_t PciAddr{};
ZeStruct<ze_pci_ext_properties_t> ZeDevicePciProperties;
ZeDevicePciProperties.address = PciAddr;
ZE2UR_CALL(zeDevicePciGetPropertiesExt, (ZeDevice, &ZeDevicePciProperties));
constexpr size_t AddressBufferSize = 13;
char AddressBuffer[AddressBufferSize];
std::snprintf(AddressBuffer, AddressBufferSize, "%04x:%02x:%02x.%01x",
Expand Down
4 changes: 1 addition & 3 deletions sycl/plugins/unified_runtime/ur/adapters/level_zero/platform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urPlatformGet(
setEnvVar("ZE_ENABLE_PARAMETER_VALIDATION", "1");
}

// Enable SYSMAN support for obtaining the PCI address
// and maximum memory bandwidth.
if (getenv("SYCL_ENABLE_PCI") != nullptr) {
setEnvVar("ZES_ENABLE_SYSMAN", "1");
urPrint("WARNING: SYCL_ENABLE_PCI is deprecated and no longer needed.\n");
}

// TODO: We can still safely recover if something goes wrong during the init.
Expand Down
3 changes: 0 additions & 3 deletions sycl/test-e2e/Basic/intel-ext-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ using namespace sycl;
#endif

int main(int argc, char **argv) {
// Must be enabled at the beginning of the application
// to obtain the PCI address
setenv("SYCL_ENABLE_PCI", "1", 0);

int pltCount = 1;
for (const auto &plt : platform::get_platforms()) {
Expand Down
4 changes: 0 additions & 4 deletions sycl/test-e2e/DeprecatedFeatures/deprecated_intel_ext_device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ using namespace sycl;
#endif

int main(int argc, char **argv) {
// Must be enabled at the beginning of the application
// to obtain the PCI address
setenv("SYCL_ENABLE_PCI", "1", 0);

int pltCount = 1;
for (const auto &plt : platform::get_platforms()) {
int devCount = 1;
Expand Down
4 changes: 0 additions & 4 deletions sycl/test-e2e/Regression/device_pci_address_bdf_format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ using namespace sycl;
#endif

int main(int argc, char **argv) {
// Must be enabled at the beginning of the application
// to obtain the PCI address
setenv("SYCL_ENABLE_PCI", "1", 0);

// Expected format is "{domain}:{bus}:{device}.{function} where:
// * {domain} is a 4 character hexadecimal value.
// * {bus} is a 2 character hexadecimal value.
Expand Down