tensorflow / mlir

"Multi-Level Intermediate Representation" Compiler Infrastructure
1.73k stars 257 forks source link

[spirv] Define bit ops #173

Open antiagainst opened 4 years ago

antiagainst commented 4 years ago

We need to have all the bit ops defined in the SPIR-V dialect. See the spec "3.32.14. Bit Instructions" for the list.

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar seems like some bit instructions still not implemented, can you please share, is anyone working on it?

antiagainst commented 4 years ago

I think no one is working on this right now. If you are interested please feel free to pick it up! Thank you very much! :)

denis0x0D commented 4 years ago

@antiagainst thanks!

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar have some quick question about ebnf for shift ops, spec specifies that result type and base type must be the same and shift operand could be any integer type, in this case we have to know the type of shift operand to resolve ssa value while parsing, so can you please suggest the right ebnf form for those operations? It could be something like this, %0 = OpShiftRightLogical %1, %2 : i32, i16 but I'm not sure. Thanks.

antiagainst commented 4 years ago

Hi @denis0x0D, what you proposed looks good to me.

I guess at some point we need to go over the scheme we used for various ops' custom assembly forms. There are inconsistency (regarding trailing types, punctuation usage, etc.) here and there and it's better to come up with a clear plan. It's good that we embed each op's ebnf in the doc so it's clear documented as of today at least. :)

MaheshRavishankar commented 4 years ago

Agreed. Except the op name should be "ShiftRightLogical" . The auto-gen-ed serialization/deserialization relies on this being the case. See here : https://github.com/tensorflow/mlir/blob/b3f8dd39a739d1652524188628a07901eccb8676/include/mlir/Dialect/SPIRV/SPIRVBase.td#L1253

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar thanks! @MaheshRavishankar you are right, the right name is "ShiftRightLogical" :) Thanks!

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar it seems like I've missed some bit ops with shader capability: OpBitFieldSExtract, OpBitFieldInsert, OpBitFieldUExtract Now I'm wondering about ebnf for this ops, for example forOpBitFieldInsert; for this op result id, base id andinsert id must have the same type, but offset andcount must be an any integer scalar type, in this case could I put restriction on offset type and count type to be the same? For example this operation could look like this: %resultID = spv.OpBitFieldInsert %baseID, %insertID, %offsetID, %countID : baseType, offsetType Thanks!

Updated: fixed typo insert ->count

antiagainst commented 4 years ago

Hey @denis0x0D, the spec does not have that requirement (that offset and count should be the same integer type) so I'd prefer we don't in the dialect too. Using different integer signedness in SPIR-V is quite common so we might actually see such a case. Besides having an additional type parameter is just trivial amount of additional parsing work. WDYT? :)

denis0x0D commented 4 years ago

@antiagainst thanks for the answer, sounds good to me, to have an additional type parameter for countID. Thanks!