Page MenuHomePhabricator

[MLIR][SPIRV] Add initial support for OpSpecConstantOp.

Authored by ergawy on Nov 27 2020, 7:56 AM.



This commit adds initial support for SPIR-V OpSpecConstantOp
instruction. The following is introdcued:

  • A new spv.specConstantOperation operation consisting of a single

region and of 2 operations within that regions (more details in the
docs of the op itself).

  • A new spv.yield instruction that acts a terminator for


For now, the generic form of the new op is supported (i.e. no custom
parsing or printing). This will be done in a follow up patch.

Diff Detail

Event Timeline

ergawy created this revision.Nov 27 2020, 7:56 AM
ergawy requested review of this revision.Nov 27 2020, 7:56 AM

This is an initial patch to make sure I didn't anything disastrously wrong :D.

ergawy updated this revision to Diff 308365.Nov 30 2020, 7:26 AM
This comment was removed by ergawy.
ergawy updated this revision to Diff 308395.Nov 30 2020, 9:20 AM

Rename to spv.mlir.yield.

antiagainst requested changes to this revision.Dec 1 2020, 12:05 PM

Nice! Just a few nits.


Yields the result computed in .. region back to .. op


Could you use the symbol SpecConstantOperation? (So capitalize the first letter.) (We have a bunch other ops starting with small cases but we should change them to all capitalized to be consistent.)


Wrap around 80 characters


We've checked this in the enclosing op. We can drop the check here?

This revision now requires changes to proceed.Dec 1 2020, 12:05 PM
antiagainst added inline comments.Dec 1 2020, 2:41 PM

This list is fine for now; but it would be nice that if we can have a following up CL to introduce an op trait, like spirv::UsableInSpecConstantOp and define its counterparts in TableGen so that we can mark those op themselves.

You can find examples of op traits in and the corresonding TableGen definition in

ergawy updated this revision to Diff 308937.Dec 2 2020, 4:59 AM
ergawy marked 4 inline comments as done.

Handle review comments.


The verifier for SpecCosntantOperation below doesn't have that check actually. I think doing this here will provide a more localized error message.


Added a TODO.

ergawy updated this revision to Diff 309821.Dec 7 2020, 12:34 AM
  • Add IsolatedFromAbove trait to spv.SpecConstantOperation.
ergawy added inline comments.Dec 7 2020, 12:37 AM

Please feel free to disagree with adding this trait. I added this to make sure we have tight control on what values get used inside the SpecConstantOperation's region and avoid capturing values from outside that aren't not constants.

Verification logic is simpler and more robust this way, I guess.

antiagainst accepted this revision.Dec 8 2020, 6:00 AM
antiagainst added inline comments.

Looks good!

This revision is now accepted and ready to land.Dec 8 2020, 6:00 AM
This revision was landed with ongoing or failed builds.Dec 8 2020, 6:09 AM
This revision was automatically updated to reflect the committed changes.