This is an archive of the discontinued LLVM Phabricator instance.

[mlir][llvm] Use interfaces in the translation to LLVMIR.
ClosedPublic

Authored by gysit on Feb 28 2023, 11:56 PM.

Details

Summary

The revision consistently uses the AliasAnalysisOp and AccessGroupOp
interfaces in the translation from MLIR to LLVMIR. It thus drops the
last string based lookups of alias scope and access group attributes.

Depends on D144851

Diff Detail

Event Timeline

gysit created this revision.Feb 28 2023, 11:56 PM
Herald added a project: Restricted Project. · View Herald Transcript
gysit requested review of this revision.Feb 28 2023, 11:56 PM
Dinistro accepted this revision.Mar 1 2023, 4:05 AM

LGTM module minor comment.

Note: Can this be considered an NFC? It does change behaviour for operations that do not implement these interfaces but have the attribute attached.

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

Are there any remaining usages of LLVMDialect::getAccessGroupsAttrName left after this revision? If not, we should consider dropping it to ensure downstream projects will be aware of this change.

1081

As above

This revision is now accepted and ready to land.Mar 1 2023, 4:05 AM
gysit added inline comments.Mar 1 2023, 4:14 AM
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
1010

I assume now that the interface exist we can indeed remove them true (@frgossen)

gysit retitled this revision from [mlir][llvm] Use interfaces in the translation to LLVMIR (NFC). to [mlir][llvm] Use interfaces in the translation to LLVMIR..Mar 2 2023, 1:03 AM
gysit updated this revision to Diff 501812.Mar 2 2023, 2:01 AM

Rebase.

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

Are there any remaining usages of LLVMDialect::getAccessGroupsAttrName left after this revision? If not, we should consider dropping it to ensure downstream projects will be aware of this change.

I will do a follow up revision for removing the strings that can land later.

This revision was automatically updated to reflect the committed changes.