This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Async] Add option to LLVM lowering to use opaque pointers
ClosedPublic

Authored by zero9178 on Feb 9 2023, 8:32 AM.

Details

Summary

Part of https://discourse.llvm.org/t/rfc-switching-the-llvm-dialect-and-dialect-lowerings-to-opaque-pointers/68179

This patch adds the pass option 'use-opaque-pointers' to allow the dialect conversion from async to LLVM to create LLVM opaque pointers instead of typed pointers.
The gist of the changes boil down to having to propagate the choice of whether opaque or typed pointers should be used, to various helper functions that then either create typed pointers or opaque pointers.
This sadly creates a bit of a code duplication in comparison to other patches in this series, which I think is mostly unavoidable however, since a lot of the patterns in this lowering require the use of the AsyncTypeConverter, instead of the LLVMTypeConverter.

Besides that, the tests have been converter to opaque pointers with one file with typed pointer support having been created as regression tests.

Diff Detail

Event Timeline

zero9178 created this revision.Feb 9 2023, 8:32 AM
zero9178 requested review of this revision.Feb 9 2023, 8:32 AM
gysit accepted this revision.Feb 9 2023, 11:42 PM

nice! LGTM modulo some nit comments.

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
74

nit: let's maybe already change to !llvm.ptr<i8> -> !llvm.ptr?

76–77

nit: drop i8*?

415–416

nit: I would be tempted to rename this to ´ptrType´ here and below?

922–926

nit: it seems like the mlir namespace is redundant here and below?

1128

nit: drop mlir:: ?

This revision is now accepted and ready to land.Feb 9 2023, 11:42 PM
zero9178 updated this revision to Diff 496410.Feb 10 2023, 3:40 AM
zero9178 marked 5 inline comments as done.

Address review comments