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

swiftshader: init at 2020-03-31 #83874

Merged
merged 1 commit into from Jun 18, 2020
Merged

Conversation

expipiplus1
Copy link
Contributor

Motivation for this change

Although swiftshader is used as part of Chromium, it's not available as a
standalone package.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/) (Tested libvulkan.so.1)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@expipiplus1
Copy link
Contributor Author

CC @Ralith

@doronbehar
Copy link
Contributor

Hey I'm currently testing the build of this, it takes very long mostly because it compiles all of *!@^ LLVM. Do you think @expipiplus1 you can make it use Nixpkgs' version of LLVM?

@doronbehar
Copy link
Contributor

Besides that build went out fine.

@Ralith
Copy link
Contributor

Ralith commented Mar 31, 2020

It would definitely be nice to try to strip out as much of third_party as possible, though the extent to which that's feasible with Google projects like this varies wildly.

Can someone who's had the time to build this paste a list of the output files, and the contents of the manifest .json it installs, if any?

@expipiplus1
Copy link
Contributor Author

Hey I'm currently testing the build of this, it takes very long mostly because it compiles all of *!@^ LLVM. Do you think @expipiplus1 you can make it use Nixpkgs' version of LLVM?

Yikes, I didn't realise!

After a quick search for LLVM in CMakeLists.txt it doesn't look as though it's trivial to use an external LLVM.

@expipiplus1
Copy link
Contributor Author

@Ralith Here is a list of everything in the build directory after building: https://gist.github.com/cc0eddaeb1c74195d12d3275d02febe2

@Ralith
Copy link
Contributor

Ralith commented Apr 1, 2020

Rather, I'm interested in the contents of the resulting nix package (and of vk_swiftshader_icd.json).

@expipiplus1
Copy link
Contributor Author

@Ralith ah sorry, here it is

result/
result/lib
result/lib/vk_swiftshader_icd.json
result/lib/libvulkan.so
result/lib/libEGL.so
result/lib/libGLESv2.so
result/lib/libvk_swiftshader.so
result/lib/libvulkan.so.1

result/lib/vk_swiftshader_icd.json

{
  "file_format_version": "1.0.0",
  "ICD": {
    "library_path": "./libvk_swiftshader.so",
    "api_version": "1.0.5"
  }
}

@Ralith
Copy link
Contributor

Ralith commented Apr 1, 2020

For this to play nicely in a larger environment, it'll need some changes:

  • The .json manifest should be installed in share/vulkan/icd.d
  • The .json manifest's "library_path" field should reference the full absolute path to libvk_swiftshader.so
  • libvulkan.so and .so.1 should probably be omitted entirely; NixOS already packages the Vulkan loader separately.

I expect something similar is necessary for the EGL library, but I'm not an expert on how glvnd works. See e.g. ${mesa.drivers}/share/glvnd/egl_vendor.d/50_mesa.json.

These changes should make it possible for swiftshader to exist as a first-class graphics device on a NixOS system alongside any actual hardware, and if upstreamed would likely be similarly useful to other distros. Otherwise, it will conflict with the regular graphics infrastructure.

@expipiplus1
Copy link
Contributor Author

@Ralith, I've changed things as you suggest, as well as copying what seems to be going on for glvnd.

I've also raised an issue on the swiftshader tracker about using an external LLVM: https://bugs.chromium.org/p/swiftshader/issues/detail?id=146

@c0d1f1ed
Copy link

c0d1f1ed commented Apr 1, 2020

Hi all, I'm the lead of SwiftShader. Thanks for your interest in providing it as a package in NixOS!

As responded on https://bugs.chromium.org/p/swiftshader/issues/detail?id=146, we can't guarantee that building it with an external LLVM library will result in correct behavior.

The libvulkan.so.1 binary is just a renamed copy of libvk_swiftshader.so. The latter is to be used as an ICD, while the former can be used on systems that lack a Vulkan Loader or for which we wish to skip it. That should probably only be done for testing purposes, so it can be left out of the package.

@expipiplus1
Copy link
Contributor Author

Unfortunately building with an external LLVM package is not desirable. We make changes to our fork of LLVM when necessary. For example LLVM 7 had bugs in its coroutine support. We use that to implement control barriers. So using upstream LLVM 7 instead could produce subtle breakages (we did contribute the patch back to LLVM, but I'm not sure if that made it into the 7 release, and we can't guarantee that in the future). We also minimize the build size and alter the LLVM build config from the default, so using an external build instead may produce a larger library with additional dependencies that can cause issues which don't occur with our build.

As responded on https://bugs.chromium.org/p/swiftshader/issues/detail?id=146, we can't guarantee that building it with an external LLVM library will result in correct behavior.

I think given this, along with the fact that building with the in-tree LLVM was so easy we should keep it as the swiftshader team suggests.

The libvulkan.so.1 binary is just a renamed copy of libvk_swiftshader.so. The latter is to be used as an ICD, while the former can be used on systems that lack a Vulkan Loader or for which we wish to skip it. That should probably only be done for testing purposes, so it can be left out of the package.

Done.

@Ralith and @doronbehar, if you agree with the above, I suppose all that's left would be to test using swiftshader as a graphics device for NixOS, or perhaps that could wait for a separate PR.

@doronbehar
Copy link
Contributor

think given this, along with the fact that building with the in-tree LLVM was so easy we should keep it as the swiftshader team suggests.

Agreed.

Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Haven't the build again but LGTM.

@Ralith
Copy link
Contributor

Ralith commented Apr 2, 2020

Changes look good; will try to test the vulkan aspects myself.

@Ralith
Copy link
Contributor

Ralith commented Apr 2, 2020

Running vulkaninfo against this ICD results in a weird crash:

VK_ICD_FILENAMES=/nix/store/fxnhndsv0accg8z0sy6mii0zzjab6dv1-swiftshader-2020-03-31/share/vulkan/icd.d/vk_swiftshader_icd.json /nix/store/7bppi7hhkhn1c7jsip2a33kmdr9kc0kj-vulkan-tools-1.2.131.1/bin/vulkaninfo
<snip>
GPU id : 0 (SwiftShader Device (LLVM 7.0.1)):
	Surface types: count = 1
		VK_KHR_xlib_surface
	Formats: count = 2
		SurfaceFormat[0]:
			format = FORMAT_B8G8R8A8_UNORM
			colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
		SurfaceFormat[1]:
			format = FORMAT_B8G8R8A8_SRGB
			colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
	Present Modes: count = 2
		PRESENT_MODE_FIFO_KHR
		PRESENT_MODE_MAILBOX_KHR
	VkSurfaceCapabilitiesKHR:
	-------------------------
		minImageCount       = 1
		maxImageCount       = 0
		currentExtent:
			width  = 256
			height = 256
		minImageExtent:
			width  = 256
			height = 256
		maxImageExtent:
			width  = 256
			height = 256
		maxImageArrayLayers = 1
		supportedTransforms:
			SURFACE_TRANSFORM_IDENTITY_BIT_KHR
		currentTransform:
			SURFACE_TRANSFORM_IDENTITY_BIT_KHR
		supportedCompositeAlpha:
			COMPOSITE_ALPHA_OPAQUE_BIT_KHR
		supportedUsageFlags:
			IMAGE_USAGE_TRANSFER_SRC_BIT
			IMAGE_USAGE_TRANSFER_DST_BIT
			IMAGE_USAGE_SAMPLED_BIT
			IMAGE_USAGE_COLOR_ATTACHMENT_BIT
	VkSurfaceCapabilities2EXT:
	--------------------------
		supportedSurfaceCounters:
zsh: segmentation fault  VK_ICD_FILENAMES= 

Running in gdb produces instead:

/build/SwiftShader-5cf1e9a/src/Vulkan/libVulkan.cpp:2937 vkGetPhysicalDeviceProperties2 TRACE_ASSERT: pProperties->pNext sType = 1000148001
Program received signal SIGABRT, Aborted.
0x00007ffff78ec15a in raise () from /nix/store/9rabxvqbv0vgjmydiv59wkz768b5fmbc-glibc-2.30/lib/libc.so.6
(gdb) bt
#0  0x00007ffff78ec15a in raise () from /nix/store/9rabxvqbv0vgjmydiv59wkz768b5fmbc-glibc-2.30/lib/libc.so.6
#1  0x00007ffff78d6548 in abort () from /nix/store/9rabxvqbv0vgjmydiv59wkz768b5fmbc-glibc-2.30/lib/libc.so.6
#2  0x00007ffff691e0d2 in ?? () from /nix/store/fxnhndsv0accg8z0sy6mii0zzjab6dv1-swiftshader-2020-03-31/lib/libvk_swiftshader.so
#3  0x00007ffff693728b in vkGetPhysicalDeviceProperties2 () from /nix/store/fxnhndsv0accg8z0sy6mii0zzjab6dv1-swiftshader-2020-03-31/lib/libvk_swiftshader.so
#4  0x0000000000457f45 in AppGpu::AppGpu(AppInstance&, unsigned int, VkPhysicalDevice_T*, pNextChainInfos) ()
#5  0x000000000040f067 in main ()

@expipiplus1
Copy link
Contributor Author

expipiplus1 commented Apr 3, 2020

I get no segfault calling getPhysicalDeviceProperties with all possible types in the struct chain: https://gist.github.com/9a1e461a8b1a76a25af0cd0da99f2ae4; Perhaps vulkaninfo is doing something out of the ordinary (seems unlikely though)?

I do however get an error when the program terminates: This is certainly because of printing unprintable characters from the UUIDs and nothing to do with swiftshader.

$ VK_ICD_FILENAMES=/home/j/src/nixpkgs2/result/share/vulkan/icd.d/vk_swiftshader_icd.json cabal exec info > properties
waitForProcess: illegal operation (Inappropriate ioctl for device)

FWIW hello triangle worked out of the box:

image

@expipiplus1
Copy link
Contributor Author

expipiplus1 commented Apr 8, 2020

@Ralith This looks like a problem in (at least) vulkaninfo. running under gdb without the code that aborts when under a debugger (in log_trap) shows the crash takes place in vulkaninfo:

Thread 1 "vulkaninfo" received signal SIGSEGV, Segmentation fault.
0x000000000042d71f in chain_iterator_surface_capabilities2(Printer&, AppInstance&, AppGpu&, void*, VulkanVersion) ()
(gdb) bt
#0  0x000000000042d71f in chain_iterator_surface_capabilities2(Printer&, AppInstance&, AppGpu&, void*, VulkanVersion) ()
#1  0x0000000000433b38 in DumpSurfaceCapabilities(Printer&, AppInstance&, AppGpu&, AppSurface&) ()
#2  0x000000000043b723 in DumpSurface(Printer&, AppInstance&, AppGpu&, AppSurface&, std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >) ()
#3  0x000000000043f3c1 in DumpPresentableSurfaces(Printer&, AppInstance&, std::vector<std::unique_ptr<AppGpu, std::default_delete<AppGpu> >, std::allocator<std::unique_ptr<AppGpu, std::default_delete<AppGpu> > > > const&, std::vector<std::unique_ptr<AppSurface, std::default_delete<AppSurface> >, std::allocator<std::unique_ptr<AppSurface, std::default_delete<AppSurface> > > > const&) ()
#4  0x000000000040f899 in main ()

@c0d1f1ed Do you agree? would it be best to open an issue against swiftshader or vulkan-tools?

@Ralith
Copy link
Contributor

Ralith commented Apr 8, 2020

That looks like a different crash than the one I observed. Note the presence of libvk_swiftshader.so in my backtrace. Oddly the crash doesn't seem to occur when I install swiftshader to my user profile so both it and my intel GPU are visible; only when I set VK_ICD_FILENAMES in the environment to force only swiftshader to be visible do things go awry. vkcube works fine regardless.

I think these are swiftshader bugs and probably don't need to block the merge of the package, particularly as actual graphics applications seem to work fine, and installation for graceful discovery by regular vulkan apps seems to work great.

@expipiplus1
Copy link
Contributor Author

That looks like a different crash than the one I observed. Note the presence of libvk_swiftshader.so in my backtrace. Oddly the crash doesn't seem to occur when I install swiftshader to my user profile so both it and my intel GPU are visible; only when I set VK_ICD_FILENAMES in the environment to force only swiftshader to be visible do things go awry. vkcube works fine regardless.

Yeah, because I removed an abort() in swiftshader (which was only present when running under a debugger.

I think these are swiftshader bugs and probably don't need to block the merge of the package, particularly as actual graphics applications seem to work fine, and installation for graceful discovery by regular vulkan apps seems to work great.

Agree

@expipiplus1
Copy link
Contributor Author

Is it possible to get this merged, please?

@expipiplus1
Copy link
Contributor Author

  • update to 2020-04-17

@expipiplus1
Copy link
Contributor Author

Is it possible to get this merged, please?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/143

@expipiplus1
Copy link
Contributor Author

PIng

@expipiplus1
Copy link
Contributor Author

2020-04-17 -> 2020-06-17

@Lassulus
Copy link
Member

hi, sorry for the delay. there are so many PRs and not enough people to review/merge everything.

while trying to build this I got this error though:

hash mismatch in fixed-output derivation '/nix/store/rjm7jr4lk3aq3daxvizhcglpc49mgnyw-SwiftShader-763957e':
  wanted: sha256:0mp77cp85mbs83wpwqhn3myzdfdid8jdq2mxf65cchb643amdsr3
  got:    sha256:0sdh48swx0qyq2nfkv1nggs14am0qc7z239qrxb69p2ddqm76g1s
cannot build derivation '/nix/store/jpw1yrmdwig3vg04vkin0lrb8c36yk5y-swiftshader-2020-06-17.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/fh8280hdfrcxxxpwxal3higzpbarvdar-env.drv': 1 dependencies couldn't be built

could you maybe squash the commits into one also?

cheers and thanks for contributing

@expipiplus1
Copy link
Contributor Author

hi, sorry for the delay. there are so many PRs and not enough people to review/merge everything.

No problem at all, I completely understand. Please don't take my pings as impatience!

while trying to build this I got this error though:
...
could you maybe squash the commits into one also?

Done!

cheers and thanks for contributing

:)

src = fetchgit {
url = "https://swiftshader.googlesource.com/SwiftShader";
rev = "763957e6b4fc1aa360ab19c4109b8b26686783e8";
sha256 = "0mp77cp85mbs83wpwqhn3myzdfdid8jdq2mxf65cchb643amdsr3";
Copy link
Member

Choose a reason for hiding this comment

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

still seems to be a wrong hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, fixed properly this time

@Lassulus Lassulus merged commit eed368a into NixOS:master Jun 18, 2020
@expipiplus1
Copy link
Contributor Author

expipiplus1 commented Jun 19, 2020 via email

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

Successfully merging this pull request may close these issues.

None yet

6 participants