Skip to content

Commit 1e640c9

Browse files
peppsacpeppsac
committed
Store ICDs in the loading order
This commit switch from prepending newly loaded ICDs to the ICD list, to appending them. Both options are fine since the order VkPhysicalDevices appear is unspecified, but by using an append operation the VkPhysicalDevices are returned in the same order as the drivers are loaded. See #1725
1 parent 586a3f8 commit 1e640c9

File tree

6 files changed

+55
-49
lines changed

6 files changed

+55
-49
lines changed

‎loader/loader.c‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,9 +1492,15 @@ struct loader_icd_term *loader_icd_add(struct loader_instance *ptr_inst, const s
14921492
icd_term->scanned_icd = scanned_icd;
14931493
icd_term->this_instance = ptr_inst;
14941494

1495-
// Prepend to the list
1496-
icd_term->next = ptr_inst->icd_terms;
1497-
ptr_inst->icd_terms = icd_term;
1495+
// Append to the list
1496+
struct loader_icd_term *prev = ptr_inst->icd_terms;
1497+
if (prev == NULL) {
1498+
ptr_inst->icd_terms = icd_term;
1499+
} else {
1500+
while (prev->next)
1501+
prev = prev->next;
1502+
prev->next = icd_term;
1503+
}
14981504
ptr_inst->icd_terms_count++;
14991505

15001506
return icd_term;

‎tests/loader_get_proc_addr_tests.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,9 @@ TEST(GetProcAddr, Verify10FunctionsFailToLoadWithSingleDriver) {
188188

189189
TEST(GetProcAddr, Verify10FunctionsLoadWithMultipleDrivers) {
190190
FrameworkEnvironment env{};
191-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});
191+
192192
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({}).set_can_query_GetPhysicalDeviceFuncs(false);
193+
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});
193194

194195
InstWrapper inst{env.vulkan_functions};
195196
inst.CheckCreate();

‎tests/loader_regression_tests.cpp‎

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3208,31 +3208,31 @@ TEST(SortedPhysicalDevices, DevicesSortedDisabled) {
32083208
ASSERT_EQ(VK_SUCCESS, instance->vkEnumeratePhysicalDevices(instance, &device_count, physical_devices.data()));
32093209
ASSERT_EQ(device_count, max_phys_devs);
32103210

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

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

32213221
instance->vkGetPhysicalDeviceProperties(physical_devices[2], &props);
3222-
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3223-
ASSERT_STREQ(props.deviceName, "pd4");
3224-
3225-
instance->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
32263222
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_CPU);
32273223
ASSERT_STREQ(props.deviceName, "pd2");
32283224

3225+
instance->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
3226+
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3227+
ASSERT_STREQ(props.deviceName, "pd3");
3228+
32293229
instance->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
32303230
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3231-
ASSERT_STREQ(props.deviceName, "pd0");
3231+
ASSERT_STREQ(props.deviceName, "pd4");
32323232

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

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

3537-
// make sure the order is what we started with - but its a bit wonky due to the loader reading physical devices "backwards"
3537+
// make sure the order is what we started with
35383538
VkPhysicalDeviceProperties props{};
3539+
35393540
inst->vkGetPhysicalDeviceProperties(physical_devices[0], &props);
3540-
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU);
3541-
ASSERT_STREQ(props.deviceName, "pd7");
3541+
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3542+
ASSERT_STREQ(props.deviceName, "pd0");
35423543

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

35473548
inst->vkGetPhysicalDeviceProperties(physical_devices[2], &props);
35483549
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3549-
ASSERT_STREQ(props.deviceName, "pd5");
3550+
ASSERT_STREQ(props.deviceName, "pd2");
35503551

35513552
inst->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
3552-
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3553-
ASSERT_STREQ(props.deviceName, "pd6");
3554-
3555-
inst->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
35563553
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_CPU);
35573554
ASSERT_STREQ(props.deviceName, "pd3");
35583555

3556+
inst->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
3557+
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3558+
ASSERT_STREQ(props.deviceName, "pd4");
3559+
35593560
inst->vkGetPhysicalDeviceProperties(physical_devices[5], &props);
35603561
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3561-
ASSERT_STREQ(props.deviceName, "pd0");
3562+
ASSERT_STREQ(props.deviceName, "pd5");
35623563

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

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

35713572
// Make sure if we call enumerate again, the information is the same
35723573
std::array<VkPhysicalDeviceGroupProperties, max_phys_dev_groups> physical_device_groups_again{};

‎tests/loader_settings_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,8 +3057,8 @@ TEST(SettingsFile, AdditionalDrivers) {
30573057
VkPhysicalDeviceProperties props1{}, props2{};
30583058
inst.functions->vkGetPhysicalDeviceProperties(pds.at(0), &props1);
30593059
inst.functions->vkGetPhysicalDeviceProperties(pds.at(1), &props2);
3060-
ASSERT_TRUE(string_eq(props1.deviceName, regular_driver_name));
3061-
ASSERT_TRUE(string_eq(props2.deviceName, settings_driver_name));
3060+
ASSERT_TRUE(string_eq(props1.deviceName, settings_driver_name));
3061+
ASSERT_TRUE(string_eq(props2.deviceName, regular_driver_name));
30623062
}
30633063
// settings file provided drivers replacing system found drivers
30643064
TEST(SettingsFile, ExclusiveAdditionalDrivers) {

‎tests/loader_version_tests.cpp‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -993,25 +993,23 @@ void CheckDirectDriverLoading(FrameworkEnvironment& env, std::vector<DriverInfo>
993993
auto phys_devs = inst.GetPhysDevs();
994994
ASSERT_EQ(phys_devs.size(), expected_driver_count);
995995

996-
// We have to iterate through the driver lists backwards because the loader *prepends* icd's, so the last found ICD is found
997-
// first in the driver list
998996
uint32_t driver_index = 0;
999-
if (!normal_drivers.empty()) {
1000-
for (int i = normal_drivers.size() - 1; i >= 0; i--) {
1001-
if (!exclusive && normal_drivers.at(i).expect_to_find) {
997+
if (!direct_drivers.empty()) {
998+
for (size_t i = 0; i < direct_drivers.size(); i++) {
999+
if (direct_drivers.at(i).expect_to_find) {
10021000
VkPhysicalDeviceProperties props{};
10031001
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
1004-
ASSERT_EQ(props.driverVersion, normal_drivers.at(i).driver_version);
1002+
ASSERT_EQ(props.driverVersion, direct_drivers.at(i).driver_version);
10051003
driver_index++;
10061004
}
10071005
}
10081006
}
1009-
if (!direct_drivers.empty()) {
1010-
for (int i = direct_drivers.size() - 1; i >= 0; i--) {
1011-
if (direct_drivers.at(i).expect_to_find) {
1007+
if (!normal_drivers.empty()) {
1008+
for (size_t i = 0; i < normal_drivers.size(); i++) {
1009+
if (!exclusive && normal_drivers.at(i).expect_to_find) {
10121010
VkPhysicalDeviceProperties props{};
10131011
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
1014-
ASSERT_EQ(props.driverVersion, direct_drivers.at(i).driver_version);
1012+
ASSERT_EQ(props.driverVersion, normal_drivers.at(i).driver_version);
10151013
driver_index++;
10161014
}
10171015
}

‎tests/loader_wsi_tests.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,9 +1066,9 @@ TEST(WsiTests, MultiPlatformGetPhysicalDeviceSurfaceSupportKHR) {
10661066
inst.CheckCreate();
10671067

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

11141114
auto phys_devs = inst.GetPhysDevs();
1115-
// Physical devices are enumerated in reverse order to the ICD order
1116-
VkPhysicalDevice xcb_physical_device = phys_devs[1];
1117-
VkPhysicalDevice wayland_physical_device = phys_devs[0];
1115+
// Physical devices are enumerated in order to the ICD order
1116+
VkPhysicalDevice xcb_physical_device = phys_devs[0];
1117+
VkPhysicalDevice wayland_physical_device = phys_devs[1];
11181118
VkPhysicalDeviceProperties props0{};
11191119
inst->vkGetPhysicalDeviceProperties(wayland_physical_device, &props0);
11201120
ASSERT_TRUE(string_eq(props0.deviceName, wayland_device_name));

0 commit comments

Comments
 (0)