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

[0010] Fix the address space for BufferPointers #98

Closed
s-perron opened this issue Sep 11, 2023 · 19 comments · Fixed by #114
Closed

[0010] Fix the address space for BufferPointers #98

s-perron opened this issue Sep 11, 2023 · 19 comments · Fixed by #114
Labels
active proposal Issues relating to active proposals

Comments

@s-perron
Copy link
Collaborator

s-perron commented Sep 11, 2023

Which proposal does this relate to?
Proposal 0010: vk::BufferPointer

Describe the issue or outstanding question.

The spec says

All buffer pointers are presumed to point into the host memory address space.

This is not correct. The pointers will contain the address of a buffer that can be used by the device to access the buffer. in device memory. That is, the address that is used by the device to access the buffer. EDIT: @jeremyong pointed out my careless wording. The memory could be located on the device or simply accessible to the device. The shader does not know or care.

Additional context

See #84 (comment) and the follow up comments by @natevm and me.

The values for these pointers come from calls to vkGetBufferDeviceAddress on the host. If we were to keep the language the same as vulkan, we would say that it is in the "device address space". I am open to other suggestions.

@s-perron s-perron added the active proposal Issues relating to active proposals label Sep 11, 2023
@s-perron
Copy link
Collaborator Author

FYI: @greg-lunarg

@jeremyong
Copy link

If we were to keep the language the same as vulkan, we would say that it is in the "device address space".

Agreed -- if possible, it would be great if we could standardize terminology for "address space" vs "memory location" since I think reading this issue and the linked proposal, I'm not actually sure which is referred to.

In particular, you could have host-local memory addressable by the GPU in the device address space. Alternatively, with resizable bar, you could have device-local memory addressable by the CPU in the host address space.

This is not correct. The pointers will contains the address of a buffer in device memory. That is, the address that is used by the device to access the buffer.

Here, when you say "address of a buffer in device memory", I read this to mean "device-local memory, with an address without a host or device-specific address space designation" I think.

@natevm
Copy link

natevm commented Sep 13, 2023

In particular, you could have host-local memory addressable by the GPU in the device address space. Alternatively, with resizable bar, you could have device-local memory addressable by the CPU in the host address space.

if it is in device address space, then it is device-local memory, not host-local. If it’s allocated with resizable bar, then I believe this is still device-local memory.

So, there isn’t a distinction, because the locality and address space are the same afaik, at least with Vulkan. I could be wrong, @s-perron probably knows better than me.

These are both also different from “visibility” and “coherency”.

I’d recommend reading this post which covers the complexity of memory terminology in Vulkan fairly well:

https://asawicki.info/news_1740_vulkan_memory_types_on_pc_and_how_to_use_them

@jeremyong
Copy link

I've read that article (and the vulkan spec on this topic, albeit years ago) so I'll assume there's something I don't understand, but I assumed that address spaces mapped to the same physical memory via different MMUs was possible.

@natevm
Copy link

natevm commented Sep 13, 2023

This page is also helpful: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetBufferDeviceAddress.html

If multiple VkBuffer objects are bound to overlapping ranges of VkDeviceMemory, implementations may return address ranges which overlap. In this case, it is ambiguous which VkBuffer is associated with any given device address.

So, ultimately this is something that is implementation specific. The wording "implementations may return" mean that it's up to the driver, and vulkan users don't really get direct control.

@jeremyong
Copy link

jeremyong commented Sep 14, 2023

Sorry, I think I'm not quite following. This issue, at least as I read it (@s-perron feel free to elaborate) pertains to address spaces, which I interpret differently from "address" and "location". Specifically, if I have an address, the value might be meaningful when dereferenced through a specific MMU (i.e. device or host). I liken this to DX12's notion of shader-visibility and CPU vs GPU handles, in addition to the notion of the "GPU address" of any given resource. Whether or not the address is defined in terms of a host or device address space has no bearing on the physical location of that memory. The question that I'm not sure you addressed is this statement:

if it is in device address space, then it is device-local memory, not host-local.

Why is this true? I don't see why host-local memory can't be made visible over the PCIe bus, addressable by the device itself.

If it’s allocated with resizable bar, then I believe this is still device-local memory.

Yes it's of course device-local memory, but the point is that this memory has at least two valid addresses, one in the device address space, and one in the host address space.

@natevm
Copy link

natevm commented Sep 14, 2023

if it is in device address space, then it is device-local memory, not host-local.

Why is this true? I don't see why host-local memory can't be made visible over the PCIe bus, addressable by the device itself.

Your definition of "address spaces" sounds similar to what vulkan describes as "visibility". Memory that is host-local and both host and device visible can be accessed by both the CPU and the GPU.

If it’s allocated with resizable bar, then I believe this is still device-local memory.

Yes it's of course device-local memory, but the point is that this memory has at least two valid addresses, one in the device address space, and one in the host address space.

Kinda... With Vulkan, if you want to access memory on the host, then you cannot use Vulkan's vkDeviceAddress values. Instead, you map the memory to a pointer on the host. You would never upload a host-side mapped pointer to the device. In that sense, there are two "addresses": the void pointer returned by the vulkan map call, and then the vkDeviceAddress handle. Again, can only use the mapped pointer on the host, and the device address handle on the device. They aren't interchangable.

@jeremyong
Copy link

Your definition of "address spaces" sounds similar to what vulkan describes as "visibility"

Sure, that's one way to interpret it (FWIW, I'm a heavy Vulkan user also).

Again, can only use the mapped pointer on the host, and the device address handle on the device. They aren't interchangable.

Yes of course, but, and again, apologies for sounding like a broken record, you aren't answering my question about this statement:

if it is in device address space, then it is device-local memory, not host-local.

In what sense are you referring to "address space" here for this statement to be true?

@natevm
Copy link

natevm commented Sep 14, 2023

Yes of course, but, and again, apologies for sounding like a broken record, you aren't answering my question about this statement:

if it is in device address space, then it is device-local memory, not host-local.

In what sense are you referring to "address space" here for this statement to be true?

I misunderstood what you meant by address spaces. (I guess that's what happened to the spec too)

An address space is the range of virtual addresses that some system assigns to a running program. Within the scope of a Vulkan program, there is only one address space for VkDeviceAddresses. If my VkDeviceAddress is 42, and another is 1337, it's a guarantee that both of those addresses still belong to the same "Vulkan Device Addresses" space.

Shader code will never need to answer the question "what address space does this address '42' live?" because it's always lives in the same "Vulkan vkDeviceAddress" space. We aren't going to run into the scenario where one device address being 42 and another being 42 actually refer to two different locations because of two different address spaces. Also, the assumption that an address space must be only CPU or only GPU memory isn't right. For example, with the unified virtual addressing space, addresses there cover all devices.

(Note, we might get multiple address spaces with multi-gpu configurations, but typically unified virtual addressing would kick in there, and then it's again one single address space.)

These addresses are of course virtual, and virtual address spaces can map to other virtual address spaces. Because they are virtual, their physical locality can be either the host or the device. But that's locality, not "address space".

@natevm
Copy link

natevm commented Sep 14, 2023

So, long and short of it, the issue with the spec is that it's talking about address spaces as if they were address localities, and then it confuses "host" (the CPU) for "device" (the GPU).

The solution I have in mind would be to omit the "Buffer Pointers and Address Space" section entirely, since A) it doesn’t make sense (it's a confusing thing, so I understand why this mistake was made), and B) the constraint doesn't make sense since address spaces and address locality are outside the scope of HLSL.

@jeremyong
Copy link

Within the scope of a Vulkan program, there is only one address space for VkDeviceAddresses. If my VkDeviceAddress is 42, and another is 1337, it's a guarantee that both of those addresses still belong to the same "Vulkan Device Addresses" space.

I think you mean to say that there is an address space per VkDevice associated with the device MMU, since it's quite possible to have multiple devices within a program. Otherwise, I agree that the "Buffer Pointers and Address Space" section doesn't really make sense.

Note that there is a non-standard way in which some DXC docs refer to address space as well. Sometimes, it seems to be used to refer to memory "tier" for lack of a better word. That is, memory that allocated from the register file and memory allocated from LDS are different "spaces" in the sense that they are physically located in different tiers of the device-local memory hierarchy.

A natural question then might be, if I can form a vk::BufferPointer to a variable, is it legal to do so for a groupshared variable? Is it possible to write a function that accepts a vk::BufferPointer parameter, where the arg may refer to LDS or VRAM?

@s-perron
Copy link
Collaborator Author

s-perron commented Sep 14, 2023

I think part of the problem we have is the definition of the term "address space". This is not defined in Vulkan.

In LLVM an address space is just a number, and its interpretation is determined by the target. If we introduce a term like "device address space" to HLSL, we get to decide what it means. I suggested this because the Vulkan spec refers to the value that will be used for the buffer pointers as device addresses. This creates a similarly between the two.

As this discussion shows it is possible that this will be confusing. It is not a stretch to guess that a "device address" or "device address space" will be an address of a memory location in the device's local memory. However that does not have to be the case, at least the way that Vulkan uses the term device local".

Consider the NVIDIA memory layout mentioned in the link above:

Heap 0: DEVICE_LOCAL
  Size = 8,421,113,856 B
  Type 0: DEVICE_LOCAL
  Type 1: DEVICE_LOCAL
Heap 1
  Size = 8,534,777,856 B
  Type 0
  Type 1: HOST_VISIBLE, HOST_COHERENT
  Type 2: HOST_VISIBLE, HOST_COHERENT, HOST_CACHED

It is possible to allocate memory on Heap 1 with memory type 1 or 2, bind a buffer to it, and then get the device address for that buffer. Using the Vulkan terms, you cannot call that address "device local" because it does not have the "device local" flag. However, I want to define "device address space" so that it will include that address.

I am open to alternatives. I am bad at naming things because I lack imagination. I am not sure of a better name for "an address that can be used by the device to access the memory."

I hope this clears up the disagreement.

@s-perron
Copy link
Collaborator Author

@greg-lunarg Let me know if you have any better ideas.

@natevm
Copy link

natevm commented Sep 14, 2023

At least personally, I like that terminology. I think it just needs to be clearly defined in the spec, similar to how you phrased it above.

@greg-lunarg
Copy link
Contributor

The term "host memory address space" was requested and approved by @llvm-beanz in #53. Since this is an HLSL proposal, I deferred to the HLSL/DirectX expert for the correct terminology.

I think this term is fine in that it implies an address space not in the GPU address space.

If we were to use a Vulkan term, I would say Buffer Device Address space, which jives with @s-perron 's reference to vkGetBufferDeviceAddress.

I will create a PR to add the following to the sentence in question: "in DirectX parlance, or buffer device address space in Vulkan parlance". If @llvm-beanz doesn't like it, he can delete it.

@natevm
Copy link

natevm commented Oct 11, 2023

I think this term is fine in that it implies an address space not in the GPU address space.

No, it implies that the addresses are in the host address space, which is wrong. The addresses are a Vulkan address space which point to either host or device memory. But the address space itself is neither host nor device.

If we were to use a Vulkan term, I would say Buffer Device Address space, which jives with @s-perron 's reference to vkGetBufferDeviceAddress.

This sounds good to me.

in DirectX parlance, or buffer device address space in Vulkan parlance

It’s not about parlance, it’s what’s true and what’s not true. Direct X doesn’t have a parlance for this, since Direct X doesn’t have device addresses. Still, if Direct X did have them, they wouldn’t be restricted to a “host address space” either.

@s-perron
Copy link
Collaborator Author

FYI: If we want to keep the language in line with the address spaces that Chris is already thinking about, I still think that "device memory" or device address space are best. See https://github.com/microsoft/hlsl-specs/pull/94/files#diff-29269dd46a5c0ae5cce91d3af91fce8d966c04887b38fe30ec54cb37cb28c5ceR230

@llvm-beanz Do you have any thoughts?

@llvm-beanz
Copy link
Collaborator

DirectX does have device addresses, we actually don't have host address support (see the DXIL docs). We don't express them in HLSL, but we do in DXIL. DirectX and Vulkan both agree on the terminology for host and device memory, although the Vulkan memory model is specified clearly and the DXIL memory model is not (although as @s-perron pointed out I'm trying to fix that).

I approved the PR with "host address space" because that's what I thought you meant based on the proposal. If that isn't what was intended, we should correct it.

Since this proposal is Vulkan-specific I think it is appropriate to use terminology from the Vulkan memory model, and maybe we should also add a link to the Vulkan memory model documentation to provide additional context.

@greg-lunarg
Copy link
Contributor

Since this proposal is Vulkan-specific I think it is appropriate to use terminology from the Vulkan memory model, and maybe we should also add a link to the Vulkan memory model documentation to provide additional context.

OK. I will create a PR consistent with this ruling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
active proposal Issues relating to active proposals
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants