VmaCountBitsSet crashes on older CPUs without support for POPCNT

[realgdman]

Godot version

4.0.dev (fe95aa2)

System information

Windows 10, Vulkan, Nvidia 750ti (30.0.15.1123)

Issue description

4.0 master crash on startup. Same with --single-window, -e project.
I have bisected and found issue starting from commit
fe95aa2 is the first bad commit
Fix extension registration order.

Example of error message

I:\Work> Godot Engine v4.0.alpha.custom_build.2b8a6c14d - https://godotengine.org
Vulkan API 1.2.189 - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 750 Ti

I:\Work> Godot Engine v4.0.alpha.custom_build.2b8a6c14d - https://godotengine.org
================================================================
CrashHandlerException: Program crashed
Vulkan API 1.2.189 - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 750 Ti
Engine version: Godot Engine v4.0.alpha.custom_build (2b8a6c14dfcfa480d8a9e88c823578a74b43bd73)
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] VmaCountBitsSet (I:\Work\godot\thirdparty\vulkan\vk_mem_alloc.h:3064)
[1] vmaFindMemoryTypeIndex (I:\Work\godot\thirdparty\vulkan\vk_mem_alloc.h:16806)
[2] VmaAllocator_T::AllocateMemory (I:\Work\godot\thirdparty\vulkan\vk_mem_alloc.h:15462)
[3] vmaCreateBuffer (I:\Work\godot\thirdparty\vulkan\vk_mem_alloc.h:17545)
[4] RenderingDeviceVulkan::_insert_staging_block (I:\Work\godot\drivers\vulkan\rendering_device_vulkan.cpp:1388)
[5] RenderingDeviceVulkan::initialize (I:\Work\godot\drivers\vulkan\rendering_device_vulkan.cpp:8932)
[6] DisplayServerWindows::DisplayServerWindows (I:\Work\godot\platform\windows\display_server_windows.cpp:3486)
[7] DisplayServerWindows::create_func (I:\Work\godot\platform\windows\display_server_windows.cpp:3529)
[8] DisplayServer::create (I:\Work\godot\servers\display_server.cpp:602)
[9] Main::setup2 (I:\Work\godot\main\main.cpp:1579)
[10] Main::setup (I:\Work\godot\main\main.cpp:1485)
[11] widechar_main (I:\Work\godot\platform\windows\godot_windows.cpp:156)
[12] _main (I:\Work\godot\platform\windows\godot_windows.cpp:191)
[13] main (I:\Work\godot\platform\windows\godot_windows.cpp:205)
[14] WinMain (I:\Work\godot\platform\windows\godot_windows.cpp:219)
[15] __scrt_common_main_seh (d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[16] BaseThreadInitThunk
-- END OF BACKTRACE --
================================================================

Steps to reproduce

1. Run, observe immediate crash

Minimal reproduction project

Not required

[akien-mga]

I have bisected and found issue starting from commit
fe95aa2 is the first bad commit

Are you sure about this? Can you confirm by doing a local revert of that commit (git revert fe95aa2) on top of the current master?

Do you use any GDExtension?

[realgdman]

Actually what I wrote makes no sense, because fe95aa2 comes before alpha 3 commit. May be I should do some clean rebuild.

[realgdman]

Still crashed after clean build, however build of alpha 2 was successful, so I will need another day to bisect.

[realgdman]

@akien-mga I have traced my crash to this commit
648a105 is the first bad commit
vk_mem_alloc: Update to latest commit

Some observation
I can't run builds starting from 648a105 neither debug or release_debug
I can't run this artifact which is corresponds to 4.0 Alpha 3 https://github.com/godotengine/godot/actions/runs/1878375090
However i CAN run alpha3 which I downloaded from https://downloads.tuxfamily.org/godotengine/4.0/alpha3/

Curious if there is some difference in build process on Windows. From what I gather default scons build is with use_volk=yes I had no success in building with use_volk=no due to fatal error LNK1181: cannot open input file 'vulkan.lib'

[bruvzg]

I had no success in building with use_volk=no due to fatal error LNK1181: cannot open input file 'vulkan.lib'

You need to have Vulkan SDK installed to build with use_volk=no (it will statically link Vulkan loader), with use_volk=yes Vulkan loader is loaded dynamically.

[akien-mga]

Could you test this PR which further updates vk_mem_alloc (and volk): #58491 ?

[realgdman]

@bruvzg I have installed Vulkan SDK but 1) file it contains named vulkan-1.lib, and 2) it still doesn't see it, even if I put into some godot build folder, do I need to specify some env variables or set scons var on Windows?

[bruvzg]

It's probably was never tested with use_volk=no.

There's no special option to set up path. You can use generic LINKFLAGS="-Lpath" for MinGW and LINKFLAGS="/LIBPATH:dir" for MSVC. Also, library name might be changed since it was added. You can try renaming the library or changing its name in the platform/windows/detect.py, but it's unlikely that the issue is related to volk.

[realgdman]

@akien-mga unfortunately building #58491 didn't help

[realgdman]

I realized what's going on. My 10 years old CPU doesnt have POPCNT instruction. VmaCountBitsSet had been optimized with intrinsic but doesn't actually check cpuid if that's available on machine.

#ifdef _MSC_VER
return __popcnt(v);
#elif defined __GNUC__ || defined __clang__
return static_cast<uint32_t>(__builtin_popcount(v));
#else
//...byte crunching implementation

Replacing with latter implementation allowed Godot to launch.

[Calinou]

My 10 years old CPU doesnt have POPCNT instruction.

Out of curiosity, what CPU model do you have?

[realgdman]

@Calinou basically LGA 775 chipset with either Intel Core2Duo E8500 or Core2Quad Q9660

[akien-mga]

CC @RandomShaper

It would be good to contribute this fix upstream https://github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator and then we can update our version (or backport the patch).

[realgdman]

@akien-mga this is merely workaround for me and not a proper fix, I don't wanna users of godot on modern hardware get performance hit, in case vulkan uses it in some double loops in heap allocation algorithm

IMHO proper fix would be either separate builds with, like, #define popcnt(v) implementation(v) for uncapable machines, or separate functions for choosing at runtime based on cpuid

[Calinou]

See also godotengine/godot-proposals#3932 which would benefit from cpuid detection as well. Eventually, we'll need to look into it to provide more optimized code without dropping support for old CPUs.

[akien-mga]

I reported it upstream: GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator#245

[clayjohn]

Looks like the fix was merged upstream and we now include that fix in Godot! GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator#245