Page MenuHomePhabricator

[mlir] Intrinsics generator: use TableGen-defined builder function
ClosedPublic

Authored by ftynse on Feb 20 2020, 4:23 AM.

Details

Summary

Originally, intrinsics generator for the LLVM dialect has been producing
customized code fragments for the translation of MLIR operations to LLVM IR
intrinsics. LLVM dialect ODS now provides a generalized version of the
translation code, parameterizable with the properties of the operation.
Generate ODS that uses this version of the translation code instead of
generating a new version of it for each intrinsic.

Depends On D74889

Diff Detail

Event Timeline

ftynse created this revision.Feb 20 2020, 4:23 AM

Thanks, nice cleanup!

mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
168

What is the story to convert MLIR operations with multiple return values into intrinsics returning multiple values (structures)?

kariddi added inline comments.Feb 21 2020, 4:26 PM
mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
168

If everything would pan out just fine for multiple results I suggest renaming "hasResult" with "hasResults"

ftynse marked an inline comment as done.Feb 24 2020, 5:45 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
168

This should happen within MLIR, on in the translation from MLIR to LLVM IR. Since we want to model LLVM IR closely in the LLVM dialect, intrinsics that return a struct (that consumers will have to "unpack" with extractvalue) are modeled as Ops that return a single value of the corresponding type. Anyone is free to introduce a slightly higher-level operation that has multiple results instead, which would be lowered to this intrinsic. We have support for generating the "unpacking" since day 1 (https://github.com/llvm/llvm-project/commit/6d37a255e2e445671e30b7471283b985cbb34727#diff-bab3c5c46bbcf237dbcfcd8a2a668454R354).

kariddi accepted this revision.Feb 24 2020, 9:35 AM
This revision is now accepted and ready to land.Feb 24 2020, 9:35 AM
antiagainst accepted this revision.Feb 24 2020, 11:15 AM

Nice clean up!

This revision was automatically updated to reflect the committed changes.

Cool, thanks Alex!