[all-commits] [llvm/llvm-project] 2692ea: [MLIR][PDL] Refactor the positions for multi-root ...

[Stanislav Funiak via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 2692eae57428e1136ab58ac4004883245d0623ca
      https://github.com/llvm/llvm-project/commit/2692eae57428e1136ab58ac4004883245d0623ca
  Author: Stanislav Funiak <stano at cerebras.net>
  Date:   2022-01-04 (Tue, 04 Jan 2022)

  Changed paths:
    M mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
    M mlir/lib/Conversion/PDLToPDLInterp/Predicate.cpp
    M mlir/lib/Conversion/PDLToPDLInterp/Predicate.h
    M mlir/lib/Conversion/PDLToPDLInterp/PredicateTree.cpp
    M mlir/test/Conversion/PDLToPDLInterp/pdl-to-pdl-interp-matcher.mlir

  Log Message:
  -----------
  [MLIR][PDL] Refactor the positions for multi-root patterns.

When the original version of multi-root patterns was reviewed, several improvements were made to the pdl_interp operations during the review process. Specifically, the "get users of a value at the specified operand index" was split up into "get users" and "compare the users' operands with that value". The iterative execution was also cleaned up to `pdl_interp.foreach`. However, the positions in the pdl-to-pdl_interp lowering were not similarly refactored. This introduced several problems, including hard-to-detect bugs in the lowering and duplicate evaluation of `pdl_interp.get_users`.

This diff cleans up the positions. The "upward" `OperationPosition` was split-out into `UsersPosition` and `ForEachPosition`, and the operand comparison was replaced with a simple predicate. In the process, I fixed three bugs:
1. When multiple roots were had the same connector (i.e., a node that they shared with a subtree at the previously visited root), we would generate a single foreach loop rather than one foreach loop for each such root. The reason for this is that such connectors shared the position. The solution for this is to add root index as an id to the newly introduced `ForEachPosition`.
2. Previously, we would use `pdl_interp.get_operands` indiscriminately, whether or not the operand was variadic. We now correctly detect variadic operands and insert `pdl_interp.get_operand` when needed.
3. In certain corner cases, we would trigger the "connector has not been traversed yet" assertion. This was caused by not inserting the values during the upward traversal correctly. This has now been fixed.

Reviewed By: Mogball

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