search

tensorflow / mlir

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

[spirv] [tracking] SPIR-V lowering #303

Open antiagainst opened 4 years ago

antiagainst commented 4 years ago

This is a tracking bug for implementing lowering towards the SPIR-V dialect. It will be periodically updated to provide the current status. Anybody interested in helping is very welcome to pick up tasks here! :)

Common utilities

From GPU dialect

From Std dialect

From Linalg dialect

denis0x0D commented 4 years ago

@antiagainst can you please clarify on "Layout decoration utilities", I thought they exist, do I miss something? Thanks.

antiagainst commented 4 years ago

Oh, sorry I thought I ticked it... Fixed now. Yes we can have a few more things to do there like supporting more rules but I don't think they are of high priority. They can be added later when needed. I would consider the general utility is there for this reason.

Also note that this is a coarse summary right now and expect this to expand. :) We also plan to expand the SPIR-V.md doc to explain more over the lowering internals and instructions on adding an new op lowering. Would highly appreciate your help (as always) on completing the lowering! :)

denis0x0D commented 4 years ago

Yes we can have a few more things to do there like supporting more rules but I don't think they are of high priority.

Thanks for mention this, I completely forget about different rules for layout, sorry about it.

Also note that this is a coarse summary right now and expect this to expand. :) We also plan to expand the SPIR-V.md doc to explain more over the lowering internals and instructions on adding an new op lowering. Would highly appreciate your help (as always) on completing the lowering! :)

Sounds great, I would like to help, if it possible. Thanks!

antiagainst commented 4 years ago

Feel free to take whatever you are interested and not done. Please create an issue; I'll link it here. Thanks! :)

MaheshRavishankar commented 4 years ago

Updated to add non-kernel functions and dynamic shapes as not completed items. I plan to focus on dynamic shapes starting next year.

denis0x0D commented 4 years ago

@MaheshRavishankar as far as I understood the lowering from std->spv mostly done by you, in this case can you please share the task which I can start from and you are not working on it? Thanks.

MaheshRavishankar commented 4 years ago

@denis0x0D : Thanks for picking up some of these tasks. For Std to SPIR-V all except call operation are fairly low hanging fruit. I am not working on this right now. Feel free to pick up any that make sense to you. Thanks!

denis0x0D commented 4 years ago

@MaheshRavishankar nice, thanks!

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar sorry for delay, is it ok if I'll take some bit and cast op lowering from std to spv? Thanks.

antiagainst commented 4 years ago

Sure, please go ahead!

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar can you please clarify on lowering for std.and and std.or, it seems like they have bitwise semantics? Do I miss something? Thanks. https://github.com/llvm/llvm-project/blob/master/mlir/lib/Conversion/StandardToSPIRV/StandardToSPIRV.td#L23

antiagainst commented 4 years ago

Oops, yes indeed as shown by https://github.com/llvm/llvm-project/commit/a8a5c069614e7d45ccb8613ecc74d0d2358b6e19. Good catch! This is a bug that should be fixed. Would you mind to send a PR for it? Thanks!

MaheshRavishankar commented 4 years ago

What's the bug here. The lowering is only supported for bool. So that should be fine right?

denis0x0D commented 4 years ago

@MaheshRavishankar oh sorry, you are right, I missed that Bitwise works for SPV_Integer

antiagainst commented 4 years ago

Oops again. Yes @MaheshRavishankar is correct. I commented too quickly...

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar just to note about patches: https://reviews.llvm.org/D72137 https://reviews.llvm.org/D72205 https://reviews.llvm.org/D72296 Thanks.

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar is it ok if I'll take lowering for br/cond_br? Thanks.

antiagainst commented 4 years ago

@denis0x0D: please feel free! But note that control flow is a bit tricky that we'll need to make sure the generated SPIR-V counterpart is structured. Starting from some simple cases is totally fine.

denis0x0D commented 4 years ago

@antiagainst indeed it's a bit tricky part to lower br/cond_br to SPIR-V, because SPIR-V cfg must be structured. As a reference, I looked how clspv does it - https://github.com/google/clspv/blob/master/lib/SPIRVProducerPass.cpp#L6091, and it performs some additional analysis to compute LoopInfo, IMHO this analysis looks an excessive for MLIR, because MLIR already has some high-level dialect Loop dialect with needed abstractions. So it might be better to lower loop.if -> spv.selection, loop.for -> spv.loop, by doing this we can lower Loop dialect to SPIR-V dialect with respect to this rules https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_structuredcontrolflow_a_structured_control_flow. What do you think? Thanks.

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar oh, I've missed that loop.for is already lowering to spv.loop in GPUToSPIRV part, is it ok if I'll add a lowering loop.if to spv.selection? Thanks.

antiagainst commented 4 years ago

Hi @denis0x0D, sorry for the late reply! Yes indeed with MLIR's multi-level capability when we translate from some high-level dialect to SPIR-V we'd prefer to go through a more structured way to preserve information as much as possible, like what you said with loop dialect (and we are doing that). I guess converting from std control flows could still be useful under certain cases (given that we cannot always guarantee the input is some high-level dialect) but it's not a priority with clear use case right now. If you are interested though, you are still very welcome to contribute (starting from simple cases). But yes if you can handle loop.if to spv.selection that would be much more helpful for the near term. Thanks a lot!

denis0x0D commented 4 years ago

@antiagainst thanks for clarification, I'll take lowering loop.if to spv.selection at first. Thanks!

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar just to note a lowering loop.if to spv.selection https://reviews.llvm.org/D72836 Is it ok if I'll take lowering for composite constant with ranked tensor type to SPIR-V?

antiagainst commented 4 years ago

Thanks @denis0x0D! for https://reviews.llvm.org/D72836. I've approved it. Yes, please feel free to pick up composite constant lowering.

denis0x0D commented 4 years ago

@antiagainst thanks a lot!

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar related to composite constant lowering https://reviews.llvm.org/D72999 https://reviews.llvm.org/D73022

denis0x0D commented 4 years ago

@antiagainst @MaheshRavishankar according to the task "Pass pipeline to go from linalg to spv", I see that @MaheshRavishankar already added something similiar to IREE https://github.com/google/iree/commit/0200efc1afe05238d1c9022eb589baf2e0b69780 is this task still under TODO list for MLIR?

antiagainst commented 4 years ago

@denis0x0D: yes it is. the task means to have some recommended pipeline to go from linalg to spv in MLIR core given that from linalg all the components are in MLIR repo. Mahesh and I will be working on this recently to flesh out more details for the pipeline.

BTW, I've https://reviews.llvm.org/D73349 to add two more GroupNonUniform ops. It would be awesome if you can help to flesh out more GroupNonUniform arithmetic ops after the above CL lands! (I'd assume the ODS definition and parsing/printing/verification code can be reused largely there.)

denis0x0D commented 4 years ago

@antiagainst thanks for clarification!

flesh out more GroupNonUniform* arithmetic ops after the above CL lands

SGTM, thanks! In this case, it seems like it's a right time to return to task mlir-vulkan-runner which was delayed for a long time ago. Thanks!