-
Notifications
You must be signed in to change notification settings - Fork 2
[mlir][dxsa] Add dcl_interface and dcl_interface_dynamicindexed #142
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
base: dxsa-mlir-functions
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -255,6 +255,23 @@ def DXSA_SystemValueNameAttr : | |||||
| let assemblyFormat = "$value"; | ||||||
| } | ||||||
|
|
||||||
| def DXSA_InterfaceAccess_Immediate : I32EnumAttrCase<"immediate", 0>; | ||||||
| def DXSA_InterfaceAccess_Dynamic : I32EnumAttrCase<"dynamic", 1>; | ||||||
|
|
||||||
| def DXSA_InterfaceAccess : I32EnumAttr< | ||||||
| "InterfaceAccess", "DXBC access kind for an interface", [ | ||||||
| DXSA_InterfaceAccess_Immediate, | ||||||
| DXSA_InterfaceAccess_Dynamic | ||||||
| ]> { | ||||||
| let cppNamespace = "::mlir::dxsa"; | ||||||
| let genSpecializedAttr = 0; | ||||||
| } | ||||||
|
|
||||||
| def DXSA_InterfaceAccessAttr : | ||||||
| EnumAttr<DXSADialect, DXSA_InterfaceAccess, "interface_access"> { | ||||||
| let assemblyFormat = "$value"; | ||||||
| } | ||||||
|
|
||||||
| //===----------------------------------------------------------------------===// | ||||||
| // DXSA ComponentMask bit-enum (mask field of operand, normalized to bits 0..3) | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
@@ -754,4 +771,28 @@ def DXSA_DclFunctionTable : DXSA_Op<"dcl_function_table"> { | |||||
| let assemblyFormat = "$index `,` `<` `functions` `=` $functions `>` attr-dict"; | ||||||
| } | ||||||
|
|
||||||
| def DXSA_DclInterface : DXSA_Op<"dcl_interface"> { | ||||||
| let summary = "declares function table pointers (interfaces)"; | ||||||
| let description = [{ | ||||||
| Declare an `array_length` of function table pointers (interfaces). Each | ||||||
| table can be bound at runtime, and it should contain `table_length` function | ||||||
| bodies. | ||||||
|
|
||||||
| Example: | ||||||
| ```mlir | ||||||
| dxsa.dcl_function_body 0 | ||||||
| dxsa.dcl_function_body 1 | ||||||
| dxsa.dcl_function_table 0, <functions = [0, 1]> | ||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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. |
||||||
| let assemblyFormat = [{ $index `,` `<` | ||||||
| `access` `=` $access `,` | ||||||
| `array_length` `=` $array_length `,` | ||||||
| `table_length` `=` $table_length `,` | ||||||
| `tables` `=` $tables `>` attr-dict | ||||||
| }]; | ||||||
| } | ||||||
|
|
||||||
| #endif // DXSA_OPS | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -646,6 +646,15 @@ class DXBuilder { | |||||
| builder, loc, index, DenseI32ArrayAttr::get(ctx, functions)); | ||||||
| } | ||||||
|
|
||||||
| Instruction buildDclInterface(uint32_t index, dxsa::InterfaceAccess access, | ||||||
| uint32_t arrayLength, uint32_t tableLength, | ||||||
| ArrayRef<int32_t> tables, Location loc) { | ||||||
| auto *ctx = builder.getContext(); | ||||||
| return dxsa::DclInterface::create(builder, loc, index, access, arrayLength, | ||||||
| tableLength, | ||||||
| DenseI32ArrayAttr::get(ctx, tables)); | ||||||
| } | ||||||
|
|
||||||
| private: | ||||||
| MLIRContext *context; | ||||||
| ModuleOp module; | ||||||
|
|
@@ -1209,6 +1218,42 @@ class Parser { | |||||
| return builder.buildDclFunctionTable(*index, functions, loc); | ||||||
| } | ||||||
|
|
||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe change the fragment to smth. like: |
||||||
|
|
||||||
| // Index of the interface (start index for an array). | ||||||
| auto index = parseToken(); | ||||||
| FAILURE_IF_FAILED(index); | ||||||
|
|
||||||
| // Number of call sites (number of bodies in each table). | ||||||
| auto tableLength = parseToken(); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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(tableLength); | ||||||
|
|
||||||
| auto interfaceArrayLength = parseToken(); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe change to rawLengths or packedLenghts (there are two different lengts inside) |
||||||
| FAILURE_IF_FAILED(interfaceArrayLength); | ||||||
|
|
||||||
| // Number of tables (variants). | ||||||
| uint32_t interfaceLength = | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I vote to change to tableLength (it will be in sync with the end of the macro DECODE_D3D11_SB_INTERFACE_TABLE_LENGTH) |
||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| DECODE_D3D11_SB_INTERFACE_ARRAY_LENGTH(*interfaceArrayLength); | ||||||
|
|
||||||
| SmallVector<int32_t, 16> tables; | ||||||
| tables.resize(interfaceLength); | ||||||
| for (uint32_t i = 0; i < interfaceLength; ++i) { | ||||||
| auto tableIndex = parseToken(); | ||||||
| FAILURE_IF_FAILED(tableIndex); | ||||||
| tables[i] = *tableIndex; | ||||||
| } | ||||||
|
|
||||||
| return builder.buildDclInterface(*index, *access, arrayLength, *tableLength, | ||||||
| tables, loc); | ||||||
| } | ||||||
|
|
||||||
| OptionalParseResult parseDclInstruction(uint32_t opcodeToken, Location loc, | ||||||
| Instruction &out) { | ||||||
| FailureOr<Instruction> result; | ||||||
|
|
@@ -1261,6 +1306,9 @@ class Parser { | |||||
| case D3D11_SB_OPCODE_DCL_FUNCTION_TABLE: | ||||||
| result = parseDclFunctionTable(loc); | ||||||
| break; | ||||||
| case D3D11_SB_OPCODE_DCL_INTERFACE: | ||||||
| result = parseDclInterface(opcodeToken, loc); | ||||||
| break; | ||||||
| default: | ||||||
| return std::nullopt; | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // RUN: mlir-translate --import-dxsa-bin %S/inputs/dcl_interface.bin | FileCheck %s | ||
|
|
||
| // 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]> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No new line at the end of file |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec defines two operations
22.3.51 dcl_interface/dcl_interface_dynamicindexed (Interface Declaration)
Is there any specific profit in collapsing them in one op with an additional enum DXSA_InterfaceAccessAttr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_interfaceanddcl_interface_dynamicindexedare identical: same opcode and semantic, documentation describes them both.