Skip to content
This repository has been archived by the owner on Apr 23, 2021. It is now read-only.

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

Open
23 of 38 tasks
antiagainst opened this issue Dec 6, 2019 · 29 comments
Open
23 of 38 tasks

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

antiagainst opened this issue Dec 6, 2019 · 29 comments

Comments

@antiagainst
Copy link
Contributor

antiagainst commented Dec 6, 2019

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

  • Target environment (version, extensions, capabilities)
  • Shader interface ABI (descriptor, storage class, etc.) and its lowering
  • Entry point ABI (dispatch configuration, etc.) and its lowering
  • SPIR-V module and resource variable creation
  • Layout decoration utilities
  • Recommended pass pipeline/recipe
  • Simple end-to-end testing infrastructure

From GPU dialect

  • Kernel function
  • Non-kernel functions
  • Block/grid dim
  • Thread/block id

From Std dialect

  • Std to spv type converter
    • Memref's with dynamic shapes
  • constant
    • scalar/vector
    • composite
  • load, store
  • Binary ops
    • addi, subi, muli
    • divis, remis
    • diviu, remiu
    • float cases
  • comparison ops
    • int eq, ne
    • int slt, sle, sgt, sge
    • int ult, ule, ugt, uge
    • float eq, ne
    • float slt, sle, sgt, sge
    • float ult, ule, ugt, uge
  • Logical ops
    • select
  • Bit ops
  • Cast ops
  • return
  • br
  • cond_br
  • call

From Linalg dialect

  • Pass pipeline to go from linalg to spv
@denis0x0D
Copy link
Contributor

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

@antiagainst
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@MaheshRavishankar
Copy link
Contributor

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

@denis0x0D
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

@MaheshRavishankar nice, thanks!

@denis0x0D
Copy link
Contributor

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

@antiagainst
Copy link
Contributor Author

Sure, please go ahead!

@denis0x0D
Copy link
Contributor

@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
Copy link
Contributor Author

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

@MaheshRavishankar
Copy link
Contributor

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

@denis0x0D
Copy link
Contributor

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

@antiagainst
Copy link
Contributor Author

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

@denis0x0D
Copy link
Contributor

@denis0x0D
Copy link
Contributor

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

@antiagainst
Copy link
Contributor Author

@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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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

@denis0x0D
Copy link
Contributor

@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
Copy link
Contributor Author

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

@denis0x0D
Copy link
Contributor

@antiagainst thanks a lot!

@denis0x0D
Copy link
Contributor

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

@denis0x0D
Copy link
Contributor

@antiagainst @MaheshRavishankar according to the task "Pass pipeline to go from linalg to spv", I see that @MaheshRavishankar already added something similiar to IREE iree-org/iree@0200efc is this task still under TODO list for MLIR?

@antiagainst
Copy link
Contributor Author

@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
Copy link
Contributor

@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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants