Page MenuHomePhabricator

[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

Unit TestsFailed

TimeTest
270 msx64 debian > MLIR.Conversion/GPUCommon::lower-launch-func-to-gpu-runtime-calls.mlir
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/mlir-opt /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir --gpu-to-llvm="gpu-binary-annotation=nvvm.cubin" | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Conversion/GPUCommon/lower-launch-func-to-gpu-runtime-calls.mlir
250 msx64 debian > MLIR.Conversion/VectorToROCDL::vector-to-rocdl.mlir
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/mlir-opt /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Conversion/VectorToROCDL/vector-to-rocdl.mlir -convert-vector-to-rocdl | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Conversion/VectorToROCDL/vector-to-rocdl.mlir
120 msx64 windows > MLIR.Conversion/GPUCommon::lower-launch-func-to-gpu-runtime-calls.mlir
Script: -- : 'RUN: at line 1'; c:\ws\w7\llvm-project\premerge-checks\build\bin\mlir-opt.exe C:\ws\w7\llvm-project\premerge-checks\mlir\test\Conversion\GPUCommon\lower-launch-func-to-gpu-runtime-calls.mlir --gpu-to-llvm="gpu-binary-annotation=nvvm.cubin" | c:\ws\w7\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w7\llvm-project\premerge-checks\mlir\test\Conversion\GPUCommon\lower-launch-func-to-gpu-runtime-calls.mlir
100 msx64 windows > MLIR.Conversion/VectorToROCDL::vector-to-rocdl.mlir
Script: -- : 'RUN: at line 1'; c:\ws\w7\llvm-project\premerge-checks\build\bin\mlir-opt.exe C:\ws\w7\llvm-project\premerge-checks\mlir\test\Conversion\VectorToROCDL\vector-to-rocdl.mlir -convert-vector-to-rocdl | c:\ws\w7\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w7\llvm-project\premerge-checks\mlir\test\Conversion\VectorToROCDL\vector-to-rocdl.mlir

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–365

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–365

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