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

Incompatibality inroduced with #168 on Vulkan 1.1.x #209

Closed
unexploredtest opened this issue Apr 17, 2021 · 6 comments
Closed

Incompatibality inroduced with #168 on Vulkan 1.1.x #209

unexploredtest opened this issue Apr 17, 2021 · 6 comments

Comments

@unexploredtest
Copy link
Contributor

unexploredtest commented Apr 17, 2021

In src/Manager.cpp
line 323:
physicalDeviceProperties.deviceName.data());
and line 381:
std::string extName(ext.extensionName.data());
Are incompatible with older versions of Vulkan (1.1.x)
Error:

   /project/src/Manager.cpp: In member function ‘void kp::Manager::createDevice(const std::vector<unsigned int>&, uint32_t, const std::vector<std::basic_string<char> >&)’:
  /project/src/Manager.cpp:304:53: error: request for member ‘data’ in ‘physicalDeviceProperties.vk::PhysicalDeviceProperties::deviceName’, which is of non-class type ‘char [256]’
    304 |                 physicalDeviceProperties.deviceName.data());
        |                                                     ^~~~
  /project/src/include/kompute/Core.hpp:87:46: note: in definition of macro ‘KP_LOG_INFO’
     87 | #define KP_LOG_INFO(...) kp_info(fmt::format(__VA_ARGS__))
        |                                              ^~~~~~~~~~~
  /project/src/Manager.cpp:362:47: error: request for member ‘data’ in ‘ext.vk::ExtensionProperties::extensionName’, which is of non-class type ‘const char [256]’
    362 |         std::string extName(ext.extensionName.data());
        |                                               ^~~~
  /project/src/OpAlgoDispatch.cpp:1:9: warning: #pragma once in main file
      1 | #pragma once
        |         ^~~~
  gmake[2]: *** [src/CMakeFiles/kompute.dir/Manager.cpp.o] Error 1
  gmake[2]: *** Waiting for unfinished jobs....
  gmake[1]: *** [src/CMakeFiles/kompute.dir/all] Error 2
  gmake: *** [all] Error 2

EDIT: Tested with Vulkan SDK-1.1.82

@unexploredtest
Copy link
Contributor Author

@axsaucedo Is there a reason .data() was used?
If I change physicalDeviceProperties.deviceName.data()); to physicalDeviceProperties.deviceName); and

    for (const vk::ExtensionProperties& ext : deviceExtensions) {
        std::string extName(ext.extensionName.data());
        uniqueExtensionNames.insert(extName);
    }

into:

    for (const vk::ExtensionProperties& ext : deviceExtensions) {
        uniqueExtensionNames.insert(ext.extensionName);
    }

The problem is solved and I get the same functionality; unless there is something I'm missing

@axsaucedo
Copy link
Member

@unexploredtest interesting, - just had a look at the codebase and it seems that extensionName is a valid std:string, not sure why it was converted into data - if this fixes it then it makes sense to update it into your suggestion

@unexploredtest
Copy link
Contributor Author

unexploredtest commented Apr 19, 2021

Sure
What about physicalDeviceProperties.deviceName.data());? Is .data() needed?

@axsaucedo
Copy link
Member

Hmm, I don't really remember why it was actually added explicitly the data - I think when I was trying it without it I was seeing some errors, so would be worth actually testing with value inputs that actually call those data-paths. Probably best would be to try this one as well, if it ultimately passes on setting different values then it should be ok to update

@unexploredtest
Copy link
Contributor Author

unexploredtest commented Apr 20, 2021

Actually, it works fine without it, could compile for both Vulkan 1.2.x and 1.1.x and pass the tests.
Made a PR #211

@unexploredtest
Copy link
Contributor Author

Closing the issue as #211 fixed the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants