Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions proposals/NNNN-spirv-push-constants.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
title: "[NNNN] - SPIR-V Push constants"
params:
status: Design In Progress
authors:
- Keenuts: Nathan Gauër
---

## Introduction

Push Constants are a Vulkan mechanism to pass a small amount of data to the
shaders without creating buffers or modifying bindings.

Usage is as follows:

```hlsl
struct S {
float3 light_color;
};

[[vk::push_constant]]
S data;

void main() {
[...]
do_something(data.light_color)
}
```
## Motivation

This is a core feature of HLSL+Vulkan.

## Proposed Solution

### HLSL - Parsing

``[[vk::push_constant]]`` is a normal attribute in the `vk` namespace, with
no custom parsing.
This attribute is only allowed on global variables. Additional checks like
storage class compatibility is done in sema.

At this stage, we know the attribute can only be attached to a global
variable.

### HLSL - Sema

During sema, compatibility checks between the global variable definition
and the attribute are checked, and the global variable definition is created.

Additional checks are:
- The attribute is only allowed on a global variable with a struct type.
- There can only be one ``[[vk::push_constant]]`` attribute in the shader.
- There can be no VLA in the struct type (or a nested struct type).

DXC allows any storage class to be attached to a push constant. I believe
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call. Please list which storage classes are currently accepted by DXC, that will not be supported in Clang.
We should update DXC to match 202X?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Keywords DXC accepts are:

  • extern
  • static
  • groupshared
  • shared
  • uniform

Seems like [const] [extern] uniform is the good class, but [const] auto/none should be allowed as the attributes implies const extern uniform already.

this is an oversight and we should probably refuse this. As result, the
following rules will apply:

- The variable storage class must be either `auto` or `uniform`.
- The variable can be marked `const`, but is always considered `const`.

### DXIL

This being a Vulkan specific feature, the attribute is ignored when targeting
DXIL. This means the push constant global becomes part of the cbuffer when
building for DXIL.

The attribute being ignored, the variable belong to the cbuffer, meaning no
additional handling is required.
Comment on lines +64 to +69
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still don't fully understand all of the requirements for HLSL attributes in SEMA. For the resource work @hekota mentioned that for the clang based tools like the rewriter, the attributes will have to be in the AST.

Can you be more explicit about where and how it will be ignored?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The AST will have the HLSLVkPushConstant attribute since parsing will see it. But in Sema, this attribute will be ignored, meaning we won't change the current behavior which is to put a global in the global cbuffer.


### SPIR-V

The created global variable after parsing has the following characteristics:
- the global variable will be marked as `const`.
- the variable address space will be `hlsl_push_constant`.

Codegen will be left almost as-is as we simply load a variable in a different
address space.
Comment on lines +77 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you be more explicit about what the layout of the struct should be? I believe it is the same as a RWStructuredBuffer.

The DXC SPIR-V documentation does not mention it.

The rules in the Vulkan documentation give the same layout rules for push constant as storage buffers.

What we need to fixup is the cbuffer logic as a global variable with the
HLSLVkPushConstant attribute should not be added to the constant buffer.

The backend will then lower variables into the `hlsl_push_constant` address
space into a `PushConstant` storage class. The rest is similar to any variable
in another address space and should already be handled.

## Draft PR

There is also a draft PR implementing this end-to-end:
- https://github.com/llvm/llvm-project/pull/166793