This is an archive of the discontinued LLVM Phabricator instance.

[mlir][FuncToLLVM] Fix arg attr memref interaction
ClosedPublic

Authored by Dinistro on Jan 20 2023, 7:13 AM.

Details

Summary

This commit ensures that argument attributes are dropped from types
that are expanded to multiple function arguments. Previously, they
were attached to all arguments belonging to the memref, which
resulted in illegal LLVMIR, e.g., noalias on integers.

Diff Detail

Event Timeline

Dinistro created this revision.Jan 20 2023, 7:13 AM
Dinistro requested review of this revision.Jan 20 2023, 7:13 AM
gysit added inline comments.Jan 20 2023, 7:32 AM
mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
367–368

Could this be done in the place where the argument (=memref) is unpacked to multiple arguments? It seems like this logic is very argument type specific.

Dinistro added inline comments.Jan 20 2023, 9:41 AM
mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
367–368

Unfortunately, the SignatureConversion cannot do that for us. It might make sense to differentiate on the original type and only perform this change when it is a memref type.

Dinistro updated this revision to Diff 491246.Jan 23 2023, 12:50 AM

Changed the handling to drop all argument attributes in the case of a type expanding to multiple arguments

gysit accepted this revision.Jan 23 2023, 1:48 AM

LGTM!

This revision is now accepted and ready to land.Jan 23 2023, 1:48 AM

The diff description no longer matches the change.

Dinistro edited the summary of this revision. (Show Details)Jan 23 2023, 2:02 AM

The diff description no longer matches the change.

Good catch! I changed it accordingly.

This revision was automatically updated to reflect the committed changes.