This is an archive of the discontinued LLVM Phabricator instance.

[mlir][LLVM] Replace readnone with memory effects
ClosedPublic

Authored by Dinistro on Jan 18 2023, 6:18 AM.

Details

Summary

This commit introduces LLVM's MemoryEffects attribute and replaces the
deprecated usage of llvm.readnone in the LLVM dialect.
The absence of the attribute on a LLVMFuncOp implies that it might
access all kinds of memory. This semantic corresponds to llvm::Function's
behaviour.

Depends on D142002

Diff Detail

Event Timeline

Dinistro created this revision.Jan 18 2023, 6:18 AM
Herald added a project: Restricted Project. · View Herald Transcript
Dinistro requested review of this revision.Jan 18 2023, 6:18 AM
gysit added inline comments.Jan 18 2023, 7:01 AM
mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
386

nit: I think most other attributes use camelCase for LLVM attribute names infavor of using them as is.

mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
390

nit: I would compute the attribute here and then use setMemoryAttr on the newFuncOp further below. That way we do not use the string to identify the right attribute.

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
914

It does not seem like the function ever return failure?

mlir/test/Target/LLVMIR/function-attributes.mlir
1–18 ↗(On Diff #490129)

is this file accidentally here? Generally it may be a good idea to factor out a separate test for the function attributes? However this seems to be a copy?

Dinistro updated this revision to Diff 490151.Jan 18 2023, 7:26 AM
Dinistro marked 4 inline comments as done.

addressing review comments

gysit accepted this revision.Jan 18 2023, 8:24 AM

LGTM!

This revision is now accepted and ready to land.Jan 18 2023, 8:24 AM
This revision was automatically updated to reflect the committed changes.