[mlir][dxsa] Add sample instruction#182
Conversation
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.
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.
Added: - sample_b - sample_c - sample_c_lz - sample_d - sample_l These have different semantic and operands, so they are modeled as separate instructions. Each also has a clamp_feedback variant with optional clamp and feeback operands.
|
Addressed comments and added other variants of the |
Co-authored-by: Vladimir Shiryaev <tagolog@users.noreply.github.com>
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.