Page MenuHomePhabricator

[MLIR][SPIRVToLLVM] Implementation of spv.func conversion, and return ops
ClosedPublic

Authored by georgemitenkov on Jun 16 2020, 5:39 AM.

Details

Summary

This patch provides an implementation for spv.func conversion. The pattern is populated in a separate method added to the pass. At the moment, the type signature conversion only includes the supported types. The conversion pattern also matches SPIR-V function control attributes to LLVM function attributes. Those are modelled as passthrough attributes in LLVM dialect. I propose the following mapping.

  • None: no attributes passed
  • Inline: alwaysinline seems to be the right equivalent (inlinehint is semantically weaker in my opinion)
  • DontInline: noinline
  • Pure and Const: I think those can be modelled as readonly and readnone attributes respectively.

Also, 2 patterns added for return ops conversion (spv.Return for void return and spv.ReturnValue for a single value return).

Diff Detail

Event Timeline

georgemitenkov created this revision.Jun 16 2020, 5:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2020, 5:39 AM
georgemitenkov edited the summary of this revision. (Show Details)Jun 16 2020, 5:40 AM
antiagainst requested changes to this revision.Jun 17 2020, 3:15 PM

Cool, just a few nits.

mlir/lib/Conversion/SPIRVToLLVM/ConvertSPIRVToLLVM.cpp
156

No need to have this comment. Comments should provide more information and explain when things are non-trivial. If the code is obvioius enough, then there is no need to have comments because comments themselves are a maintenance burden. :)

251

You can directly get the function control directly via funcOp.function_control() here. Note that ODS generates handy getters for both operands and attributes listed in the spec.

mlir/test/Conversion/SPIRVToLLVM/func-to-llvm.mlir
31

No need to check the trailing {. Similarly for the rest.

59

No need to check this here? We are testing spv.func converion. This should be convered by spv.ReturnValue's check.

This revision now requires changes to proceed.Jun 17 2020, 3:15 PM
georgemitenkov marked 4 inline comments as done.
  • [MLIR][SPIRVToLLVM] Added straight call to FuncOp function_control() conversion and refactored code style

Removed comments from return ops patterns. Added a call to function_control() directly from funcOp. In tests, removed check on braces and on return op when testing function signature.

antiagainst accepted this revision.Jun 23 2020, 8:16 AM
This revision is now accepted and ready to land.Jun 23 2020, 8:16 AM
This revision was automatically updated to reflect the committed changes.