This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Add casting ops to/from generic storage space

Authored by nirvedhmeshram on Sep 19 2022, 1:03 PM.

Diff Detail

Event Timeline

nirvedhmeshram created this revision.Sep 19 2022, 1:03 PM
nirvedhmeshram requested review of this revision.Sep 19 2022, 1:03 PM
antiagainst requested changes to this revision.Sep 19 2022, 3:38 PM
antiagainst added inline comments.

These ops should not derive from SPV_CastOp; they should derive from SPV_Op directly. (SPV_CastOp is meant for numeric value casting; here we are doing pointer casting.)

let arguments = (ins

let results = (outs

let assemblyFormat = [{
  $operand attr-dict `:` type($ptr) `to` type($result)

This is missing availability spec (which says these ops depend on Kernel)?

Did you pull these op definitions in using


Similarly like the above op.


Similarly like the above op.


Note that for this op the spec requires two operands, one pointer, one storage. We have two choices:

  1. Define the op as what the spec requires---two operands. The storage operands kinda duplicates with the storage class in the result type. Pro is that we don't need to manually code the serialization/deserialization.
  2. Define the op as only having one operand. This would make the IR cleaner. It would require customized (de)serialization though.

My preference is 2. (You can find examples of how to write customized (de)serialization in the SerializeOps.cpp or DeseralizeOps.cpp.) But if you want to go with 1 i'm also fine with it.


This check isn't necessary if we have let arguments = (ins SPV_AnyPtr:$pointer) as said in the above.


Same here.


Same here.


We need to additionally verify the storage operand matches the storage class in the result type (if defining the op as taking two operands).

This revision now requires changes to proceed.Sep 19 2022, 3:38 PM
nirvedhmeshram marked 10 inline comments as done.
nirvedhmeshram added inline comments.

Took a stab at option 2.

nirvedhmeshram marked an inline comment as done.Sep 20 2022, 2:03 PM
antiagainst accepted this revision.Sep 20 2022, 2:30 PM

Nice, thanks! I just have a few more nits. Feel free to land after addressing them. :)


Please name this as $pointer to follow the spec.

Similarly for the other two ops.

552 ↗(On Diff #461698)

Use opBuilder.create<spirv::GenericCastToPtrExplicitOp>(..) to avoid the explicit OperationState and string name?

This revision is now accepted and ready to land.Sep 20 2022, 2:30 PM
nirvedhmeshram marked 2 inline comments as done.Sep 20 2022, 3:00 PM