Skip to content

[mlir][dxsa] Add dcl constant buffer instruction#133

Open
tagolog wants to merge 1 commit into
access-softek:dxsa-mlirfrom
tagolog:dxsa-mlir-dcl_constant_buffer-and-other-ops
Open

[mlir][dxsa] Add dcl constant buffer instruction#133
tagolog wants to merge 1 commit into
access-softek:dxsa-mlirfrom
tagolog:dxsa-mlir-dcl_constant_buffer-and-other-ops

Conversation

@tagolog
Copy link
Copy Markdown

@tagolog tagolog commented May 21, 2026

Example:
dxsa.dcl_constant_buffer <slot = 0, size = 1>,
dxsa.dcl_constant_buffer <id = 0, lbound = 0, ubound = 3, size = 4, space = 1>,

@tagolog tagolog requested a review from asavonic May 21, 2026 05:34
return builder.buildDclOutput(*operand, loc);
}

FailureOr<Instruction> parseDclConstantBuffer(uint32_t opcodeToken,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread mlir/lib/Target/DXSA/BinaryParser.cpp Outdated
if (!accessPattern)
return emitError(loc, "unknown constant buffer access pattern: ")
<< rawAccessPattern;
auto operand = parseInlineOperand();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

Comment thread mlir/lib/Dialect/DXSA/IR/DXSA.cpp Outdated
LogicalResult
InlineOperandAttr::verify(function_ref<InFlightDiagnostic()> emitError,
::mlir::dxsa::InlineOperandType /*type*/,
uint32_t components,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we verify that components can only be 0, 1, or 4?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I plan to handle this in the separate PR that refines operands.

Comment thread mlir/lib/Target/DXSA/BinaryParser.cpp Outdated
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]));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use symbolize to verify range?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This fragment was removed form the PR.

def DXSA_SystemValueNameAttr :
EnumAttr<DXSADialect, DXSA_SystemValueName, "system_value_name"> {
let assemblyFormat = "$value";
let assemblyFormat = "`<` $value `>`";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need angle brackets here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Angle brackets matches the established MLIR convention — < $value > for EnumAttr.

}];
let parameters = (ins
EnumParameter<DXSA_InlineOperandType>:$type,
"uint32_t":$components,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why uint32_t and not, say, I32Attr?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Let's refine this in the separate PR for operands. The inline operand is not using for dcl_constant_buffer any more.

@tagolog tagolog force-pushed the dxsa-mlir-dcl_constant_buffer-and-other-ops branch from f593309 to 62108d3 Compare May 21, 2026 19:45
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>
@tagolog tagolog force-pushed the dxsa-mlir-dcl_constant_buffer-and-other-ops branch from 62108d3 to d14c5f0 Compare May 25, 2026 04:48
@tagolog tagolog changed the title Dxsa mlir dcl constant buffer and other ops [mlir][dxsa] Add dcl constant buffer instruction May 25, 2026
@tagolog tagolog requested a review from asavonic May 25, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants