[mlir][dxsa] Add sample instruction#182
Conversation
asavonic
force-pushed
the
dxsa-mlir-sampler
branch
from
562d731 to
22f12b4
Compare
|
|
||
| let assemblyFormat = [{ | ||
| $dst `,` $src_address `,` $src_resource `,` $src_sampler | ||
| (`,` $offset^)? (`,` $clamp_feedback^)? |
There was a problem hiding this comment.
It seems to me that two optional attrs could broke roundtrip. I vote to enable roundtrip in the test files and retest.
There was a problem hiding this comment.
Good point. Offset and Clamp/Feedback syntax is not ambiguous. Offset is <u = 1, v = 2, w = 3>, and clamp/feedback are two operands. Added --verify-roundtrip.
| } while (DECODE_IS_D3D10_SB_OPCODE_EXTENDED(*opcodeToken1)); | ||
| } | ||
|
|
||
| // TODO: extended instructions: |
There was a problem hiding this comment.
Partially. There is still #160 conditional instructions that I need to rebase and merge.
| ExtendedInstruction &ext, Location loc) { | ||
| dxsa::SampleOffsetAttr offset; | ||
| if (auto sampleOffset = ext.sampleOffset) { | ||
| offset = dxsa::SampleOffsetAttr::get(context, sampleOffset->u, |
There was a problem hiding this comment.
Maybe we can build attribute outside of this function, to make it interface more consistent?
Rather then passing the structure from parser level?
There is a minor inconsistency here - the clampFeedback is build on the parser layer, but offset is inside the builder.
There was a problem hiding this comment.
That is a good point. I think it makes sense to keep all MLIR construction in the builder, and parsing stuff in the parser. Refactored the code to avoid constructing SampleClampFeedbackAttr in the parser.
|
Addressed comments and added other variants of the |
asavonic
force-pushed
the
dxsa-mlir-sampler
branch
from
e6be522 to
cc0bf04
Compare
Sample instruction takes an address, a resource (texture), a sampler, and writes texture data to the destination register. There are several optional that can be present: - Offset is encoded as an extended opcode. - LOD clamp and feeback for sample_cl_s. The spec mentions clamp and feedback as optional, but DXC decodes them both. It is possible that they are always present as operands, but can be null. Other extended instruction are added for other resource instructions, and not enabled for sample instruction. Feedback (ClampFeedback) and Offset are both supposed to be optional attributes. MLIR parser is not able to handle cases where feedback is present, but offset is absent, so and fails roundtrip verifier tests. Therefore such instructions are separate, and feedback operand is not optional.
asavonic
force-pushed
the
dxsa-mlir-sampler
branch
from
cc0bf04 to
161d8b2
Compare
2 participants
Sample instruction takes an address, a resource (texture), a sampler, and writes texture data to the destination register.
There are several optional that can be present:
The spec mentions clamp and feedback as optional, but DXC decodes them both. It is possible that they are always present as operands, but can be null.
Other extended instruction are added for other resource instructions, and not enabled for sample instruction.