This is an archive of the discontinued LLVM Phabricator instance.

[mlir][llvm] Ensure immediate usage in intrinsics
ClosedPublic

Authored by Dinistro on May 30 2023, 12:20 AM.

Details

Summary

This commit changes intrinsics that have immarg parameter attributes to
model these parameters as attributes, instead of operands. Using
operands only works if the operation is an llvm.mlir.constant,
otherwise the exported LLVMIR is invalid.

Diff Detail

Event Timeline

Dinistro created this revision.May 30 2023, 12:20 AM
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a reviewer: dcaballe. · View Herald Transcript
Herald added a reviewer: kuhar. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Dinistro requested review of this revision.May 30 2023, 12:20 AM
gysit added a comment.May 30 2023, 4:56 AM

I have only some minor comments.

@ftynse it may make sense you have a look. The change potentially impacts downstream projects.

mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir
301

nit: remaining

mlir/test/Target/LLVMIR/llvmir-intrinsics.mlir
466

Should we have an export test with i32 as well?

Dinistro updated this revision to Diff 526653.May 30 2023, 8:45 AM
Dinistro marked an inline comment as done.

address review comments

gysit accepted this revision.Jun 9 2023, 2:27 AM

Thanks! LGTM

This revision is now accepted and ready to land.Jun 9 2023, 2:27 AM
This revision was automatically updated to reflect the committed changes.
Dinistro marked an inline comment as done.Jun 20 2023, 5:40 AM