Page MenuHomePhabricator

[mlir] LLVM dialect: use addressof instead of constant to create function pointers
ClosedPublic

Authored by ftynse on Fri, Jun 26, 10:06 AM.

Details

Summary

llvm.mlir.constant was originally introduced as an LLVM dialect counterpart
to std.constant. As such, it was supporting "function pointer" constants
derived from the symbol name. This is different from std.constant that allows
for creation of a "function" constant since MLIR, unlike LLVM IR, supports
this. Later, llvm.mlir.addressof was introduced as an Op that obtains a
constant pointer to a global in the LLVM dialect. It naturally extends to
functions (in LLVM IR, functions are globals) and should be used for defining
"function pointer" values instead.

Fixes PR46344.

Depends On D82666

Diff Detail

Event Timeline

ftynse created this revision.Fri, Jun 26, 10:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jun 26, 10:06 AM
silvas accepted this revision.Fri, Jun 26, 10:45 AM

It would be good to update https://mlir.llvm.org/docs/Dialects/LLVM/

  • section on llvm.mlir.addressof could use an example of function pointers
  • would be nice to add a section describing the high level model of how LLVM's notion of constant gets mapped into the mlir dialect (don't need to say more than "for these cases, llvm.mlir.addressof is used, for these other cases, llvm.mlir.constant is used"), so that people Ctrl-F'ing quickly find out the mental model.

LGTM with those changes.

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
1549

nit: for consistency, no underscore in address_of

1554

notifyMatchFailure?

1566

notifyMatchFailure?

This revision is now accepted and ready to land.Fri, Jun 26, 10:45 AM
rriddle added inline comments.Fri, Jun 26, 12:08 PM
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
861

nit: static

ftynse updated this revision to Diff 274033.Mon, Jun 29, 3:16 AM
ftynse marked 4 inline comments as done.

Address review

It would be good to update https://mlir.llvm.org/docs/Dialects/LLVM/

Done.

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
861

Is it necessary for function templates?

This revision was automatically updated to reflect the committed changes.
rriddle added inline comments.Mon, Jun 29, 11:07 AM
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
861

Yes.