[all-commits] [llvm/llvm-project] a65514: [mlir] Toy tutorial: insert terminators at the end...

Wed May 20 07:15:08 PDT 2020

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: a655144f57f8b6a9d5f4ab42b425b1f284353487
      https://github.com/llvm/llvm-project/commit/a655144f57f8b6a9d5f4ab42b425b1f284353487
  Author: Alex Zinenko <zinenko at google.com>
  Date:   2020-05-20 (Wed, 20 May 2020)

  Changed paths:
    M mlir/examples/toy/Ch6/mlir/LowerToLLVM.cpp
    M mlir/examples/toy/Ch7/mlir/LowerToLLVM.cpp

  Log Message:
  -----------
  [mlir] Toy tutorial: insert terminators at the end of the loop during rewrite

When creating temporary `scf.for` loops in `toy.print` lowering, the block
insertion point was erronously set up to the beginning of the block rather than
to its end, contradicting the comment just above the insertion point change.
The code was nevertheless operational because `scf.for` was setting up its
`scf.yield` terminator in an opaque to the pattern rewriting infrastructure
way. Now that it is about to change, the problem would have been exposed and
lead to conversion failures.

Differential Revision: https://reviews.llvm.org/D80133

  Commit: 5d5df06aac58cfa83ae3d22d01cfc1b25b7a656c
      https://github.com/llvm/llvm-project/commit/5d5df06aac58cfa83ae3d22d01cfc1b25b7a656c
  Author: Alex Zinenko <zinenko at google.com>
  Date:   2020-05-20 (Wed, 20 May 2020)

  Changed paths:
    M mlir/lib/Transforms/DialectConversion.cpp
    M mlir/test/Transforms/test-legalizer.mlir
    M mlir/test/lib/Dialect/Test/TestOps.td
    M mlir/test/lib/Dialect/Test/TestPatterns.cpp

  Log Message:
  -----------
  [mlir] DialectConversion: avoid double-free when rolling back op creation

Dialect conversion infrastructure may roll back op creation by erasing the
operations in the reverse order of their creation. While this guarantees uses
of values will be deleted before their definitions, this does not guarantee
that a parent operation will not be deleted before its child. (This may happen
in case of block inlining or if child operations, such as terminators, are
created in the parent's `build` function before the parent itself.) Handle the
parent/child relationship between ops by removing all child ops from the blocks
before erasing the parent. The child ops remain live, detached from a block,
and will be safely destroyed in their turn, which may come later than that of
the parent.

Differential Revision: https://reviews.llvm.org/D80134

  Commit: df48026b4c30d88cc1221883631ac8aa7c4c376b
      https://github.com/llvm/llvm-project/commit/df48026b4c30d88cc1221883631ac8aa7c4c376b
  Author: Alex Zinenko <zinenko at google.com>
  Date:   2020-05-20 (Wed, 20 May 2020)

  Changed paths:
    M mlir/lib/Transforms/DialectConversion.cpp
    M mlir/test/Transforms/test-legalizer.mlir
    M mlir/test/lib/Dialect/Test/TestPatterns.cpp

  Log Message:
  -----------
  [mlir] DialectConversion: support erasing blocks

PatternRewriter has support for erasing a Block from its parent region, but
this feature has not been implemented for ConversionPatternRewriter that needs
to keep track of and be able to undo block actions. Introduce support for
undoing block erasure in the ConversionPatternRewriter by marking all the ops
it contains for erasure and by detaching the block from its parent region. The
detached block is stored in the action description and is not actually deleted
until the rewrites are applied.

Differential Revision: https://reviews.llvm.org/D80135

  Commit: 57cbeaa8b5de01cf72e739df467fb47f3867e1af
      https://github.com/llvm/llvm-project/commit/57cbeaa8b5de01cf72e739df467fb47f3867e1af
  Author: Alex Zinenko <zinenko at google.com>
  Date:   2020-05-20 (Wed, 20 May 2020)

  Changed paths:
    M mlir/examples/toy/Ch5/mlir/LowerToAffineLoops.cpp
    M mlir/examples/toy/Ch6/mlir/LowerToAffineLoops.cpp
    M mlir/examples/toy/Ch6/mlir/LowerToLLVM.cpp
    M mlir/examples/toy/Ch7/mlir/LowerToAffineLoops.cpp
    M mlir/examples/toy/Ch7/mlir/LowerToLLVM.cpp
    M mlir/lib/Conversion/AffineToStandard/AffineToStandard.cpp
    M mlir/lib/Conversion/GPUToSPIRV/ConvertGPUToSPIRV.cpp

  Log Message:
  -----------
  [mlir] Erase or clear blocks through ConversionPatternRewriter when applicable

Multiple places in the code base were erasing Blocks or operations in them
using in-place modifications (`Block::erase` or `Block::clear`) unknown to
ConversionPatternRewriter. These operations could not be undone if the pattern
failed and could lead to inconsistent in-memory state of the IR with dangling
pointers. Use `ConversionPatternRewriter::eraseOp` and `::eraseBlock` instead.

Differential Revision: https://reviews.llvm.org/D80136

  Commit: a7d88a90386d6c4b456a68710ea48069b5d3a0bb
      https://github.com/llvm/llvm-project/commit/a7d88a90386d6c4b456a68710ea48069b5d3a0bb
  Author: Alex Zinenko <zinenko at google.com>
  Date:   2020-05-20 (Wed, 20 May 2020)

  Changed paths:
    M mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
    M mlir/test/Conversion/SCFToStandard/convert-to-cfg.mlir

  Log Message:
  -----------
  [mlir] SCFToStandard: support any ops in and around the control flow ops

Originally, the SCFToStandard conversion only declared Ops from the Standard
dialect as legal after conversion. This is undesirable as it would fail the
conversion if the SCF ops contained ops from any other dialect. Furthermore,
this would be problematic for progressive lowering of `scf.parallel` to
`scf.for` after `ensureRegionTerminator` is made aware of the pattern rewriting
infrastructure because it creates temporary `scf.yield` operations declared
illegal. Change the legalization target to declare any op other than `scf.for`,
`scf.if` and `scf.parallel` legal.

Differential Revision: https://reviews.llvm.org/D80137

  Commit: 3ccf4a5bd1099bfba544bec7ddbe610cc9531bb2
      https://github.com/llvm/llvm-project/commit/3ccf4a5bd1099bfba544bec7ddbe610cc9531bb2
  Author: Alex Zinenko <zinenko at google.com>
  Date:   2020-05-20 (Wed, 20 May 2020)

  Changed paths:
    M mlir/include/mlir/IR/OpDefinition.h
    M mlir/lib/IR/Operation.cpp

  Log Message:
  -----------
  [mlir] ensureRegionTerminator: take OpBuilder

The SingleBlockImplicitTerminator op trait provides a function
`ensureRegionTerminator` that injects an appropriate terminator into the block
if necessary, which is used during operation constructing and parsing.
Currently, this function directly modifies the IR using low-level APIs on
Operation and Block. If this function is called from a conversion pattern,
these manipulations are not reflected in the ConversionPatternRewriter and thus
cannot be undone or, worse, lead to tricky memory errors and malformed IR.
Change `ensureRegionTerminator` to take an instance of `OpBuilder` instead of
`Builder`, and use it to construct the block and the terminator when required.
Maintain overloads taking an instance of `Builder` and creating a simple
`OpBuilder` to use in parsers, which don't have an `OpBuilder` and cannot
interact with the dialect conversion mechanism. This change was one of the
reasons to make `<OpTy>::build` accept an `OpBuilder`.

Differential Revision: https://reviews.llvm.org/D80138

Compare: https://github.com/llvm/llvm-project/compare/19e5b2bccb4c...3ccf4a5bd109