This is an archive of the discontinued LLVM Phabricator instance.

[mlir] replace llvm.mlir.cast with unrealized_conversion_cast
ClosedPublic

Authored by ftynse on Jul 13 2021, 2:05 AM.

Details

Summary

The dialect-specific cast between builtin (ex-standard) types and LLVM
dialect types was introduced long time before built-in support for
unrealized_conversion_cast. It has a similar purpose, but is restricted
to compatible builtin and LLVM dialect types, which may hamper
progressive lowering and composition with types from other dialects.
Replace llvm.mlir.cast with unrealized_conversion_cast, and drop the
operation that became unnecessary.

Also make unrealized_conversion_cast legal by default in
LLVMConversionTarget as the majority of convesions using it are partial
conversions that actually want the casts to persist in the IR. The
standard-to-llvm conversion, which is still expected to run last, cleans
up the remaining casts standard-to-llvm conversion, which is still
expected to run last, cleans up the remaining casts

Diff Detail

Event Timeline

ftynse created this revision.Jul 13 2021, 2:05 AM
ftynse requested review of this revision.Jul 13 2021, 2:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2021, 2:06 AM
rriddle added inline comments.Jul 13 2021, 12:21 PM
mlir/lib/Conversion/LLVMCommon/ConversionTarget.cpp
17

Why the flip in legality here?

mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
1136–1138

Can you move this create into a temporary variable? The formatting isn't great here.

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
328

Why is this commented out?

579–580

Use convertTypes instead?

588–591

Same here.

ftynse updated this revision to Diff 358880.Jul 15 2021, 2:51 AM
ftynse marked 4 inline comments as done.

Address review.

mlir/lib/Conversion/LLVMCommon/ConversionTarget.cpp
17

Most of the users are partial conversions that actually want cast operations to persist. Only the last conversion, typically std-to-llvm, wants to disallow type casts. So allowing them is a more sensible default. This is explained in the commit message.

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
328

Forgot to remove, this is no longer necessary, argument materialization handles this.

nicolasvasilache accepted this revision.Jul 16 2021, 6:11 AM
This revision is now accepted and ready to land.Jul 16 2021, 6:11 AM
mlir/test/Dialect/ArmSVE/memcpy.mlir