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.

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

[ur] Remove null adapter default load #261

Merged
merged 2 commits into from
Feb 15, 2023
Merged

[ur] Remove null adapter default load #261

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ec1153
[ur] Remove null adapter default load
Feb 14, 2023
f09b645
[ur] Make known plaforms std::array
Feb 15, 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
4 changes: 1 addition & 3 deletions source/loader/linux/platform_discovery_lin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@

namespace loader {

static const char *knownPlatformNames[] = {
MAKE_LIBRARY_NAME("ur_null", UR_VERSION),
};
static constexpr char *knownPlatformNames[] = {};

std::vector<PlatformLibraryPath> discoverEnabledPlatforms() {
std::vector<PlatformLibraryPath> enabledPlatforms;
Expand Down
10 changes: 1 addition & 9 deletions source/loader/windows/platform_discovery_win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,14 @@

namespace loader {

static const char *knownAdaptersNames[] = {
MAKE_LIBRARY_NAME("ur_null", UR_VERSION),
};

std::vector<PlatformLibraryPath> discoverEnabledPlatforms() {
std::vector<PlatformLibraryPath> enabledPlatforms;

// UR_ADAPTERS_FORCE_LOAD is for development/debug only
char *altPlatforms = nullptr;
_dupenv_s(&altPlatforms, NULL, "UR_ADAPTERS_FORCE_LOAD");

if (altPlatforms == nullptr) {
for (auto libName : knownAdaptersNames) {
enabledPlatforms.emplace_back(libName);
}
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately I had to remove knownAdaptersNames from the windows build as the MSVC compiler is quite picky about zero-length arrays. Perhaps I should remove from linux equivalent too?

Copy link
Member

Choose a reason for hiding this comment

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

You could use std::array. zero-length std:array is well defined so Windows should not complain. But if we don't have any adapters yet removing it will work as well I guess.

Copy link
Contributor

@pbalcer pbalcer Feb 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yea, I wouldn't worry about removing this if you need to - this code needs to be refactored anyway.

if (altPlatforms != nullptr) {
std::stringstream ss(altPlatforms);
while (ss.good()) {
std::string substr;
Expand Down
32 changes: 30 additions & 2 deletions test/conformance/source/environment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

namespace uur {

constexpr char ERROR_NO_ADAPTER[] = "Could not load adapter";

PlatformEnvironment *PlatformEnvironment::instance = nullptr;

std::ostream &operator<<(std::ostream &out,
Expand All @@ -32,7 +34,13 @@ uur::PlatformEnvironment::PlatformEnvironment(int argc, char **argv)
: platform_options{parsePlatformOptions(argc, argv)} {
instance = this;
ur_device_init_flags_t device_flags = 0;
if (urInit(device_flags)) {
switch (urInit(device_flags)) {
case UR_RESULT_SUCCESS:
break;
case UR_RESULT_ERROR_UNINITIALIZED:
error = ERROR_NO_ADAPTER;
return;
default:
error = "urInit() failed";
return;
}
Expand Down Expand Up @@ -100,11 +108,18 @@ uur::PlatformEnvironment::PlatformEnvironment(int argc, char **argv)

void uur::PlatformEnvironment::SetUp() {
if (!error.empty()) {
FAIL() << error;
if (error == ERROR_NO_ADAPTER) {
GTEST_SKIP() << error;
} else {
FAIL() << error;
}
}
}

void uur::PlatformEnvironment::TearDown() {
if (error == ERROR_NO_ADAPTER) {
return;
}
ur_tear_down_params_t tear_down_params{};
if (urTearDown(&tear_down_params)) {
FAIL() << "urTearDown() failed";
Expand Down Expand Up @@ -155,8 +170,21 @@ DevicesEnvironment::DevicesEnvironment(int argc, char **argv)

void DevicesEnvironment::SetUp() {
PlatformEnvironment::SetUp();
if (error == ERROR_NO_ADAPTER) {
return;
}
if (devices.empty() || !error.empty()) {
FAIL() << error;
}
}

void DevicesEnvironment::TearDown() {
PlatformEnvironment::TearDown();
for (auto device : devices) {
if (urDeviceRelease(device)) {
error = "urDeviceRelease() failed";
return;
}
}
}
} // namespace uur
2 changes: 1 addition & 1 deletion test/conformance/testing/uur/environment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct DevicesEnvironment : PlatformEnvironment {
virtual ~DevicesEnvironment() override = default;

virtual void SetUp() override;
inline virtual void TearDown() override { PlatformEnvironment::TearDown(); }
virtual void TearDown() override;

inline const std::vector<ur_device_handle_t> &GetDevices() const {
return devices;
Expand Down
3 changes: 2 additions & 1 deletion test/layers/validation/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ function(add_validation_test name)
set_tests_properties(${name} PROPERTIES LABELS "validation")
set_property(TEST ${name} PROPERTY ENVIRONMENT
"UR_ENABLE_VALIDATION_LAYER=1"
"UR_ENABLE_PARAMETER_VALIDATION=1")
"UR_ENABLE_PARAMETER_VALIDATION=1"
"UR_ADAPTERS_FORCE_LOAD=$<TARGET_FILE:ur_null>")
endfunction()

add_validation_test(parameters parameters.cpp)
6 changes: 4 additions & 2 deletions test/loader/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Copyright (C) 2022 Intel Corporation
# SPDX-License-Identifier: MIT

add_test(NAME example-hello-world COMMAND hello_world)
set_tests_properties(${example-hello-world} PROPERTIES LABELS "loader")
add_test(NAME example-hello-world COMMAND hello_world DEPENDS hello_world)
set_tests_properties(example-hello-world PROPERTIES LABELS "loader"
ENVIRONMENT "UR_ADAPTERS_FORCE_LOAD=$<TARGET_FILE:ur_null>"
)
2 changes: 1 addition & 1 deletion test/python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function(add_python_test name)
endif()
# this is for importing the include/ur.py module in other python files
set_property(TEST ${TEST_NAME} APPEND PROPERTY
ENVIRONMENT "PYTHONPATH=${PROJECT_SOURCE_DIR}")
ENVIRONMENT "PYTHONPATH=${PROJECT_SOURCE_DIR}" "UR_ADAPTERS_FORCE_LOAD=$<TARGET_FILE:ur_null>")
endfunction()

add_python_test(basic)