Supporting MemoryRef?

[Seelengrab]

Hi! With the new Memory PR on 1.11/nightly, it seems there's a new failure mode in the generated code here (or I'm doing something wrong again). The top-level call I'm doing is Vulkan.create_device, like so:

    queueInfo = Vulkan.DeviceQueueCreateInfo(indices.graphicsFamily, queuePriorities)
    deviceinfo = Vulkan.DeviceCreateInfo([queueInfo], [], [])
    deviceFeatures = Vulkan.create_device(app.physicalDevice, deviceinfo) |> unwrap

This is the stacktrace I get:

ERROR: LoadError: MethodError: no method matching pointer_length(::MemoryRef{Float32})

Closest candidates are:
  pointer_length(::Ptr{Nothing})
   @ Vulkan ~/.julia/packages/Vulkan/uRhTL/src/prewrap/pointers.jl:25
  pointer_length(::Tuple{Any, Any})
   @ Vulkan ~/.julia/packages/Vulkan/uRhTL/src/prewrap/pointers.jl:28
  pointer_length(::Base.RefArray)
   @ Vulkan ~/.julia/packages/Vulkan/uRhTL/src/prewrap/pointers.jl:27
  ...

Stacktrace:
  [1] Vulkan._DeviceQueueCreateInfo(queue_family_index::UInt32, queue_priorities::Vector{Float32}; next::Ptr{Nothing}, flags::Vulkan.DeviceQueueCreateFlag)
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:26450
  [2] Vulkan._DeviceQueueCreateInfo(x::Vulkan.DeviceQueueCreateInfo)
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:50115
  [3] convert(T::Type{Vulkan._DeviceQueueCreateInfo}, x::Vulkan.DeviceQueueCreateInfo)
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:56829
  [4] setindex!(A::Memory{Vulkan._DeviceQueueCreateInfo}, x::Vulkan.DeviceQueueCreateInfo, i1::Int64)
    @ Base ./genericmemory.jl:188
  [5] unsafe_copyto!(dest::Memory{Vulkan._DeviceQueueCreateInfo}, doffs::Int64, src::Memory{Vulkan.DeviceQueueCreateInfo}, soffs::Int64, n::Int64)
    @ Base ./genericmemory.jl:103
  [6] unsafe_copyto!(dest::Memory{Vulkan._DeviceQueueCreateInfo}, doffs::Int64, src::Memory{Vulkan.DeviceQueueCreateInfo}, soffs::Int64, n::Int64)
    @ Base ./genericmemory.jl:83 [inlined]
  [7] _copyto_impl!
    @ ./array.jl:308 [inlined]
  [8] copyto!
    @ ./array.jl:294 [inlined]
  [9] copyto!
    @ ./array.jl:319 [inlined]
 [10] copyto_axcheck!
    @ ./abstractarray.jl:1195 [inlined]
 [11] Array
    @ ./array.jl:612 [inlined]
 [12] convert
    @ ./array.jl:604 [inlined]
 [13] convert_nonnull
    @ ~/.julia/packages/Vulkan/uRhTL/src/prewrap/pointers.jl:30 [inlined]
 [14] Vulkan._DeviceCreateInfo(x::Vulkan.DeviceCreateInfo)
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:50117
 [15] convert(T::Type{Vulkan._DeviceCreateInfo}, x::Vulkan.DeviceCreateInfo)
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:56831
 [16] macro expansion
    @ ~/.julia/packages/Vulkan/uRhTL/src/prewrap/errors.jl:48 [inlined]
 [17] create_device(physical_device::Vulkan.PhysicalDevice, create_info::Vulkan.DeviceCreateInfo; allocator::Ptr{Nothing})
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:68603
 [18] create_device(physical_device::Vulkan.PhysicalDevice, create_info::Vulkan.DeviceCreateInfo)
    @ Vulkan ~/.julia/packages/Vulkan/uRhTL/generated/linux.jl:68602
 [19] createLogicalDevice!(app::MLTarget.App)
    @ MLTarget ~/Documents/projects/MLTarget.jl/src/MLTarget.jl:145
 [20] initVulkan!(app::MLTarget.App)
    @ MLTarget ~/Documents/projects/MLTarget.jl/src/MLTarget.jl:48
 [21] run!(app::MLTarget.App)
    @ MLTarget ~/Documents/projects/MLTarget.jl/src/MLTarget.jl:209
 [22] main()
    @ MLTarget ~/Documents/projects/MLTarget.jl/src/MLTarget.jl:233
 [23] top-level scope
    @ ~/Documents/projects/MLTarget.jl/run.jl:7
in expression starting at /home/sukera/Documents/projects/MLTarget.jl/run.jl:7

If I'm reading the existing definition for pointer_length right, this assumed that everything worked with RefArray (as it did in the past), though this now gets a MemoryRef, which isn't supported (and doesn't have the link back to the parent array). Not entirely sure how to fix this best.

[serenity4]

Thanks for reporting this! Indeed, we need to get the number of elements in the array in pointer_length. Now we get a ref::MemoryRef, which has a ref.mem.length::Int field but that might not correspond to the number of elements in the array and can only be assumed to be an overestimate (which we don't want to use as it might incur accesses to undefined data).

One fix might be to tweak the generated code such that for AbstractArray arguments pointer_length is taken before cconvert (which is how we get this MemoryRef), if we really can't do it in a better way.

Can't really think of another solution though. The accurate size is encoded in Array, but never in MemoryRef, right?

[Seelengrab]

Thanks for the quick reply!

One fix might be to tweak the generated code such that for AbstractArray arguments pointer_length is taken before cconvert (which is how we get this MemoryRef), if we really can't do it in a better way.

Yes, looks like it.

Can't really think of another solution though. The accurate size is encoded in Array, but never in MemoryRef, right?

Yes, that is correct! If you haven't yet, the Memory Julep is well worth a read. Might also be useful in the future to represent device memory more explicitly (there has been quite some discussion on having Memory support such usecases from other folks of GPUCompiler and CUDA).

[serenity4]

I'll get that done soon then. Shouldn't be hard to fix but one needs to know his way around a few significant portions of the generator to implement it. Unless you really want to give it a try that is 😄

[Seelengrab]

I dug into it a bit at first, but pretty much decided to leave this to you once I saw that this will have to touch the generator 😂 Thank you for fixing this!

[serenity4]

Should be fixed in a619f63 and available in the newly released 0.6.10.