[SM6.8] Add proposal for DXIL 1.8 #77

llvm-beanz · 2023-08-17T16:09:14Z

This change proposes an abbreviated DXIL 1.8 specification. We should fill this out with more detail as appropriate, but this should document the basic bits that we need to verify the compiler implementation.

jeremyong · 2023-09-09T09:16:51Z

While a new shader model is introduced, do you think it might be possible to also deprecate constant buffer layout usage (with a flag) at the same time? Or would that introduce too much risk to the shader model release? I'm thinking the later that gets pushed out, the further into the future we'd have to maintain all that legacy cbuffer layout code :)

This updates the new barrier instruction signatures.

llvm-beanz · 2023-09-25T21:45:23Z

While a new shader model is introduced, do you think it might be possible to also deprecate constant buffer layout usage (with a flag) at the same time? Or would that introduce too much risk to the shader model release? I'm thinking the later that gets pushed out, the further into the future we'd have to maintain all that legacy cbuffer layout code :)

Unfortunately, the non-legacy cbuffer layout has lots of bugs and fixing those is out of scope for SM 6.8 (we're too late in the process). It is on our mind as a change we want to get resolved for the future, but it will miss SM 6.8

pow2clk

I don't know if anything is missing, but whatevber might be can be added later.

pow2clk · 2023-09-26T19:55:40Z

proposals/NNNN-dxil18.md

+├───────────────────────────────┼────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
+│allocateNodeOutputRecord       │  238   │%dx.types.NodeRecordHandle @dx.op.allocateNodeOutputRecords(i32, %dx.types.NodeHandle, i32, i1)                        │
+├───────────────────────────────┼────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
+│getNodeRecordPtr               │  239   │<type> addrspace(6)* @dx.op.getNodeRecordPtr.<type>(i32, %dx.types.NodeRecordHandle, i32)                              │


We should document the new address space somewhere. Glancing at DXIL.rst, we don't document two (4 and 5) that we don't really use as-is.

pow2clk · 2023-09-26T21:53:22Z

proposals/NNNN-dxil18.md

+    ThreadNodeOutputRecords = (uint32_t)NodeIOFlags::ReadWrite |
+                              (uint32_t)NodeIOFlags::Output |
+                              (uint32_t)NodeIOFlags::ThreadRecord,
+  };


I think this might be more easily consumable as a table either with a single field that lists the flags enabled or maybe even with a column for each flag.

pow2clk · 2023-09-26T21:55:00Z

proposals/NNNN-dxil18.md

+Unlike memory operations in earlier versions of DXIL, reading and writing node
+record memory uses LLVM's load and store instructions. DXIL 1.8 reserves address
+space 6 for node memory. The `getNodeRecordPtr` DXIL operation returns a pointer
+to node record memory in address space 6.


I see now this addresses the space. I feel like maybe this would be more useful higher anyway and it would make the appearance of address space 6 in that table less surprising as well as the absence of some expected intrinsics.

This updates the DXIL 1.8 additions document with the expected changes to SM 6.8 features.

pow2clk · 2024-01-16T22:29:53Z

proposals/NNNN-dxil18.md

+Nick Feeney
+Amar Patel
+Tex Riddell
+Greg Roth


You may want to put this into a list with asterisks or find some other way to separate them. As written, these will all be on the same line, which makes it hard to tell where one name stops and the next starts.

tex3d · 2024-03-07T21:52:54Z

proposals/NNNN-dxil18.md

+
+### New DXIL Metadata
+
+Node shader entry metadata can contain the following tagged entries:


I feel we should normalize language here for tag,value lists. This particular list are tags used in the optional extended properties MDList for an entry point, which also needs to be made clear.

tex3d · 2024-03-07T21:56:06Z

proposals/NNNN-dxil18.md

+
+The `kDxilNodeInputsTag` and `kDxilNodeOutputsTag` values are lists of node
+metadata entries, where each node is an MDList of sub metadata entries based on
+the following index table:


This is another tag,value MDList. The values under Index here are not indices of an MDList, but the tag constant MDList operands preceding the value operand. As mentioned above, our language should be consistent across the tag,value MDList entries.

tex3d · 2024-03-07T21:57:49Z

proposals/NNNN-dxil18.md

+└─────────────────────────────────────────┴──────────┴──────────────────────────┘
+```
+
+`kDxilNodeRecordType` is a tagged metadata list based on the following tags:


This is better, but perhaps we should link to a common term introduced and explained earlier for all of these.

tex3d · 2024-03-09T02:00:08Z

proposals/NNNN-dxil18.md

+┌─────────────────────────────────────────┬──────────┬──────────────────────────┐
+│                   Tag                   │ Constant │          Value           │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNumThreadsTag                       │    4     │MDList: (i32, i32, i32)   │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeLaunchTypeTag                   │    13    │MDList: (i32)             │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeIsProgramEntryTag               │    14    │MDList: (i1)              │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeIdTag                           │    15    │MDList: (MDString, i32)   │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeLocalRootArgumentsTableIndexTag │    16    │MDList: (i32)             │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilShareInputOfTag                     │    17    │MDList: (MDString, i32)   │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeDispatchGridTag                 │    18    │MDList: (i32, i32, i32)   │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeMaxRecursionDepthTag            │    19    │MDList: (i32)             │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeInputsTag                       │    20    │MDList: (MDList[])        │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeOutputsTag                      │    21    │MDList: (MDList[])        │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeMaxDispatchGridTag              │    22    │MD list: (i32, i32, i32)  │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilRangedWaveSize                      │    23    │MD list: (i32, i32, i32)  │
+└─────────────────────────────────────────┴──────────┴──────────────────────────┘


The Value column doesn't look right. Each value seems to imply that it points to another MDList containing a value or values. This isn't the case. The single values here are operands in-line after the associated tag that identifies the next operand. The multiple values are another MDList, containing that set of values.

The node inputs/outputs is confusing, as it appears to me like an MDList containing a single MDList operand with some set of entries (which might imply that this set of entries are the inputs or outputs).

The meaning/encoding of most of the metadata values is not described.

Suggested change

┌─────────────────────────────────────────┬──────────┬──────────────────────────┐

│ Tag │ Constant │ Value │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNumThreadsTag │ 4 │MDList: (i32, i32, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeLaunchTypeTag │ 13 │MDList: (i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeIsProgramEntryTag │ 14 │MDList: (i1) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeIdTag │ 15 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeLocalRootArgumentsTableIndexTag │ 16 │MDList: (i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilShareInputOfTag │ 17 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeDispatchGridTag │ 18 │MDList: (i32, i32, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeMaxRecursionDepthTag │ 19 │MDList: (i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeInputsTag │ 20 │MDList: (MDList[]) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeOutputsTag │ 21 │MDList: (MDList[]) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeMaxDispatchGridTag │ 22 │MD list: (i32, i32, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilRangedWaveSize │ 23 │MD list: (i32, i32, i32) │

└─────────────────────────────────────────┴──────────┴──────────────────────────┘

┌─────────────────────────────────────────┬──────────┬──────────────────────────────┐

│ Tag │ Constant │ Value │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNumThreadsTag │ 4 │MDList: (i32 x, i32 y, i32 z) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeLaunchTypeTag │ 13 │i32 │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeIsProgramEntryTag │ 14 │i1 │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeIdTag │ 15 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeLocalRootArgumentsTableIndexTag │ 16 │i32 │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilShareInputOfTag │ 17 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeDispatchGridTag │ 18 │MDList: (i32 x, i32 y, i32 z) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeMaxRecursionDepthTag │ 19 │i32 │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeInputsTag │ 20 │MDList: (MDList[], ...) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeOutputsTag │ 21 │MDList: (MDList[], ...) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilNodeMaxDispatchGridTag │ 22 │MDList: (i32 x, i32 y, i32 z) │

├─────────────────────────────────────────┼──────────┼──────────────────────────────┤

│kDxilRangedWaveSize │ 23 │MDList: (i32 x, i32 y, i32 z) │

└─────────────────────────────────────────┴──────────┴──────────────────────────────┘

tex3d · 2024-03-09T02:05:41Z

proposals/NNNN-dxil18.md

+┌─────────────────────────────────────────┬──────────┬──────────────────────────┐
+│                   Tag                   │  Index   │          Value           │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeOutputIDTag                     │    0     │MDList: (MDString, i32)   │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeIOFlagsTag                      │    1     │MDList: (i32)             │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeRecordTypeTag                   │    2     │MDList: (MDList[])        │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeMaxRecordsTag                   │    3     │MDList: (i32)             │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeMaxRecordsSharedWithTag         │    4     │MDList: (MDString, i32)   │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeOutputArraySizeTag              │    5     │MDList: (i32)             │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeAllowSparseNodesTag             │    6     │MDList: (i1)              │
+└─────────────────────────────────────────┴──────────┴──────────────────────────┘


Similar issues with value as described in prior comment.

Suggested change

┌─────────────────────────────────────────┬──────────┬──────────────────────────┐

│ Tag │ Index │ Value │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeOutputIDTag │ 0 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeIOFlagsTag │ 1 │MDList: (i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeRecordTypeTag │ 2 │MDList: (MDList[]) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeMaxRecordsTag │ 3 │MDList: (i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeMaxRecordsSharedWithTag │ 4 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeOutputArraySizeTag │ 5 │MDList: (i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeAllowSparseNodesTag │ 6 │MDList: (i1) │

└─────────────────────────────────────────┴──────────┴──────────────────────────┘

┌─────────────────────────────────────────┬──────────┬──────────────────────────┐

│ Tag │ Index │ Value │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeOutputIDTag │ 0 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeIOFlagsTag │ 1 │i32 │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeRecordTypeTag │ 2 │MDList: (<tag-value list>)│

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeMaxRecordsTag │ 3 │i32 │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeMaxRecordsSharedWithTag │ 4 │MDList: (MDString, i32) │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeOutputArraySizeTag │ 5 │i32 │

├─────────────────────────────────────────┼──────────┼──────────────────────────┤

│kDxilNodeAllowSparseNodesTag │ 6 │i1 │

└─────────────────────────────────────────┴──────────┴──────────────────────────┘

tex3d · 2024-03-09T02:10:05Z

proposals/NNNN-dxil18.md

+┌─────────────────────────────────────────┬──────────┬──────────────────────────┐
+│                   Tag                   │ Constant │          Value           │
+├─────────────────────────────────────────┼──────────┼──────────────────────────┤
+│kDxilNodeRecordSizeTag                   │    0     │MDList: (i32)             │


Suggested change

│kDxilNodeRecordSizeTag │ 0 │MDList: (i32) │

│kDxilNodeRecordSizeTag │ 0 │i32 │

tex3d

I'm going to approve so we may add the document. However, there are a lot of deficiencies to address after we add the document, which should be captured as follow-up issues/tasks.

We should probably update the status to reflect the fact that we have already shipped this as well.

Besides the review feedback so far, there is also:

Missing feature details for: expanded-comparison-sampling and extended-command-info
Lacking descriptions for each new DXIL intrinsic and their parameters.
Lacking descriptions for new metadata
Lacking new feature flags and descriptions

There may be more missing that I'm not thinking of right now, but we can address that as we update the document and these things come up.

[SM6.8] Add proposal for DXIL 1.8

e39707b

This change proposes an abbreviated DXIL 1.8 specification. We should fill this out with more detail as appropriate, but this should document the basic bits that we need to verify the compiler implementation.

llvm-beanz added this to the Shader Model 6.8 milestone Aug 17, 2023

llvm-beanz added the Requires Shader Model Feature requests that require DXIL and Shader Model updates label Aug 17, 2023

llvm-beanz mentioned this pull request Sep 25, 2023

[Work Graphs] Remove SYNC_FLAG from Barrier intrinsics #101

Closed

Update barrier arguments

8aeac7e

This updates the new barrier instruction signatures.

pow2clk approved these changes Sep 26, 2023

View reviewed changes

pow2clk assigned pow2clk and tex3d Dec 11, 2023

Update DXIL 1.8

fb3fca1

This updates the DXIL 1.8 additions document with the expected changes to SM 6.8 features.

pow2clk reviewed Jan 17, 2024

View reviewed changes

Fix list formatting

b79c0cd

tex3d reviewed Mar 9, 2024

View reviewed changes

tex3d approved these changes Apr 3, 2024

View reviewed changes

damyanp unassigned pow2clk Apr 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SM6.8] Add proposal for DXIL 1.8 #77

[SM6.8] Add proposal for DXIL 1.8 #77

llvm-beanz commented Aug 17, 2023

jeremyong commented Sep 9, 2023

llvm-beanz commented Sep 25, 2023

pow2clk left a comment

pow2clk Sep 26, 2023

pow2clk Sep 26, 2023

pow2clk Sep 26, 2023

pow2clk Jan 16, 2024

tex3d Mar 7, 2024

tex3d Mar 7, 2024

tex3d Mar 7, 2024

tex3d Mar 9, 2024

tex3d Mar 9, 2024

tex3d Mar 9, 2024

tex3d left a comment •

edited

Loading


		### New DXIL Metadata

		Node shader entry metadata can contain the following tagged entries:

	│kDxilNodeRecordSizeTag │ 0 │MDList: (i32) │
	│kDxilNodeRecordSizeTag │ 0 │i32 │

[SM6.8] Add proposal for DXIL 1.8 #77

Are you sure you want to change the base?

[SM6.8] Add proposal for DXIL 1.8 #77

Conversation

llvm-beanz commented Aug 17, 2023

jeremyong commented Sep 9, 2023

llvm-beanz commented Sep 25, 2023

pow2clk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tex3d left a comment • edited Loading

Choose a reason for hiding this comment

tex3d left a comment •

edited

Loading