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

Jump to bottom

Fix build on FreeBSD i386 - nullptr vs VK_NULL_HANDLE #9138

Merged
merged 1 commit into from Nov 3, 2020
Merged

Fix build on FreeBSD i386 - nullptr vs VK_NULL_HANDLE #9138

merged 1 commit into from Nov 3, 2020

Conversation

martymac
Copy link
Contributor

@martymac martymac commented Oct 7, 2020

Hello,

I am getting errors on FreeBSD i386 (12.1-RELEASE/LLVM 8.0.1) when trying to build Dolphin 5.0.12716:

/usr/bin/c++ -DDATA_DIR=\"/usr/local/share/dolphin-emu/\" -DFMT_SHARED -DHAVE_EGL=1 -DHAVE_FFMPEG -DHAVE_X11=1 -DHAVE_XRANDR=1 -DUSE_ANALYTICS=1 -DUSE_MEMORYWATCHER=1 -DUSE_PIPES=1 -DUSE_UPNP -D_ARCH_32=1 -D_DEFAULT_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_M_GENERIC=1 -D__LIBUSB__ -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Source/Core -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/enet/include -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/External/minizip -I/usr/local/include/libpng16 -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/soundtouch -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/picojson -ISource/Core -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/Vulkan/Include -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/minizip/. -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/cubeb/include -Iexports -I/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/xxhash -isystem /usr/local/include -isystem /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/glslang/StandAlone -isystem /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/glslang/glslang/Public -isystem /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/glslang/SPIRV -isystem /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/glslang -isystem /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Externals/rangeset/include -isystem /usr/local/include/hidapi -O2 -pipe -DLIBICONV_PLUG -fstack-protector-strong -fno-strict-aliasing  -DLZO_CFG_PREFER_TYPEOF_ACC_INT32E_T=LZO_TYPEOF_INT -DLIBICONV_PLUG -O2 -pipe -DLIBICONV_PLUG -fstack-protector-strong -fno-strict-aliasing  -DLZO_CFG_PREFER_TYPEOF_ACC_INT32E_T=LZO_TYPEOF_INT -DLIBICONV_PLUG -fdiagnostics-color -Wall -Wtype-limits -Wsign-compare -Wignored-qualifiers -Wuninitialized -Wshadow -Winit-self -Wmissing-declarations -Wmissing-variable-declarations -fno-strict-aliasing -fno-exceptions -fvisibility-inlines-hidden -fvisibility=hidden -fomit-frame-pointer -std=c++17 -MD -MT Source/Core/VideoBackends/Vulkan/CMakeFiles/videovulkan.dir/ObjectCache.cpp.o -MF Source/Core/VideoBackends/Vulkan/CMakeFiles/videovulkan.dir/ObjectCache.cpp.o.d -o Source/Core/VideoBackends/Vulkan/CMakeFiles/videovulkan.dir/ObjectCache.cpp.o -c /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Source/Core/VideoBackends/Vulkan/ObjectCache.cpp
In file included from /wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Source/Core/VideoBackends/Vulkan/ObjectCache.cpp:21:
/wrkdirs/usr/ports/emulators/dolphin-emu/work/dolphin-3152428/Source/Core/VideoBackends/Vulkan/VKTexture.h:57:51: error: invalid operands to binary expression ('const VkDeviceMemory' (aka 'const unsigned long long') and 'nullptr_t')
  bool IsAdopted() const { return m_device_memory != nullptr; }
                                  ~~~~~~~~~~~~~~~ ^  ~~~~~~~
1 error generated.
ninja: build stopped: subcommand failed.
*** Error code 1

Stop.

This PR re-enables build for i386 (while still maintaining it buildable on amd64).

Warnings :

  • I could not test the resulting Vulkan backend
  • I am not sure this is the best way to fix that (comments welcome...)

Best regards,

Ganael.

stenzek
stenzek reviewed Oct 15, 2020
View reviewed changes
Externals/Vulkan/Include/vulkan/vulkan_core.h Outdated
@@ -47,7 +47,11 @@ extern "C" {
#define VK_HEADER_VERSION 121


#if defined(__LP64__) || defined(_WIN64) || (defined(__x86_64__) && !defined(__ILP32__) ) || defined(_M_X64) || defined(__ia64) || defined (_M_IA64) || defined(__aarch64__) || defined(__powerpc64__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? 0 should work for both u64s and pointer types for comparison.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is modifying an externel header, this can also break the build if the headers are ever updated in the future (as new versions of the header won't contain this change).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's probably not a good idea to have a local version of external code. I'll push an updated version of the patch not modifying Externals/.

@martymac
Copy link
Contributor Author

Hello,

Do those new changes seem reasonable now ? Unfortunately, I am neither a C++ expert, nor a Vulkan dev, so I may be totally wrong. I am just looking for a way to fix those build errors on i386. Any comment welcome !

Thanks in advance,
Ganael.

@martymac
Copy link
Contributor Author

martymac commented Nov 2, 2020

Hello,

Any comment on that new version of the patch ?

@jordan-woyak
Copy link
Member

Squash the commits then this looks good to me.

@martymac
Copy link
Contributor Author

martymac commented Nov 2, 2020

Done, thanks!

@jordan-woyak
Copy link
Member

The commit message has a typo, "Instanciate".
Maybe just change the message to:
"Resolve VkDeviceMemory/nullptr type mismatch to fix build on FreeBSD."

@martymac
Copy link
Contributor Author

martymac commented Nov 3, 2020 •
edited

Commit message modified, thanks!

@jordan-woyak jordan-woyak merged commit d2f80a4 into dolphin-emu:master Nov 3, 2020
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lioncash lioncash lioncash left review comments

@stenzek stenzek stenzek left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
4 participants
@martymac @jordan-woyak @lioncash @stenzek