[mlir][dxsa] Add dcl_interface and dcl_interface_dynamicindexed#142
[mlir][dxsa] Add dcl_interface and dcl_interface_dynamicindexed#142asavonic wants to merge 1 commit into
Conversation
asavonic
force-pushed
the
dxsa-mlir-interface
branch
from
45258c4 to
56156d2
Compare
asavonic
force-pushed
the
dxsa-mlir-functions
branch
from
543afcc to
cfe95be
Compare
asavonic
force-pushed
the
dxsa-mlir-interface
branch
from
56156d2 to
dd48216
Compare
asavonic
force-pushed
the
dxsa-mlir-functions
branch
from
cfe95be to
d03dc8a
Compare
dcl_interface binds multiple dcl_function_table declarations, so they can be accessed either statically or dynamically. Each table is bound at runtime, but the compiler must know every variant to generate a specialized version of the shader.
asavonic
force-pushed
the
dxsa-mlir-interface
branch
from
dd48216 to
3d63662
Compare
|
Let's wait for |
| dxsa.dcl_interface 1, <access = dynamic, array_length = 1, table_length = 2, tables = [0]> | ||
| ``` | ||
| }]; | ||
| let arguments = (ins I32Attr:$index, DXSA_InterfaceAccessAttr:$access, I32Attr:$array_length, I32Attr:$table_length, DenseI32ArrayAttr:$tables); |
There was a problem hiding this comment.
| let arguments = (ins I32Attr:$index, DXSA_InterfaceAccessAttr:$access, I32Attr:$array_length, I32Attr:$table_length, DenseI32ArrayAttr:$tables); | |
| let arguments = (ins I32Attr:$index, DXSA_InterfaceAccessAttr:$access, I32Attr:$array_length, I32Attr:$num_call_sites, DenseI32ArrayAttr:$tables); |
I vote to call this argument exactly as in the specification. table_length is a bit misleading, especially with the following table of a different size.
| auto tableLength = parseToken(); | ||
| FAILURE_IF_FAILED(tableLength); | ||
|
|
||
| auto interfaceArrayLength = parseToken(); |
There was a problem hiding this comment.
Maybe change to rawLengths or packedLenghts (there are two different lengts inside)
| FAILURE_IF_FAILED(index); | ||
|
|
||
| // Number of call sites (number of bodies in each table). | ||
| auto tableLength = parseToken(); |
There was a problem hiding this comment.
I vote to change the variable name to numCallSites for parser to be close to the spec. Right now var name and comment are not in sync.
| FAILURE_IF_FAILED(interfaceArrayLength); | ||
|
|
||
| // Number of tables (variants). | ||
| uint32_t interfaceLength = |
There was a problem hiding this comment.
I vote to change to tableLength (it will be in sync with the end of the macro DECODE_D3D11_SB_INTERFACE_TABLE_LENGTH)
and also use auto instead of exact type.
| DECODE_D3D11_SB_INTERFACE_TABLE_LENGTH(*interfaceArrayLength); | ||
|
|
||
| // Number of slots to be defined at runtime. | ||
| uint32_t arrayLength = |
There was a problem hiding this comment.
| uint32_t arrayLength = | |
| auto arrayLength = |
| } | ||
|
|
||
| def DXSA_InterfaceAccess_Immediate : I32EnumAttrCase<"immediate", 0>; | ||
| def DXSA_InterfaceAccess_Dynamic : I32EnumAttrCase<"dynamic", 1>; |
There was a problem hiding this comment.
The spec defines two operations
22.3.51 dcl_interface/dcl_interface_dynamicindexed (Interface Declaration)
Instruction: dcl_interface
fp#[arraySize][numCallSites] = {ft#, ft#, ...}
dcl_interface_dynamicindexed
fp#[arraySize][numCallSites] = {ft#, ft#, ...}
Is there any specific profit in collapsing them in one op with an additional enum DXSA_InterfaceAccessAttr?
There was a problem hiding this comment.
Dynamic indexing is a flag of how the declared array is going to be used (if the interface operand has immediate or register indices).
Otherwise dcl_interface and dcl_interface_dynamicindexed are identical: same opcode and semantic, documentation describes them both.
| FailureOr<Instruction> parseDclInterface(uint32_t opcodeToken, Location loc) { | ||
| bool isDynamic = DECODE_D3D11_SB_INTERFACE_INDEXED_BIT(opcodeToken); | ||
| auto access = dxsa::symbolizeInterfaceAccess(isDynamic); | ||
| assert(access && "unhandled interface access kind"); // access kind is 1 bit |
There was a problem hiding this comment.
Maybe change the fragment to smth. like:
bool isDynamic = DECODE_D3D11_SB_INTERFACE_INDEXED_BIT(opcodeToken);
auto access = isDynamic ? dxsa::InterfaceAccess::dynamic
: dxsa::InterfaceAccess::immediate;
| // CHECK-LABEL: module | ||
|
|
||
| // CHECK-NEXT: dxsa.dcl_interface 0, <access = dynamic, array_length = 253, table_length = 1, tables = [0, 1]> | ||
| // CHECK-NEXT: dxsa.dcl_interface 0, <access = immediate, array_length = 253, table_length = 1, tables = [0, 1]> No newline at end of file |
2 participants
dcl_interfacebinds multipledcl_function_tabledeclarations, so they can be accessed either statically or dynamically.Each table is bound at runtime, but the compiler must know every variant to generate a specialized version of the shader.