Add support for KHR_buffer_device_address

[spnda]

Quick patch to add support for KHR_buffer_device_address for devices with iOS 16 or macOS 13.

Though also, was a small concern I had about #1600, this never checks if the MTLDevice actually supports Metal 3. Perhaps there are some changes to be made to MVKExtensions.def and MVKExtensions.mm. Somewhere, it should be checking supportsMTLGPUFamily(MTLGPUFamilyMetal3) and only then return the extension. And for VK_KHR_fragment_shader_barycentric, this:

if ([mtlDevice responsToSelector:@selector(supportsShaderBarycentricCoordinates)]) {
    return [mtlDevice supportsShaderBarycentricCoordinates]; // Metal 3
} else {
    return [mtlDevice areBarycentricCoordsSupported]; // Metal 2
}

Also because the gpuAddress getter obviously doesn't exist on older versions of macOS or Xcode, should I wrap the function definition with MVK_XCODE_14?

[billhollings]

Thanks for the submission!

Have you tested this? I'm asking, because coincidentally, I started working on this extension a couple of weeks ago, before WWDC introduced gpuAddress (I had been curious if contents would work on shared memory), and one thing I found when trying to run the buffer_device_address sample from Vulkan-Samples was that the MSL generated by SPIRV-Cross did not generate the right address space for the GPU pointers.

I do have a SPIRV-Cross change for that, which I will publish now that we can actually definitely do this.

Also because the gpuAddress getter obviously doesn't exist on older versions of macOS or Xcode, should I wrap the function definition with MVK_XCODE_14?

Yes...add an MVK_XCODE_14 exactly like MVK_XCODE_13, and wrap the call to gpuAddress in that...and I guess return either 0 or MVKBuffer::getHostMemoryAddress() in the #else part. I recommend adding a MVKBuffer::gpuAddress() function and handle all this messiness there...so we can use it more generally (for example as part of eventually supporting capture replay)

Also...add...

ADD_DVC_EXT_ENTRY_POINT(vkGetBufferDeviceAddressKHR, KHR_BUFFER_DEVICE_ADDRESS);

in the appropriate place in MVKInstance.mm (line 629 I think).

Also...even though we indicate that capture replay is disabled, we should add the functions anyway, so that app build linkages won't break from app code that checks for that flag any maybe calls those functions. For now you can just stub them out (plus add the corresponding MVKInstance.mm entry points).

Finally...there are some other housekeeping items that normally get added regarding formal extension enablement. But rather than explain them here, I'll add them from my version of this after we pull yours in.

[spnda]

I will test how [buffer gpuAddress] behaves in a second. I've pushed the fixed code for now though. Hope all is good now.

[mbarriault]

Just a note, I can't remember what page it was but I think gpuAddress was already deprecated and a holdover from an alpha design and gpuResourceID is to be used instead. Neither are documented and the session explaining them doesn't release until tomorrow.

[andyoneal]

@mbarriault macOS 13 release notes mention that MTLResource.gpuHandle is deprecated and should use gpuResourceID instead. Maybe that's what you're thinking of?

[mbarriault]

Yep that's what I was thinking of, and got confused not realizing gpuAddress and gpuHandle were distinct.

[spnda]

I also just finished upgrading to macOS 13 and tested the selectors.

    id<MTLBuffer> buffer = [device newBufferWithLength:64
                                    options:MTLResourceStorageModeShared];
    std::cout << [buffer contents] << std::endl;
    std::cout << [buffer gpuAddress] << std::endl;

For some reason I get the following output in the console:

0x0
0

But running with Xcode 14 in the debugger:

0x1005c8000
0x1500000000

Here it shows that they're two different values... Though why and how I'll see tomorrow, it's getting late.

[cdavis5e]

Though also, was a small concern I had about #1600, this never checks if the MTLDevice actually supports Metal 3.

I'm not sure it matters. macOS 13 drops support for practically everything that doesn't support Metal 3.

[jackis-simlabs]

Though also, was a small concern I had about #1600, this never checks if the MTLDevice actually supports Metal 3.

I'm not sure it matters. macOS 13 drops support for practically everything that doesn't support Metal 3.

macOS13 will support AMD Bronze GPUs like Radeon560, which is the default GPU for 2019 MacPro, but Bronze does not support Metal3. I would say - having a support check still matters

[billhollings]

Suggested cleanups for getMTLBufferAddress().

[billhollings]

macOS13 will support AMD Bronze GPUs like Radeon560, which is the default GPU for 2019 MacPro, but Bronze does not support Metal3. I would say - having a support check still matters

I agree with this.

Once we pull this in, I'll add a check for Metal3 when I add the code to track enablement of this extension in the VkDevice.

[billhollings]

@spnda

I commented above...but haven't heard back...about whether this has been tested on shaders.

I ran into problems with the pointer address space that SPIRV-Cross generates in the MSL. I have a SPIRV-Cross fix for that...but I'm curious if there are shaders for which it doesn't matter.

[spnda]

@billhollings So I'm testing a simple application atm. And I did post a small message yesterday here. However, I'll just recap what I wrote there.

So, it seems as if [buffer contents] returns a different address to what [buffer gpuAddress] returns. In this test, I write the [buffer gpuAddress] to the first uint64_t of the buffer bound to index 0. The pointer value of test which is the buffer bound at index 0 is the same address as returned by [buffer contents]. It seems that the pointer doesn't trigger a segfault or anything, but it still seems to be different to what the test pointer is. I validated this by offsetting by the thread index, and I do get validation errors for those indices because the buffer is 0 in those areas.

Quite frankly, I don't think the gpuAddress works as intended currently. This is a Beta for a reason. Sometimes in Xcode and nearly always outside of Xcode, I get 0x0 returned for the address, as I had already shown yesterday.

About your issue with address spaces I found the following on the Metal language specification. Essentially, using device should always work with pointers. constant is only allowed for read-only buffers, which is hard to guarantee at compile-time with SPIRV-Cross without any additional information.

For graphics functions, an argument that is a pointer or reference to a type must be declared in the device or constant address space. For kernel functions, an argument that is a pointer or reference to a type must be declared in the device, threadgroup, threadgroup_imageblock, or constant address space.

[K0bin]

Are you even allowed to use gpuAddress outside of argument buffers? Every mention of it from Apple mentions it as a means to fill an argument buffer.

[spnda]

With Metal 3 argument buffers, the usage I just showed is the same as what you would "normally" do. Metal 3 argument buffers are essentially just a struct where you assign each member with a MTLGPUHandle or the gpuAddress (I think MTLBuffer has no getter for the GPU handle). This is to bypass MTLArgumentEncoder. Though in my test application, I essentially create a struct of 8 uint64_t and assign the gpuAddress value to the first of those integers. But otherwise, I don't think of why the address would only work in argument buffers.

[billhollings]

I did post a small message yesterday

Yup...I saw that. It didn't talk about shaders and SPIRV-Cross...which is my main concern in my question. Is your gpuAddressTest() function coming from SPIR-V, or are you hand-writing MSL?

Quite frankly, I don't think the gpuAddress works as intended currently. This is a Beta for a reason. Sometimes in Xcode and nearly always outside of Xcode, I get 0x0 returned for the address, as I had already shown yesterday.

Okay. I've marked this PR as WIP for now...and it can evolve as the Beta, and other testing, progresses.

[spnda]

@billhollings I did some testing in my own renderer and in a simple test application, which simply tries writing to a buffer by using its uint64_t address as a pointer. Xcode shows "Invalid buffer" in the debug viewer, however the code still runs fine. I tested this by using the shader to multiply a value and the output buffer did indeed have the correct value in it. Another person, as well as myself, also noticed that when running the same code inside of an AppKit application makes Xcode correctly pick up the buffer. So, ultimately, it seems it's currently just a bug with Xcode that it falsely displays the buffer as being invalid. Therefore, I would think that it does actually work as expected and that SPIRV-Cross would only need to change to use device for the pointer's address space.

Is your gpuAddressTest() function coming from SPIR-V, or are you hand-writing MSL?

It is hand-written, although I did try and compile a GLSL shader separately and got a similar result to it and I think it would still be valid (given that your patch to fix the address space is applied).

I had been curious if contents would work on shared memory

I have a feeling the CPU and GPU access memory differently. The contents pointer is at a different address than the gpuAddress. Accessing the address from gpuAddress on the CPU results in EXC_BAD_ACCESS, similar to when trying to access contents from the GPU. Also quoting the documentation for the gpuAddress property, it explicitly says "virtual":

Represents the GPU virtual address of a buffer resource

Sometimes in Xcode and nearly always outside of Xcode, I get 0x0 returned for the address, as I had already shown yesterday.

I cannot reproduce this exact case, but the gpuAddress has not been 0x0 ever since... So perhaps this was just invalid testing by me.

So, in conclusion, I think this is actually perfectly valid and good-to-go. The gpuAddress is in fact a pointer to the memory that the GPU can access and works perfectly fine currently.

[cdavis5e]

I did some testing in my own renderer and in a simple test application, which simply tries writing to a buffer by using its uint64_t address as a pointer. Xcode shows "Invalid buffer" in the debug viewer, however the code still runs fine. I tested this by using the shader to multiply a value and the output buffer did indeed have the correct value in it. Another person, as well as myself, also noticed that when running the same code inside of an AppKit application makes Xcode correctly pick up the buffer.

Does it help if you link explicitly to CoreGraphics.framework? I know MTLCreateSystemDefaultDevice() doesn't work outside of AppKit unless you do that. Maybe other aspects of Metal are broken without Core Graphics initialized.

[spnda]

Does it help if you link explicitly to CoreGraphics.framework? I know MTLCreateSystemDefaultDevice() doesn't work outside of AppKit unless you do that. Maybe other aspects of Metal are broken without Core Graphics initialized.

No, linking CoreGraphics.framework does nothing. Also, MTLCreateSystemDefaultDevice has always worked for me in my application in which I use GLFW (it creates a NSApplication manually I think) without linking to CoreGraphics.framework explicitly.

[spnda]

@billhollings I received an answer from Apple regarding gpuAddress being shown as invalid in the Xcode debugger window:

Engineering has provided the following information regarding this issue:

Please know that this is a known issue when using captures + Shader Validation layer, so you should try running without shader validation. Otherwise, it should work, if it doesn’t, please let us know.

So my concerns came from a bug within Xcode; the code itself actually works perfectly. Without the shader validation layer the gpuAddress shows as expected and Xcode also shows the pointers in argument buffers properly:

So yeah, there's definitely nothing wrong with this PR anymore :)

[billhollings]

So my concerns came from a bug within Xcode; the code itself actually works perfectly.

Thanks for the update. I have also confirmed that gpuAddress() works correctly in my testing with the Vulkan-Samples sample app.

Just one last change needed to fix an issue I encountered in my testing.

[billhollings]

SPIRV-Cross PR #1965 adds the shader pointer behavior needed for this functionality.

Once that is pulled in, I'll pull this in, and add the additional functionality described above on a second PR here.

[billhollings]

if ([mtlDevice responsToSelector:@selector(supportsShaderBarycentricCoordinates)]) {
    return [mtlDevice supportsShaderBarycentricCoordinates]; // Metal 3
} else {
    return [mtlDevice areBarycentricCoordsSupported]; // Metal 2
}

I've created issue #1644 to track the update required for this.

supportsShaderBarycentricCoordinates seems to be supported wherever areBarycentricCoordsSupported is (macOS 10.15 / iOS 14.0), so we should be able to just use it.