[all-commits] [llvm/llvm-project] 023f7c: [mlir][Transforms][NFC] Dialect Conversion: Update...

[Matthias Springer via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 023f7c9382599111244e682ea6b26011dbf7fc56
      https://github.com/llvm/llvm-project/commit/023f7c9382599111244e682ea6b26011dbf7fc56
  Author: Matthias Springer <me at m-sp.org>
  Date:   2024-09-30 (Mon, 30 Sep 2024)

  Changed paths:
    M mlir/lib/Transforms/Utils/DialectConversion.cpp

  Log Message:
  -----------
  [mlir][Transforms][NFC] Dialect Conversion: Update docs for `remapValues` (#110414)

Simplify the nesting structure of "if" checks in `remapValues` and
update the code comments.

This is what the comments stated in case there is no type converter:
```
      // TODO: What we should do here is just set `desiredType` to `origType`
      // and then handle the necessary type conversions after the conversion
      // process has finished. Unfortunately a lot of patterns currently rely on
      // receiving the new operands even if the types change, so we keep the
      // original behavior here for now until all of the patterns relying on
      // this get updated.
```

However, without a type converter it is not possible to perform any
materializations. Furthermore, the absence of a type converter indicates
that the pattern does not care about type legality. Therefore, the
current implementation is correct and this TODO can be removed.

Note: Patterns that actually require a remapped type to match the
original operand type can be equipped with a type converter that maps
each type to itself.

This TODO is outdated:
```
      // TODO: There currently isn't any mechanism to do 1->N type conversion
      // via the PatternRewriter replacement API, so for now we just ignore it.
```
1->N type conversions are already possible as part of block signature
conversions. It is incorrect to just ignore such cases. However, there
is currently no better way to handle 1->N conversions in this function
because of infrastructure limitations. This is now clarified in the
comments.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications