[mlir][dxsa] Add dcl constant buffer instruction#133
Conversation
| return builder.buildDclOutput(*operand, loc); | ||
| } | ||
|
|
||
| FailureOr<Instruction> parseDclConstantBuffer(uint32_t opcodeToken, |
There was a problem hiding this comment.
For Shader Model 5.1 and later, constant buffer opcode is followed by 3 operands: range, size and index.
Looks like we only handle Shader Model 4.0 through 5.0.
| if (!accessPattern) | ||
| return emitError(loc, "unknown constant buffer access pattern: ") | ||
| << rawAccessPattern; | ||
| auto operand = parseInlineOperand(); |
There was a problem hiding this comment.
The operand for dcl_constantBuffer is fake - it can only be D3D10_SB_OPERAND_TYPE_CONSTANT_BUFFER, and we only need indices from it. The second index is also fake: it defines size of a buffer. Other properties of operands are meaningless - components, mask, swizzle. I don't think they are used for this instruction.
| LogicalResult | ||
| InlineOperandAttr::verify(function_ref<InFlightDiagnostic()> emitError, | ||
| ::mlir::dxsa::InlineOperandType /*type*/, | ||
| uint32_t components, |
There was a problem hiding this comment.
Do we verify that components can only be 0, 1, or 4?
There was a problem hiding this comment.
I plan to handle this in the separate PR that refines operands.
| ctx, static_cast<dxsa::ComponentName>(comp.swizzle[0]), | ||
| static_cast<dxsa::ComponentName>(comp.swizzle[1]), | ||
| static_cast<dxsa::ComponentName>(comp.swizzle[2]), | ||
| static_cast<dxsa::ComponentName>(comp.swizzle[3])); |
There was a problem hiding this comment.
Can we use symbolize to verify range?
There was a problem hiding this comment.
This fragment was removed form the PR.
| def DXSA_SystemValueNameAttr : | ||
| EnumAttr<DXSADialect, DXSA_SystemValueName, "system_value_name"> { | ||
| let assemblyFormat = "$value"; | ||
| let assemblyFormat = "`<` $value `>`"; |
There was a problem hiding this comment.
Why do we need angle brackets here?
There was a problem hiding this comment.
Angle brackets matches the established MLIR convention — < $value > for EnumAttr.
- https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td#L66-L86
- https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Tosa/IR/TosaOpBase.td#L228-L232
It was my mistake to implement several ops without it.
| }]; | ||
| let parameters = (ins | ||
| EnumParameter<DXSA_InlineOperandType>:$type, | ||
| "uint32_t":$components, |
There was a problem hiding this comment.
Why uint32_t and not, say, I32Attr?
There was a problem hiding this comment.
Let's refine this in the separate PR for operands. The inline operand is not using for dcl_constant_buffer any more.
f593309 to
62108d3
Compare
Example: dxsa.dcl_constant_buffer <slot = 0, size = 1>, <immediateIndexed> dxsa.dcl_constant_buffer <id = 0, lbound = 0, ubound = 3, size = 4, space = 1>, <dynamicIndexed> Signed-off-by: Vladimir Shiryaev <tagolog@users.noreply.github.com>
62108d3 to
d14c5f0
Compare
Example:
dxsa.dcl_constant_buffer <slot = 0, size = 1>,
dxsa.dcl_constant_buffer <id = 0, lbound = 0, ubound = 3, size = 4, space = 1>,