[mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` #116524

matthias-springer · 2024-11-17T03:39:30Z

This commit updates the internal ConversionValueMapping data structure in the dialect conversion driver to support 1:N replacements. This is the last major commit for adding 1:N support to the dialect conversion driver.

Since #116470, the infrastructure already supports 1:N replacements. But the ConversionValueMapping still stored 1:1 value mappings. To that end, the driver inserted temporary argument materializations (converting N SSA values into 1 value). This is no longer the case. Argument materializations are now entirely gone. (They will be deleted from the type converter after some time, when we delete the old 1:N dialect conversion driver.)

Note for LLVM integration: Replace all occurrences of addArgumentMaterialization (except for 1:N dialect conversion passes) with addSourceMaterialization.

github-actions · 2024-11-17T04:22:20Z

✅ With the latest revision this PR passed the C/C++ code formatter.

…se (#121271) This commit adds a test case that performs two back-to-back 1:N replacements: `(i16) -> (i16, i16) -> ((i16, i16), (i16, i16))`. For the moment, 3 argument materializations are inserted. In the future (when the conversion value mapping supports 1:N), a single target materialization will be inserted. Addresses a [comment](#116524 (comment)) in #116524.

Co-authored-by: Markus Böck <[email protected]>

mlir/lib/Transforms/Utils/DialectConversion.cpp

zero9178 · 2025-01-03T13:57:04Z

mlir/lib/Transforms/Utils/DialectConversion.cpp

@@ -1478,34 +1497,12 @@ Value ConversionPatternRewriterImpl::findOrBuildReplacementValue(
  }
  Value castValue = buildUnresolvedMaterialization(
      MaterializationKind::Source, computeInsertPoint(repl), value.getLoc(),


I feared that this would be the case. I fully agree with all your points, the current approach is probably the best for now.
Could we document some of these constraints somewhere? I feel we are missing comments explaining "why" things are being done the way they are.

zero9178

LGTM!

mlir/lib/Transforms/Utils/DialectConversion.cpp

llvm-ci · 2025-01-03T15:15:49Z

LLVM Buildbot has detected a new failure on builder openmp-offload-sles-build-only running on rocm-worker-hw-04-sles while building flang,mlir at step 5 "compile-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/13959

Here is the relevant piece of the build log for the reference

Step 5 (compile-openmp) failure: build (failure)
...
     for (auto [i, off] : llvm::enumerate(offsets)) {
                      ^
At global scope:
cc1plus: warning: unrecognized command line option ‘-Wno-deprecated-copy’
117.342 [2146/32/4867] Building CXX object tools/mlir/test/lib/Dialect/Test/CMakeFiles/MLIRTestDialect.dir/TestTraits.cpp.o
117.385 [2145/32/4868] Building CXX object tools/mlir/test/lib/Dialect/Test/CMakeFiles/MLIRTestDialect.dir/TestTypes.cpp.o
117.438 [2144/32/4869] Building CXX object tools/mlir/test/lib/Dialect/Test/CMakeFiles/MLIRTestDialect.dir/TestOpsSyntax.cpp.o
117.495 [2143/32/4870] Building CXX object tools/mlir/test/lib/Dialect/Test/CMakeFiles/MLIRTestDialect.dir/TestDialectInterfaces.cpp.o
117.552 [2142/32/4871] Building CXX object tools/mlir/test/lib/Dialect/Test/CMakeFiles/MLIRTestDialect.dir/TestOps.0.cpp.o
117.582 [2141/32/4872] Building CXX object tools/mlir/lib/Transforms/Utils/CMakeFiles/obj.MLIRTransformUtils.dir/DialectConversion.cpp.o
FAILED: tools/mlir/lib/Transforms/Utils/CMakeFiles/obj.MLIRTransformUtils.dir/DialectConversion.cpp.o 
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/mlir/lib/Transforms/Utils -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils -Itools/mlir/include -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include -Iinclude -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -Wundef -Wno-unused-but-set-parameter -Wno-deprecated-copy -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++1z -MD -MT tools/mlir/lib/Transforms/Utils/CMakeFiles/obj.MLIRTransformUtils.dir/DialectConversion.cpp.o -MF tools/mlir/lib/Transforms/Utils/CMakeFiles/obj.MLIRTransformUtils.dir/DialectConversion.cpp.o.d -o tools/mlir/lib/Transforms/Utils/CMakeFiles/obj.MLIRTransformUtils.dir/DialectConversion.cpp.o -c /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp: In member function ‘ValueVector {anonymous}::ConversionValueMapping::lookupOrDefault(mlir::Value, mlir::TypeRange) const’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp:230:26: error: call of overloaded ‘TypeRange(ValueVector&)’ is ambiguous
     if (TypeRange(current) == desiredTypes)
                          ^
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/Support/TypeID.h:20:0,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/MLIRContext.h:13,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/DialectRegistry.h:16,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/Dialect.h:16,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/OpDefinition.h:22,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/Builders.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/PatternMatch.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/Rewrite/FrozenRewritePatternSet.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/Transforms/DialectConversion.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp:9:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/STLExtras.h:1281:3: note: candidate: llvm::detail::indexed_accessor_range_base<DerivedT, BaseT, T, PointerT, ReferenceT>::indexed_accessor_range_base(const llvm::iterator_range<llvm::detail::indexed_accessor_range_base<DerivedT, BaseT, T, PointerT, ReferenceT>::iterator>&) [with DerivedT = mlir::TypeRange; BaseT = llvm::PointerUnion<const mlir::Value*, const mlir::Type*, mlir::OpOperand*, mlir::detail::OpResultImpl*>; T = mlir::Type; PointerT = mlir::Type; ReferenceT = mlir::Type]
   indexed_accessor_range_base(const iterator_range<iterator> &range)
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/OperationSupport.h:23:0,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/Dialect.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/OpDefinition.h:22,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/Builders.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/PatternMatch.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/Rewrite/FrozenRewritePatternSet.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/Transforms/DialectConversion.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp:9:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/TypeRange.h:38:21: note:   inherited here
   using RangeBaseT::RangeBaseT;
                     ^~~~~~~~~~
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/TypeRange.h:42:12: note: candidate: mlir::TypeRange::TypeRange(mlir::ValueRange)
   explicit TypeRange(ValueRange values);
            ^~~~~~~~~
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp: In member function ‘ValueVector {anonymous}::ConversionValueMapping::lookupOrNull(mlir::Value, mlir::TypeRange) const’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/lib/Transforms/Utils/DialectConversion.cpp:274:31: error: call of overloaded ‘TypeRange(ValueVector&)’ is ambiguous
   TypeRange resultTypes(result);
                               ^
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/Support/TypeID.h:20:0,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/mlir/include/mlir/IR/MLIRContext.h:13,

Fix build errors after #116524. ``` error: call of overloaded ‘TypeRange(ValueVector&)’ is ambiguous ```

Since #116524, an integration test started to become flaky (failure rate ~15%). ``` bin/mlir-opt mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier="enable-arm-sve=true enable-runtime-library=false vl=2 reassociate-fp-reductions=true enable-index-optimizations=true" | mlir-cpu-runner --march=aarch64 --mattr="+sve" -e main -entry-point-result=void -shared-libs=./lib/libmlir_runner_utils.so,./lib/libmlir_c_runner_utils.so | bin/FileCheck mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir # executed command: bin/mlir-opt mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir '--sparsifier=enable-arm-sve=true enable-runtime-library=false vl=2 reassociate-fp-reductions=true enable-index-optimizations=true' # .---command stderr------------ # | mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir:71:10: error: null operand found # | %0 = linalg.generic #trait_mul # | ^ # | mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir:71:10: note: see current operation: %70 = "arith.mulf"(<<NULL VALUE>>, %69) <{fastmath = #arith.fastmath<none>}> : (<<NULL TYPE>>, vector<[2]xf64>) -> vector<[2]xf64> # `----------------------------- # error: command failed with exit status: 1 ``` I traced the issue back to the `DenseMap<ValueVector, ValueVector, ValueVectorMapInfo> mapping;` data structure: previously, some `mapping.erase(foo)` calls were unsuccessful (returning `false`), even though the `mapping` contains `foo` as a key.

…ent test case (#121271) This commit adds a test case that performs two back-to-back 1:N replacements: `(i16) -> (i16, i16) -> ((i16, i16), (i16, i16))`. For the moment, 3 argument materializations are inserted. In the future (when the conversion value mapping supports 1:N), a single target materialization will be inserted. Addresses a [comment](llvm/llvm-project#116524 (comment)) in #116524.

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch 2 times, most recently from 16e58e7 to f027116 Compare November 17, 2024 04:19

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from f027116 to 2f3f0ae Compare November 17, 2024 08:31

matthias-springer changed the base branch from users/matthias-springer/1n_pattern to users/matthias-springer/memref_mat_extra_checks November 17, 2024 08:31

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch 2 times, most recently from ef8d04d to 3930f40 Compare November 17, 2024 08:42

This was referenced Nov 18, 2024

[mlir][LLVM] LLVMTypeConverter: Tighten materialization checks #116532

Merged

[mlir][Transforms] Add 1:N matchAndRewrite overload #116470

Merged

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from 3930f40 to ce53d95 Compare November 18, 2024 13:36

matthias-springer changed the base branch from users/matthias-springer/memref_mat_extra_checks to users/matthias-springer/1n_pattern November 18, 2024 13:36

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from ce53d95 to 0bbad0b Compare November 18, 2024 13:40

matthias-springer changed the base branch from users/matthias-springer/1n_pattern to users/matthias-springer/memref_mat_extra_checks November 18, 2024 13:41

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from 0bbad0b to e3946a5 Compare November 18, 2024 13:56

matthias-springer force-pushed the users/matthias-springer/memref_mat_extra_checks branch 2 times, most recently from fe68c3c to 1511e50 Compare November 24, 2024 03:10

Base automatically changed from users/matthias-springer/memref_mat_extra_checks to main November 24, 2024 03:20

matthias-springer mentioned this pull request Nov 27, 2024

[mlir][Transforms] Dialect Conversion: No target mat. for 1:N replacement #117513

Merged

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from e3946a5 to f983e30 Compare December 14, 2024 14:03

matthias-springer changed the base branch from main to users/matthias-springer/do_not_build_target_mat December 14, 2024 14:03

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch 3 times, most recently from 98214ce to 0e985da Compare December 15, 2024 19:28

matthias-springer force-pushed the users/matthias-springer/do_not_build_target_mat branch from f9e8758 to ddc9294 Compare December 15, 2024 19:29

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch 2 times, most recently from eff9c47 to bc93c78 Compare December 15, 2024 21:38

matthias-springer force-pushed the users/matthias-springer/do_not_build_target_mat branch from ddc9294 to 6041464 Compare December 20, 2024 13:48

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from bc93c78 to 53f97f5 Compare December 20, 2024 13:50

matthias-springer marked this pull request as ready for review December 20, 2024 13:58

matthias-springer requested a review from aartbik as a code owner December 20, 2024 13:58

matthias-springer mentioned this pull request Dec 28, 2024

[mlir][Transforms] Dialect Conversion: Add 1:N op replacement test case #121271

Merged

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from 680692f to 5b49dce Compare December 29, 2024 12:25

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch 2 times, most recently from ba9e0e6 to 8e9b48a Compare December 31, 2024 12:33

matthias-springer mentioned this pull request Dec 31, 2024

[mlir][Transforms] Delete 1:N dialect conversion driver #121389

Merged

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from 8e9b48a to d78dfb2 Compare January 3, 2025 12:33

matthias-springer and others added 7 commits January 3, 2025 14:35

ex

e112855

Apply suggestions from code review

6db4cdf

Co-authored-by: Markus Böck <[email protected]>

rebase fixes

e96dbaa

use universal references for map

cf1c9a9

address comments

3ad5421

fix windows build

c411bd0

address comments

209d922

matthias-springer force-pushed the users/matthias-springer/1n_conversion_value_mapping branch from 488d8be to 209d922 Compare January 3, 2025 13:35

zero9178 reviewed Jan 3, 2025

View reviewed changes

zero9178 approved these changes Jan 3, 2025

View reviewed changes

mlir/lib/Transforms/Utils/DialectConversion.cpp Outdated Show resolved Hide resolved

address comments

f43e564

matthias-springer merged commit 3ace685 into main Jan 3, 2025
8 checks passed

matthias-springer deleted the users/matthias-springer/1n_conversion_value_mapping branch January 3, 2025 15:12

matthias-springer added a commit that referenced this pull request Jan 3, 2025

[mlir][Transforms] Fix build after #116524

9970623

Fix build errors after #116524. ``` error: call of overloaded ‘TypeRange(ValueVector&)’ is ambiguous ```

matthias-springer mentioned this pull request Jan 3, 2025

[mlir][Transforms] Fix build after #116524 #121578

Merged

matthias-springer added a commit that referenced this pull request Jan 3, 2025

[mlir][Transforms] Fix build after #116524 (#121578)

faa30be

Fix build errors after #116524. ``` error: call of overloaded ‘TypeRange(ValueVector&)’ is ambiguous ```

matthias-springer added a commit that referenced this pull request Jan 4, 2025

[mlir][Transforms] Fix build after #116524 (part 2)

01d6bd1

matthias-springer mentioned this pull request Jan 4, 2025

[mlir][Transforms] Fix build after #116524 (part 2) #121662

Merged

makslevental mentioned this pull request Jan 15, 2025

[mlir] fix infinite while in 1:N dialect conversion #123122

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` #116524

[mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` #116524

matthias-springer commented Nov 17, 2024 •

edited

Loading

github-actions bot commented Nov 17, 2024 •

edited

Loading

zero9178 Jan 3, 2025

zero9178 left a comment

llvm-ci commented Jan 3, 2025

[mlir][Transforms] Support 1:N mappings in ConversionValueMapping #116524

[mlir][Transforms] Support 1:N mappings in ConversionValueMapping #116524

Conversation

matthias-springer commented Nov 17, 2024 • edited Loading

github-actions bot commented Nov 17, 2024 • edited Loading

zero9178 Jan 3, 2025

Choose a reason for hiding this comment

zero9178 left a comment

Choose a reason for hiding this comment

llvm-ci commented Jan 3, 2025

[mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` #116524

[mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` #116524

matthias-springer commented Nov 17, 2024 •

edited

Loading

github-actions bot commented Nov 17, 2024 •

edited

Loading