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.

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

Add support for memory region which is BOTH: DeviceLocal AND HostCoherent #66

Open
johannesugb opened this issue Jul 26, 2022 · 0 comments
Labels
enhancement New feature or request urgent This issue has high priority Vulkan Vulkan-centric task

Comments

@johannesugb
Copy link
Member

Modern GPUs typically have a (relatively small) memory region which is both: vk::MemoryPropertyFlagBits::eDeviceLocal | vk::MemoryPropertyFlagBits::eHostCoherent. See the following output of context().print_available_memory_types();:

INFO: ========== MEMORY PROPERTIES OF DEVICE 'NVIDIA GeForce RTX 3070'   | file[avk.cpp] line[118]
INFO: ------------------------------------------------------------- | file[avk.cpp] line[119]
INFO:  HEAP TYPES:                                                  | file[avk.cpp] line[120]
INFO:  heap-idx |               bytes | heap flags                  | file[avk.cpp] line[121]
INFO: ------------------------------------------------------------- | file[avk.cpp] line[122]
INFO:         0 |       8,433,696,768 | { DeviceLocal }             | file[avk.cpp] line[139]
INFO:         1 |      17,141,145,600 | {}                          | file[avk.cpp] line[139]
INFO:         2 |         224,395,264 | { DeviceLocal }             | file[avk.cpp] line[139]
INFO: ============================================================= | file[avk.cpp] line[141]
INFO:  MEMORY TYPES:                                                | file[avk.cpp] line[142]
INFO:  mem-idx | heap-idx | memory propety flags                    | file[avk.cpp] line[143]
INFO: ------------------------------------------------------------- | file[avk.cpp] line[144]
INFO:        0 |         1 | {}                                     | file[avk.cpp] line[154]
INFO:        1 |         0 | { DeviceLocal }                        | file[avk.cpp] line[154]
INFO:        2 |         1 | { HostVisible | HostCoherent }         | file[avk.cpp] line[154]
INFO:        3 |         1 | { HostVisible | HostCoherent | HostCached } | file[avk.cpp] line[154]
INFO:        4 |         2 | { DeviceLocal | HostVisible | HostCoherent } | file[avk.cpp] line[154]
INFO: ============================================================= | file[avk.cpp] line[156]

The problem is just, that enum struct memory_usage only supports device OR host_coherent, but not the combination of both.

Furthermore, the framework does probably disregard which memory region exactly a memory_usage::device is allocated from (in the example above, the options would be memory regions at heap indices 0 or 2, but the framework does probably not really care which one it chooses -- but it should!)

memory_usage::device should allocate from the memory region that has the flag vk::MemoryPropertyFlagBits::eDeviceLocal, but does not have vk::MemoryPropertyFlagBits::eHostCoherent! Furthermore, this flag should probably be renamed into memory_usage::device_local!

memory_usage::host_coherent should allocate from the memory region that has the flag vk::MemoryPropertyFlagBits::eHostCoherent, but does not have vk::MemoryPropertyFlagBits::eDeviceLocal!

And a new memory_usage::device_local_and_host_coherent should allocate from the memory region that has both flags: vk::MemoryPropertyFlagBits::eDeviceLocal | vk::MemoryPropertyFlagBits::eHostCoherent. The question is just if there is a better name for this. (I think, there was a dedicated name for this memory region, but I can't remember.)

@johannesugb johannesugb added enhancement New feature or request Vulkan Vulkan-centric task urgent This issue has high priority C++ C++-centric task and removed C++ C++-centric task labels Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request urgent This issue has high priority Vulkan Vulkan-centric task
Development

No branches or pull requests

1 participant