Vulkan 1.4 - and lots of clean ups - #62
Open
SimonDanisch wants to merge 20 commits into
Open
SimonDanisch wants to merge 20 commits into
SimonDanisch wants to merge 20 commits into
Conversation
Two-call enumerations allocated their output array with
`Vector{T}(undef, n)` and handed it straight to the driver. For an
sType-bearing element type that is wrong: the caller must set `sType` (and
`pNext`) on every element before the call, because the API reads them as
input. The driver was reading uninitialized memory.
Validation catches it, e.g. against
vkGetPhysicalDeviceCooperativeMatrixPropertiesKHR:
pProperties[0].sType must be VK_STRUCTURE_TYPE_COOPERATIVE_MATRIX_PROPERTIES_KHR
pProperties[0].pNext must be NULL
`initialize_array` now emits `fill(initialize_core(T, []), n)` whenever the
element type has a `structure_type` method, reusing the helper that already
zeroes the struct (giving pNext = NULL) and stamps sType. Gated on
`api.structure_types`, the same test `wrap/call.jl` uses.
This is systemic rather than coopmat-specific: 58 enumerations per platform
were affected, including get_physical_device_queue_family_properties_2. The
generated/ files are the regenerated output, not hand edits.
Clang.jl emits a struct containing a union as an opaque `NTuple{N,UInt8}` blob
with pointer accessors and one explicitly typed constructor, rather than the
usual field-wise struct. Julia converts arguments at field assignment but never
to make a method match, so passing the high-level wrapper types to an opaque
constructor is a hard MethodError — even though every conversion exists.
`Vulkan.ImageLayout` is not `VkImageLayout`, it merely converts to one.
`_RenderingAttachmentInfo` passes its arguments straight through, so every
dynamic-rendering call died at construction:
MethodError: no method matching VkRenderingAttachmentInfo(::VkStructureType,
::Ptr{Nothing}, ::ImageView, ::ImageLayout, ::VkResolveModeFlagBits, ...)
Field-wise structs like VkImageViewCreateInfo are unaffected, which is why
everything else works and only the union-bearing ones break. 122 structs are
opaque in the Windows bindings; this covers the one that is actually
constructed, and the comment says how to spot the next.
Found by Lava's test suite, where it accounted for ~112 errors: the MethodErrors
fire mid command-buffer recording, which loses the device, after which every
allocation fails with "device is lost". Lava's test_graphics_pipeline.jl goes
16/16 with this.
The general fix belongs in the generator — wrapping every field argument in
`convert(<field type>, ...)` is correct for opaque structs and a no-op for
field-wise ones — but that needs per-field raw types threaded through
wrap/structs.jl, so it is noted rather than done here.
PhysicalDeviceMemoryProperties no longer has memory_type_count /
memory_heap_count — the high-level wrapper truncates memory_types and
memory_heaps to the real counts instead.
Two sites still read them:
* show.jl — showing the struct threw
`FieldError: no field memory_type_count`. Anything that displays one throws
instead of printing: a REPL inspection, an error message, and in particular a
failing @test whose values reach a VkContext, where Test.jl's own failure
printing turns a plain Fail into a confusing Error pointing at the wrong
thing. That is how this was found.
* precompile_workload.jl — same field in the buffer memory-type search, so the
workload would throw wherever it runs.
Both now derive the count from the vector length.
`get_pipeline_executable_statistics_khr` never returned anything: it builds the
result array with `_initialize_core`, which patches `sType`/`pNext` through
`ConstructionBase.setproperties`, and that refuses any struct with overloaded
`propertynames`. `VkPipelineExecutableStatisticKHR` has them because of its C
union, so every call failed before returning a single statistic.
Right to refuse — its only field is `data::NTuple{544,UInt8}`, so a field-wise
rebuild would write the patch into the wrong bytes. But the struct does know
where its fields live: Clang.jl emits `setproperty!` on a `Ptr{T}` with the
correct offsets, and writing through a `Ref` uses exactly those.
Goes in `opaque_struct_ctors.jl`, which already exists for the same union-blob
shape in the other direction (constructors that cannot take high-level types).
ConstructionBase becomes a direct dependency: `setproperties` was only imported
from Accessors, and extending needs the defining module.
With this, the driver reports Register Count, Binary Size, Stack Size, Local and
Shared Memory per compute pipeline — used to rule out a resource limit as the
cause of Lava's workgroup-size truncation.
The companion to 9c4a25b. That commit fixed CONSTRUCTING a Clang-opaque struct (the blob-with-a-union shape, where Julia will not convert arguments to make a method match). This is the same shape failing on the way out. `propertynames` for a blob struct lists the LOGICAL fields while `fieldnames` is the single `data`, and `ConstructionBase.setproperties` refuses any type whose `propertynames` is overloaded -- correctly, since it cannot know the mapping: The `VkPipelineExecutableStatisticKHR` type defines custom properties ... Please define `ConstructionBase.setproperties(...)` to set its properties. `_initialize_core` sets sType/pNext through exactly that call, so every API returning one of these 46 structs threw inside the wrapper before reaching the driver -- on every vendor, every platform. Found through `get_pipeline_executable_statistics_khr`, where it presented as "AMD reports no pipeline statistics" and was nearly written up as a driver limitation. RADV in fact returns TWENTY-SIX statistics per compute pipeline -- VGPRs, SGPRs, spills, LDS size, scratch, subgroups per SIMD, VALU/SALU/VMEM counts, latency, inverse throughput -- considerably more than the NVIDIA numbers the consuming code was written against. Generated for every blob-shaped struct rather than the ones used today: unlike the constructors above it needs no per-struct knowledge, and a struct that grows a union in a later header would otherwise reintroduce the same silence. Deduplicated by TYPE, not by name: `names(...; all = true)` reaches several of these under an alias as well as their own name, and defining the same method twice is a hard error during precompilation ("Method overwriting is not permitted"), not a warning -- the first version of this commit broke `using Lava` outright. ConstructionBase is reached through Accessors, which already depends on it, so this needs no new dependency and no manifest change.
The rule was `*.jl.mem`, which matches nothing --track-allocation=user produces — the profiler writes `linux.jl.1678653.mem`, with the pid between the extension and .mem. Four such files were sitting untracked in generated/. Same wrong pattern was in JuliaVision (fixed) and is why profiling output reached two commits today before anyone noticed.
__init__ called fill_dispatch_table unconditionally, which asks vkGetInstanceProcAddr for every core entry point and so dlopens libvulkan. On a machine without one that threw from module init, which means the package could not be precompiled, which means merely DEPENDING on it was fatal — on a Mac, every `using` that triggered an env precompile died on Vulkan and its five extensions, and took Lava with it. VulkanCore already answers that question rather than failing on it (`VkCore.loaded()`); this is the same answer one level up. With a loader present nothing changes: it is the call it always was. Without one the dispatch table stays empty and function_pointer says which of the two reasons a pointer is missing — no loader, or an extension not enabled. The precompile workload already guarded itself on find_library, so __init__ was the only thing left. Verified on macOS with no libvulkan: `using Vulkan` loads, `VkCore.loaded()` is false, and `using Metal, RayMakie` now succeeds on the FIRST try with a cold cache where it used to error.
A package that cannot work on this machine should not cost anything to depend
on. VulkanCore compiled 34,000 generated lines of ccall wrappers, Vulkan
127,000 more plus a dispatch table and an 8,573-name export list, and Lava a
whole Julia->SPIR-V compiler — all on every Mac in this tree, for code Mantle
declares and, under its own `@static if Sys.isapple()`, never imports.
VulkanCore probes the loader once at precompile time and exposes the answer as
HAS_LOADER; Vulkan and Lava gate their bodies on it, and Vulkan re-exposes it
outside its own gate so a dependent can ask without first checking whether
there is anything to ask. The extensions are gated too — Vk.Format does not
exist when the bindings do not, and those were the last thing still failing.
The gated text is UNCHANGED and unindented on purpose: with a loader present
each module is byte-for-byte the one it was, so the only case this can break is
the empty one.
Measured on macOS with no libvulkan: VulkanCore 3670 -> 554 ms, Vulkan 13098 ->
363 ms, and Lava now precompiles at all — gated out, it no longer evaluates the
`NativeEmitter{Out, Flats}` its body wants and this KernelAbstractions checkout
does not have. All three load in 0.4 s. `using Metal, RayMakie` takes 3.1 s
from a cold RayMakie cache, where it used to error.
A precompile-time answer, so installing a driver later needs
Pkg.precompile(; force = true).
NOT verified with a loader present — there is none on this machine.
Two bugs, both reaching code the wrapper generates for every platform.
A struct Clang emits as an opaque blob -- one holding a union or a bitfield --
gets a single explicitly typed constructor, and Julia does not convert arguments
to make a method match. The generated wrapper handed it the high-level `Format`,
`IndexType` or handle its own signature asks for, so constructing one was a
MethodError: 23 structs the wrapper builds, among them every acceleration
structure geometry and build info, the micromap and cluster builds,
`VkDescriptorGetInfoEXT` and `VkRenderingAttachmentInfo`. `vk_ctor_call` now
converts each member to its raw type, skipping only the expressions that are
already exactly that type.
Every member rather than the blob-shaped ones, because which structs Clang makes
opaque cannot be read off the specification: it is opaque for bitfields too, and
the spec models `VkAccelerationStructureInstanceKHR`'s packed fields as plain
UInt32 members -- a union closure over the spec misses that struct and six
others. Nothing rests on classifying the struct.
`raw_julia_type` wraps NTuple lengths in `Int`: the spec gives an array length as
the C constant naming it and those are Cuint, so `NTuple{VK_UUID_SIZE, UInt8}` is
a TypeError. Function pointers are left alone, since the nine `PFN_*` typedefs
are VulkanCore names the wrapper never brings into scope.
Second, a union whose alternatives are pointers. `idiomatic_julia_type` maps
`Ptr{VkX}` to the wrapper struct, giving 18 constructors a signature promising a
value the body then hands to a constructor that stores a pointer.
`VkIndirectCommandsTokenDataEXT`, `VkIndirectExecutionSetInfoEXT` and
`VkClusterAccelerationStructureOpInputNV`, all new in 1.4. They take the pointer
now, as `VkDescriptorDataEXT` already did by hand: a union wrapper has one field
and no `deps`, so the caller owns the pointee.
Ten expectations re-pinned and two regression tests added, both confirmed
failing before the fix.
Output of the generator change before this one. 1087 lines per platform, the same change in each. Also replaces a hand-edit that had been made directly in generated/linux.jl -- a `convert` on the token type of `_IndirectCommandsLayoutTokenEXT` -- which fixed one struct on one platform and would have been lost on the next regeneration; the generator now emits exactly that line, and the same treatment for the other 22. Verified reproducible: regenerating on top of this commit gives all four files byte for byte.
Breaking: 1.4 removed PhysicalDeviceMemoryProperties' memory_type_count and
memory_heap_count, so memory_types and memory_heaps are truncated to the real
counts and `length` is the way to ask.
`test/generated_scope.jl` parses the generated wrapper and checks that every type
named in an emitted `convert` resolves inside `Vulkan`. Those names appear inside
function bodies, so one that does not resolve is an UndefVarError at call time
rather than at precompilation, and hides until someone constructs that one
struct. `convert(PFN_vkDebugUtilsMessengerCallbackEXT, ...)` is how that was
found. Parsed rather than grepped, so `convert(T::Type{X}, x)` definitions and
the commas in `NTuple{2, UInt32}` cannot be mistaken for call arguments.
`opaque_struct_ctors.jl` loses its permissive VkRenderingAttachmentInfo
constructor: the generator covers that struct and the other 22 now. The
setproperties half stays, being the same shape failing on the way out, which the
generator cannot reach.
The high-level struct drops length members because the count is recoverable: a pointer array becomes a Vector and `length` gives it back. A fixed-size array carries no such information, and the 1.4 registry annotates `memoryTypes` with len="memoryTypeCount", which made the count a length member for the first time. The result was a `PhysicalDeviceMemoryProperties` reporting 32 memory types on a device that has 11, the remaining 21 zeroed and indistinguishable from real entries, with the count gone from the struct and no way left to ask for it. Measured on a 7900 XTX: 11 real types, 2 real heaps, `length(memory_types)` 32. `PhysicalDeviceGroupProperties` and `QueueFamilyGlobalPriorityProperties` lost their counts the same way. `counts_a_fixed_array` distinguishes the two cases, and the four members it matches keep their counts. 21 regenerated lines per platform. This is also what two of the generator's own tests have been reporting since the 1.4 update: `codegen/structs/high_level.jl` pins the form WITH the counts and has been failing. The suite goes from 8 failures to 6. Reverts the two workarounds from "Fix two uses of the memory-property count fields removed in vk1.4". The premise there was wrong -- 1.4 removes neither field from `VkPhysicalDeviceMemoryProperties`, they are still in the headers, and this wrapper had dropped them. Deriving the count from the array length instead gave 32 and 16 rather than 11 and 2, so `show` printed every zeroed entry.
`LibVulkan` exports by prefix -- `VK_`, `Vk`, `vk`, `StdVideo`, `STD_VIDEO` and nothing else -- so a type outside that set is not reachable through `using .vk` and has to be written `vk.X`. Otherwise it is an UndefVarError raised when the constructor runs rather than when the package precompiles, and it hides until someone builds that one struct. Converting members at the construction site made the wrapper NAME those types for the first time, and on macOS it named six it cannot reach: `MTLDevice_id`, `MTLBuffer_id`, `MTLTexture_id`, `MTLSharedEvent_id`, `MTLCommandQueue_id` and `IOSurfaceRef`, the Metal object types behind VK_EXT_metal_objects. Ten constructors, all of `VK_EXT_metal_objects`. `raw_julia_type` qualifies them now; 10 regenerated lines, macos.jl only, the other three byte-identical. The equivalent Windows and X11 types are unaffected because they are in VulkanSpec's `extension_types` and reach the constructor through `unsafe_convert`, which already names the type itself. `test/generated_scope.jl` checks all FOUR platform files rather than the host's. Its earlier version checked only the host, which is why this reached a commit: no machine here runs macos.jl, and no CI runner does either. It fails with the six names before this change.
Optional members default to the integer 0, and the wrapper turns a flag into its raw Vk*FlagBits by reading `.val` -- which an integer does not have. So `_RenderingAttachmentInfo` threw `FieldError: type Int64 has no field val` unless the caller named `resolve_mode`, and nineteen flag members have such a default. Found by constructing one on Windows. `flag_value` takes the integer and the BitMask alike. By dispatch rather than `UInt32(x)`: that is right for all nineteen today and would silently truncate the first 64-bit flag to reach this path. Verified on Linux and on Windows, 126 tests each. The regression test builds a `_RenderingAttachmentInfo` both ways, and the emission it pins was `VkResolveModeFlagBits(resolve_mode.val)` before.
Two changes, both about using a Vulkan implementation directly. `vkGetDeviceProcAddr` is the one entry point the dispatch table has to bootstrap, and it was fetched from the library BY NAME. A driver on its own does not export it: SwiftShader, lavapipe and MoltenVK export `vkGetInstanceProcAddr` and `vk_icdGetInstanceProcAddr` and nothing else, because supplying the rest is the Khronos loader's job. So every device function pointer failed with `could not load symbol "vkGetDeviceProcAddr"` against any driver used without a loader -- which is how MoltenVK is meant to be used on macOS. It is asked of the instance now, through `vkGetInstanceProcAddr`, which an implementation must export; it is resolved once and kept in the instance's table, whose lifetime already matches. Measured against SwiftShader on macOS with no loader: 83 of 89 tests before, 124 of 126 after. The two that remain are the public `get_device_proc_addr`, which still asks by name, and validation layers, which only a loader can provide. `set_driver` moves OUT of the `HAS_LOADER` gate. Everything else here is compiled only when there is something to bind to, but this is what a user reaches for when there is NOT -- inside the gate, the one escape hatch did not exist on a machine with no driver. It writes a VulkanCore preference rather than an environment variable, so the choice is part of the precompile hash and takes effect on the next load. It knows `:SwiftShader`, `:Lavapipe`, `:Loader` and `:System`, and resolves each through the active environment by NAME rather than a hard-coded UUID, since `Lavapipe_jll` may be a local build. Linux, real hardware: VulkanCore green, Vulkan 126 of 126.
… exports Which symbol a Vulkan implementation exports is not the same everywhere. A Khronos loader and MoltenVK export `vkGetInstanceProcAddr`. A bare Mesa ICD exports only the three `vk_icd*` entry points: lavapipe's version script exports exactly `vk_icdGetInstanceProcAddr`, `vk_icdGetPhysicalDeviceProcAddr` and `vk_icdNegotiateLoaderICDInterfaceVersion` -- verified with `nm -D` on the artifact, three symbols and no more. A named ccall to `vkGetInstanceProcAddr` cannot load its symbol against one, so every Mesa driver used without a loader was unreachable. Found by dlsym now, taking whichever the library has. An ICD never told otherwise assumes loader interface version 0, where the two behave identically, so there is nothing to negotiate. Lavapipe then works with no loader at all: llvmpipe (LLVM 22.1.8), Vulkan 1.4.354, 182 device extensions, and a device created with VK_KHR_acceleration_structure, VK_KHR_ray_query, VK_KHR_ray_tracing_pipeline, VK_KHR_deferred_host_operations and VK_KHR_buffer_device_address all enabled, plus a compute queue. That is a CPU backend that can run the ray tracing path. Its own suite is 53 passed, 0 failed, 1 errored there, and the error is `vkEnumerateInstanceLayerProperties` returning ERROR_LAYER_NOT_PRESENT -- layers come from the loader, so a bare ICD has none. The test assumes one. No regression with a real driver: 126 of 126 on RADV.
Lavapipe_jll registers its ICD manifest additively at init, so loading it before Vulkan exposes the software device alongside every system device. Document that direct path and remove set_driver(:Lavapipe), whose direct-ICD binding hid the hardware drivers and required a restart. Vulkan remains 126/126 on the four-device loader setup.
# Conflicts: # src/Vulkan.jl
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR roughly changes:
set_driverwrites a preference instead of an env var; useusing Lavapipe_jllfor lavapipe.It's all to get Mantle work cleanly across operating systems and GPU vendors - and to make it possible to depend on it on Apple, without paying the cost.