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

[msbuild] Minor refactor + VulkanSDK 1.2.135.0+ build fix #8169

Merged
merged 5 commits into from May 10, 2020

Conversation

ZeeWanderer
Copy link
Contributor

@ZeeWanderer ZeeWanderer commented May 7, 2020

  • cmake cmd line extracted to a variable in spirv and glslang msbuild projects
  • decreased VULKAN_SDK lib dir priority to avoid linking provided libs instead of rpcs3-built ones because VulkanSDK 1.2.135.0+ now comes with
glslang.lib
HLSL.lib
OGLCompiler.lib
OSDependent.lib
shaderc.lib
shaderc_combined.lib
shaderc_shared.lib
shaderc_util.lib
SPIRV.lib
spirv-cross-c.lib
spirv-cross-core.lib
spirv-cross-cpp.lib
spirv-cross-c-shared.lib
spirv-cross-glsl.lib
spirv-cross-hlsl.lib
spirv-cross-msl.lib
spirv-cross-reflect.lib
spirv-cross-util.lib
SPIRV-Tools.lib
SPIRV-Tools-link.lib
SPIRV-Tools-opt.lib
SPIRV-Tools-reduce.lib
SPIRV-Tools-shared.lib
SPVRemapper.lib
VkLayer_utils.lib
vulkan-1.lib

VulkanSDK 1.2.135.0+ comes with a lot more libs including glslang.lib and SPIRV*.lib. This allows to load all rpcs built libs first.
Same change as was made for llvm_build
@AniLeo AniLeo requested a review from kd-11 May 7, 2020 13:20
@AniLeo
Copy link
Member

AniLeo commented May 7, 2020

Better to update SDK version used in CI too

@ZeeWanderer
Copy link
Contributor Author

ZeeWanderer commented May 7, 2020

This change allows to build with 1.2.135.0 sdk version and should not cause problems for older versions. But ok sure.
EDIT: not sure how to do that for Linux build

@ZeeWanderer ZeeWanderer marked this pull request as ready for review May 7, 2020 21:33
@JohnHolmesII
Copy link
Contributor

You would need to pr the number to here:
https://github.com/hcorion/rpcs3-docker/blob/master/xenial/Dockerfile#L47

Copy link
Contributor

@kd-11 kd-11 left a comment

Choose a reason for hiding this comment

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

Looks fine?? Maybe. The inner workings of visual studio files remains largely a mystery to me, but the reasoning is sound. I have only one suggestion, with spirv opt library shipping with the SDK, we don't need to force build the spirv-tools-build project, making it an optional rebuild like llvm or glslang.

@ZeeWanderer
Copy link
Contributor Author

ZeeWanderer commented May 8, 2020

@kd-11 spiv-tools and glslang libs provided by VulkanSDK use MD runtime which is strange since it seems like vulkan-1 is statically linked to runtime or at least it does not contain /FAILIFMISMATCH:RuntimeLibrary=MD_DynamicRelease directive. Runtime mismatch error is the reason i noticed the wrong linkage in the first place.

Facepalm, right vulkan-1.lib is pure C so linker has no need to check runtime linkage.

@kd-11
Copy link
Contributor

kd-11 commented May 8, 2020

Ok, then, I suppose current setup will do.

@AniLeo AniLeo merged commit 5dc8b05 into RPCS3:master May 10, 2020
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

Successfully merging this pull request may close these issues.

None yet

4 participants