Skip to content

Commit 7db9142

Browse files
peppsaccharles-lunarg
peppsac
authored and
charles-lunarg
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 9c934e9 commit 7db9142

File tree

6 files changed

+59
-55
lines changed

6 files changed

+59
-55
lines changed

‎loader/loader.c‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,9 +1768,14 @@ struct loader_icd_term *loader_icd_add(struct loader_instance *ptr_inst, const s
17681768
icd_term->scanned_icd = scanned_icd;
17691769
icd_term->this_instance = ptr_inst;
17701770

1771-
// Prepend to the list
1772-
icd_term->next = ptr_inst->icd_terms;
1773-
ptr_inst->icd_terms = icd_term;
1771+
// Append to the list
1772+
struct loader_icd_term *prev = ptr_inst->icd_terms;
1773+
if (prev == NULL) {
1774+
ptr_inst->icd_terms = icd_term;
1775+
} else {
1776+
while (prev->next) prev = prev->next;
1777+
prev->next = icd_term;
1778+
}
17741779
ptr_inst->icd_terms_count++;
17751780

17761781
return icd_term;

‎tests/loader_get_proc_addr_tests.cpp‎

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

189189
TEST(GetProcAddr, Verify10FunctionsLoadWithMultipleDrivers) {
190190
FrameworkEnvironment env{};
191-
env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device({});
192191
env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device({}).set_can_query_GetPhysicalDeviceFuncs(false);
192+
env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device({});
193193

194194
InstWrapper inst{env.vulkan_functions};
195195
inst.CheckCreate();

‎tests/loader_regression_tests.cpp‎

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3658,31 +3658,31 @@ TEST(SortedPhysicalDevices, DevicesSortedDisabled) {
36583658
ASSERT_EQ(VK_SUCCESS, instance->vkEnumeratePhysicalDevices(instance, &device_count, physical_devices.data()));
36593659
ASSERT_EQ(device_count, max_phys_devs);
36603660

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

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

36713671
instance->vkGetPhysicalDeviceProperties(physical_devices[2], &props);
3672-
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3673-
ASSERT_STREQ(props.deviceName, "pd4");
3674-
3675-
instance->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
36763672
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_CPU);
36773673
ASSERT_STREQ(props.deviceName, "pd2");
36783674

3675+
instance->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
3676+
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3677+
ASSERT_STREQ(props.deviceName, "pd3");
3678+
36793679
instance->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
36803680
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3681-
ASSERT_STREQ(props.deviceName, "pd0");
3681+
ASSERT_STREQ(props.deviceName, "pd4");
36823682

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

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

3977-
// make sure the order is what we started with - but its a bit wonky due to the loader reading physical devices "backwards"
3977+
// make sure the order is what we started with
39783978
VkPhysicalDeviceProperties props{};
3979+
39793980
inst->vkGetPhysicalDeviceProperties(physical_devices[0], &props);
3980-
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU);
3981-
ASSERT_STREQ(props.deviceName, "pd7");
3981+
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3982+
ASSERT_STREQ(props.deviceName, "pd0");
39823983

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

39873988
inst->vkGetPhysicalDeviceProperties(physical_devices[2], &props);
39883989
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3989-
ASSERT_STREQ(props.deviceName, "pd5");
3990+
ASSERT_STREQ(props.deviceName, "pd2");
39903991

39913992
inst->vkGetPhysicalDeviceProperties(physical_devices[3], &props);
3992-
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3993-
ASSERT_STREQ(props.deviceName, "pd6");
3994-
3995-
inst->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
39963993
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_CPU);
39973994
ASSERT_STREQ(props.deviceName, "pd3");
39983995

3996+
inst->vkGetPhysicalDeviceProperties(physical_devices[4], &props);
3997+
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
3998+
ASSERT_STREQ(props.deviceName, "pd4");
3999+
39994000
inst->vkGetPhysicalDeviceProperties(physical_devices[5], &props);
40004001
ASSERT_EQ(props.deviceType, VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU);
4001-
ASSERT_STREQ(props.deviceName, "pd0");
4002+
ASSERT_STREQ(props.deviceName, "pd5");
40024003

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

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

40114012
// Make sure if we call enumerate again, the information is the same
40124013
std::array<VkPhysicalDeviceGroupProperties, max_phys_dev_groups> physical_device_groups_again{};
@@ -4446,9 +4447,9 @@ TEST(ManifestDiscovery, AppleBundles) {
44464447
// Should get both GPUs, in reverse order to driver enumeration (due to enumerating the last driver first)
44474448
VkPhysicalDeviceProperties props{};
44484449
inst->vkGetPhysicalDeviceProperties(physical_devices[0], &props);
4449-
ASSERT_EQ(test_physical_device_1.properties.deviceID, props.deviceID);
4450-
inst->vkGetPhysicalDeviceProperties(physical_devices[1], &props);
44514450
ASSERT_EQ(test_physical_device_0.properties.deviceID, props.deviceID);
4451+
inst->vkGetPhysicalDeviceProperties(physical_devices[1], &props);
4452+
ASSERT_EQ(test_physical_device_1.properties.deviceID, props.deviceID);
44524453
}
44534454

44544455
// Add two drivers, one to the bundle and one using the driver env-var
@@ -4717,10 +4718,10 @@ TEST(EnumerateAdapterPhysicalDevices, SameAdapterLUID_reordered) {
47174718

47184719
// Physical devices are enumerated:
47194720
// a) first in the order of LUIDs showing up in DXGIAdapter list
4720-
// b) then in the reverse order to the drivers insertion into the test framework
4721-
add_dxgi_adapter(env, "physical_device_2", LUID{10, 100}, 2);
4722-
add_dxgi_adapter(env, "physical_device_1", LUID{20, 200}, 1);
4721+
// b) then in the order to the drivers insertion into the test framework
47234722
auto phys_dev_handle = add_dxgi_adapter(env, "physical_device_0", LUID{10, 100}, 2);
4723+
add_dxgi_adapter(env, "physical_device_1", LUID{20, 200}, 1);
4724+
add_dxgi_adapter(env, "physical_device_2", LUID{10, 100}, 2);
47244725

47254726
{
47264727
uint32_t returned_physical_count = 0;
@@ -4764,7 +4765,7 @@ TEST(EnumerateAdapterPhysicalDevices, SameAdapterLUID_reordered) {
47644765
}
47654766
// Set the first physical device that is enumerated to be a 'layered' driver so it should be swapped with the first physical
47664767
// device
4767-
env.get_test_icd(2).physical_devices.at(phys_dev_handle).layered_driver_underlying_api =
4768+
env.get_test_icd(0).physical_devices.at(phys_dev_handle).layered_driver_underlying_api =
47684769
VK_LAYERED_DRIVER_UNDERLYING_API_D3D12_MSFT;
47694770
{
47704771
uint32_t returned_physical_count = 0;

‎tests/loader_settings_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3069,8 +3069,8 @@ TEST(SettingsFile, AdditionalDrivers) {
30693069
VkPhysicalDeviceProperties props1{}, props2{};
30703070
inst.functions->vkGetPhysicalDeviceProperties(pds.at(0), &props1);
30713071
inst.functions->vkGetPhysicalDeviceProperties(pds.at(1), &props2);
3072-
ASSERT_TRUE(string_eq(props1.deviceName, regular_driver_name));
3073-
ASSERT_TRUE(string_eq(props2.deviceName, settings_driver_name));
3072+
ASSERT_TRUE(string_eq(props1.deviceName, settings_driver_name));
3073+
ASSERT_TRUE(string_eq(props2.deviceName, regular_driver_name));
30743074
}
30753075
// settings file provided drivers replacing system found drivers
30763076
TEST(SettingsFile, ExclusiveAdditionalDrivers) {

‎tests/loader_version_tests.cpp‎

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

994-
// We have to iterate through the driver lists backwards because the loader *prepends* icd's, so the last found ICD is found
995-
// first in the driver list
996994
uint32_t driver_index = 0;
997-
if (!normal_drivers.empty()) {
998-
for (int i = normal_drivers.size() - 1; i >= 0; i--) {
999-
if (!exclusive && normal_drivers.at(i).expect_to_find) {
995+
if (!direct_drivers.empty()) {
996+
for (size_t i = 0; i < direct_drivers.size(); i++) {
997+
if (direct_drivers.at(i).expect_to_find) {
1000998
VkPhysicalDeviceProperties props{};
1001999
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
1002-
ASSERT_EQ(props.driverVersion, normal_drivers.at(i).driver_version);
1000+
ASSERT_EQ(props.driverVersion, direct_drivers.at(i).driver_version);
10031001
driver_index++;
10041002
}
10051003
}
10061004
}
1007-
if (!direct_drivers.empty()) {
1008-
for (int i = direct_drivers.size() - 1; i >= 0; i--) {
1009-
if (direct_drivers.at(i).expect_to_find) {
1005+
if (!normal_drivers.empty()) {
1006+
for (size_t i = 0; i < normal_drivers.size(); i++) {
1007+
if (!exclusive && normal_drivers.at(i).expect_to_find) {
10101008
VkPhysicalDeviceProperties props{};
10111009
inst.functions->vkGetPhysicalDeviceProperties(phys_devs.at(driver_index), &props);
1012-
ASSERT_EQ(props.driverVersion, direct_drivers.at(i).driver_version);
1010+
ASSERT_EQ(props.driverVersion, normal_drivers.at(i).driver_version);
10131011
driver_index++;
10141012
}
10151013
}

‎tests/loader_wsi_tests.cpp‎

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

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

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

0 commit comments

Comments
 (0)