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

[WorkGraphs] Proposal to Add MaxRecordsPerNode attribute to NodeOutputArrays. #323

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

anupamachandra
Copy link

WRT Workgraphs Feature:
This is the HLSL cpunterpart of the proposal to new node output attribute called MaxRecordsPerNode. This parameter is only required for output node arrays. This attribute specifies the maximum number of records that can be written to any single output node within a node array.
When determining backing store memory requirements, an implementation must assume the worst-case of MaxRecords written
to any single node in the output array. However, a common use-case is for a small number records to be written to
select nodes in a very large array of nodes. Some implementations can take advantage of this knowledge to significantly
reduce the backing store memory requirements while maintaining peak performance.

@anupamachandra anupamachandra changed the title First Draft [WorkGraphs] Proposal to Add MaxRecordsPerNode attribute to NodeOutputArrays. Sep 20, 2024
@anupamachandra
Copy link
Author

anupamachandra commented Sep 23, 2024 via email

proposals/NNNN-max-records-per-node.md Outdated Show resolved Hide resolved

| attribute | required | description |
|:--- |:--------:|:------------|
| `[MaxRecords(count)]` or `[MaxRecords(count, maxRecordsPerNode)]` | Y (this or below attribute) | Given uint `count` declaration, the thread group can output `0...count` records to this output. The variant with `maxRecordsPerNode` is required for `NodeArrayOutput`, where `count` applies across all the output nodes in the array and `maxRecordsPerNode` specifies the maximum number of records that can be written to a single output node within the array. Exceeding these limits results in undefined behavior. The value of `maxRecordsPerNode` must be less-than or equal to the value of `count`. These attributes can be overridden via the `NumOutputOverrides / pOutputOverrides` option when constructing a work graph as part of the [definition of a node](). See [Node output limits](). |
Copy link
Collaborator

Choose a reason for hiding this comment

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

MaxRecordsPerNode is defined as a separate attribute earlier on, instead of an extra parameter to the existing MaxRecords attribute. Which is it?

Choose a reason for hiding this comment

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

The proposal is a separate attribute. This section shows an "alternative" method as an extra parameter, which was earlier discussed in other forums.

Comment on lines +109 to +112
This attribute could be made optional, for maximum backward compatibility; i.e. existing SM6.8 Work Graphs compile with
the newer Shader Model. When `MaxRecordsPerNode` is _not_ specified, the implicit value of `MaxRecordsPerNode` is equal
to `MaxRecords`. This also avoids redundant attribute specifications for those usage models where the values of
`MaxRecords` and `MaxRecordsPerNode` are identical.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This case is already described above to have a different behavior.

Choose a reason for hiding this comment

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

Correct, this is an alternative option.

@damyanp damyanp added this to the Shader Model 6.9 milestone Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triaged
Development

Successfully merging this pull request may close these issues.

5 participants