Skip to content

Sort PhysicalDevices to match ICD's ordering #1744

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

Open
peppsac wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Sort PhysicalDevices to match ICD's ordering #1744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
586a3f8
Fix backward loop condition in tests
peppsac Jun 11, 2025
d129f08
Store ICDs in the loading order
peppsac Jun 11, 2025
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
11 changes: 8 additions & 3 deletions loader/loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1492,9 +1492,14 @@ struct loader_icd_term *loader_icd_add(struct loader_instance *ptr_inst, const s
icd_term->scanned_icd = scanned_icd;
icd_term->this_instance = ptr_inst;

// Prepend to the list
icd_term->next = ptr_inst->icd_terms;
ptr_inst->icd_terms = icd_term;
// Append to the list
struct loader_icd_term *prev = ptr_inst->icd_terms;
if (prev == NULL) {
ptr_inst->icd_terms = icd_term;
} else {
while (prev->next) prev = prev->next;
prev->next = icd_term;
}
Copy link
Contributor

@cubanismo cubanismo Jul 30, 2025

Choose a reason for hiding this comment

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

It would be nice to track the list tail to avoid iterating the entire list here, but probably a moot point with realistic numbers of ICDs, so not a big deal. This way is certainly less complex.

ptr_inst->icd_terms_count++;

return icd_term;
Expand Down
3 changes: 2 additions & 1 deletion tests/loader_get_proc_addr_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,9 @@ TEST(GetProcAddr, Verify10FunctionsFailToLoadWithSingleDriver) {

TEST(GetProcAddr, Verify10FunctionsLoadWithMultipleDrivers) {
FrameworkEnvironment env{};
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});

env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({}).set_can_query_GetPhysicalDeviceFuncs(false);
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});

InstWrapper inst{env.vulkan_functions};
inst.CheckCreate();
Expand Down
59 changes: 30 additions & 29 deletions tests/loader_regression_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3208,31 +3208,31 @@ TEST(SortedPhysicalDevices, DevicesSortedDisabled) {
ASSERT_EQ(VK_SUCCESS, instance->vkEnumeratePhysicalDevices(instance, &device_count, physical_devices.data()));
ASSERT_EQ(device_count, max_phys_devs);

// make sure the order is what we started with - but its a bit wonky due to the loader reading physical devices "backwards"
// make sure the order is what we started with
VkPhysicalDeviceProperties props{};
instance->vkGetPhysicalDeviceProperties(physical_devices[0], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU);
ASSERT_STREQ(props.deviceName, "pd5");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd0");

instance->vkGetPhysicalDeviceProperties(physical_devices[1], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd3");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU);
ASSERT_STREQ(props.deviceName, "pd1");

instance->vkGetPhysicalDeviceProperties(physical_devices[2], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd4");

instance->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_CPU);
ASSERT_STREQ(props.deviceName, "pd2");

instance->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd3");

instance->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd0");
ASSERT_STREQ(props.deviceName, "pd4");

instance->vkGetPhysicalDeviceProperties(physical_devices[5], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU);
ASSERT_STREQ(props.deviceName, "pd1");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU);
ASSERT_STREQ(props.deviceName, "pd5");

// Make sure if we call enumerate again, the information is the same
std::array<VkPhysicalDevice, max_phys_devs> physical_devices_again;
Expand Down Expand Up @@ -3534,39 +3534,40 @@ TEST(SortedPhysicalDevices, DeviceGroupsSortedDisabled) {
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, physical_device_groups.data()));
ASSERT_EQ(group_count, max_phys_dev_groups);

// make sure the order is what we started with - but its a bit wonky due to the loader reading physical devices "backwards"
// make sure the order is what we started with
VkPhysicalDeviceProperties props{};

inst->vkGetPhysicalDeviceProperties(physical_devices[0], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU);
ASSERT_STREQ(props.deviceName, "pd7");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd0");

inst->vkGetPhysicalDeviceProperties(physical_devices[1], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd4");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU);
ASSERT_STREQ(props.deviceName, "pd1");

inst->vkGetPhysicalDeviceProperties(physical_devices[2], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd5");
ASSERT_STREQ(props.deviceName, "pd2");

inst->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd6");

inst->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_CPU);
ASSERT_STREQ(props.deviceName, "pd3");

inst->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd4");

inst->vkGetPhysicalDeviceProperties(physical_devices[5], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd0");
ASSERT_STREQ(props.deviceName, "pd5");

inst->vkGetPhysicalDeviceProperties(physical_devices[6], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU);
ASSERT_STREQ(props.deviceName, "pd1");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd6");

inst->vkGetPhysicalDeviceProperties(physical_devices[7], &props);
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
ASSERT_STREQ(props.deviceName, "pd2");
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU);
ASSERT_STREQ(props.deviceName, "pd7");

// Make sure if we call enumerate again, the information is the same
std::array<VkPhysicalDeviceGroupProperties, max_phys_dev_groups> physical_device_groups_again{};
Expand Down Expand Up @@ -4295,10 +4296,10 @@ TEST(EnumerateAdapterPhysicalDevices, SameAdapterLUID_reordered) {

// Physical devices are enumerated:
// a) first in the order of LUIDs showing up in DXGIAdapter list
// b) then in the reverse order to the drivers insertion into the test framework
// b) then in the order to the drivers insertion into the test framework
add_dxgi_adapter(env, "physical_device_2", LUID{10, 100}, 2);
add_dxgi_adapter(env, "physical_device_1", LUID{20, 200}, 1);
add_dxgi_adapter(env, "physical_device_0", LUID{10, 100}, 2);
add_dxgi_adapter(env, "physical_device_1", LUID{20, 200}, 1);

{
uint32_t returned_physical_count = 0;
Expand Down
4 changes: 2 additions & 2 deletions tests/loader_settings_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3057,8 +3057,8 @@ TEST(SettingsFile, AdditionalDrivers) {
VkPhysicalDeviceProperties props1{}, props2{};
inst.functions->vkGetPhysicalDeviceProperties(pds.at(0), &props1);
inst.functions->vkGetPhysicalDeviceProperties(pds.at(1), &props2);
ASSERT_TRUE(string_eq(props1.deviceName, regular_driver_name));
ASSERT_TRUE(string_eq(props2.deviceName, settings_driver_name));
ASSERT_TRUE(string_eq(props1.deviceName, settings_driver_name));
ASSERT_TRUE(string_eq(props2.deviceName, regular_driver_name));
}
// settings file provided drivers replacing system found drivers
TEST(SettingsFile, ExclusiveAdditionalDrivers) {
Expand Down
30 changes: 16 additions & 14 deletions tests/loader_version_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,23 +993,25 @@ void CheckDirectDriverLoading(FrameworkEnvironment& env, std::vector<DriverInfo>
auto phys_devs = inst.GetPhysDevs();
ASSERT_EQ(phys_devs.size(), expected_driver_count);

// We have to iterate through the driver lists backwards because the loader *prepends* icd's, so the last found ICD is found
// first in the driver list
uint32_t driver_index = 0;
for (size_t i = normal_drivers.size() - 1; i == 0; i--) {
if (normal_drivers.at(i).expect_to_find) {
VkPhysicalDeviceProperties props{};
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
ASSERT_EQ(props.driverVersion, normal_drivers.at(i).driver_version);
driver_index++;
if (!direct_drivers.empty()) {
for (size_t i = 0; i < direct_drivers.size(); i++) {
if (direct_drivers.at(i).expect_to_find) {
VkPhysicalDeviceProperties props{};
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
ASSERT_EQ(props.driverVersion, direct_drivers.at(i).driver_version);
driver_index++;
}
}
}
for (size_t i = direct_drivers.size() - 1; i == 0; i--) {
if (direct_drivers.at(i).expect_to_find) {
VkPhysicalDeviceProperties props{};
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
ASSERT_EQ(props.driverVersion, direct_drivers.at(i).driver_version);
driver_index++;
if (!normal_drivers.empty()) {
for (size_t i = 0; i < normal_drivers.size(); i++) {
if (!exclusive && normal_drivers.at(i).expect_to_find) {
VkPhysicalDeviceProperties props{};
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
ASSERT_EQ(props.driverVersion, normal_drivers.at(i).driver_version);
driver_index++;
}
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions tests/loader_wsi_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,9 +1066,9 @@ TEST(WsiTests, MultiPlatformGetPhysicalDeviceSurfaceSupportKHR) {
inst.CheckCreate();

auto phys_devs = inst.GetPhysDevs();
// Physical devices are enumerated in reverse order to the ICD order
VkPhysicalDevice xcb_physical_device = phys_devs[1];
VkPhysicalDevice wayland_physical_device = phys_devs[0];
// Physical devices are enumerated in order to the ICD order
VkPhysicalDevice xcb_physical_device = phys_devs[0];
VkPhysicalDevice wayland_physical_device = phys_devs[1];
VkPhysicalDeviceProperties props0{};
inst->vkGetPhysicalDeviceProperties(wayland_physical_device, &props0);
ASSERT_TRUE(string_eq(props0.deviceName, wayland_device_name));
Expand Down Expand Up @@ -1112,9 +1112,9 @@ TEST(WsiTests, MultiPlatformGetPhysicalDeviceSurfaceSupportKHR) {
inst.CheckCreate();

auto phys_devs = inst.GetPhysDevs();
// Physical devices are enumerated in reverse order to the ICD order
VkPhysicalDevice xcb_physical_device = phys_devs[1];
VkPhysicalDevice wayland_physical_device = phys_devs[0];
// Physical devices are enumerated in ICD discovery order
VkPhysicalDevice xcb_physical_device = phys_devs[0];
VkPhysicalDevice wayland_physical_device = phys_devs[1];
VkPhysicalDeviceProperties props0{};
inst->vkGetPhysicalDeviceProperties(wayland_physical_device, &props0);
ASSERT_TRUE(string_eq(props0.deviceName, wayland_device_name));
Expand Down
Loading