[all-commits] [llvm/llvm-project] 1a8bdf: [DAG] Fix in ReplaceAllUsesOfValuesWith

Björn Pettersson via All-commits all-commits at lists.llvm.org
Thu Feb 17 05:31:10 PST 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1a8bdf95a3361a90e49c96c3b4eaeda6462fe878
      https://github.com/llvm/llvm-project/commit/1a8bdf95a3361a90e49c96c3b4eaeda6462fe878
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2022-02-17 (Thu, 17 Feb 2022)

  Changed paths:
    M llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
    A llvm/test/CodeGen/AArch64/dag-ReplaceAllUsesOfValuesWith.ll

  Log Message:
  -----------
  [DAG] Fix in ReplaceAllUsesOfValuesWith

When doing SelectionDAG::ReplaceAllUsesOfValuesWith a worklist is
prepared containing all users that should be updated. Then we use
the RemoveNodeFromCSEMaps/AddModifiedNodeToCSEMaps helpers to handle
recursive CSE updates while doing the replacements.

This patch aims at solving a problem that could arise if the recursive
CSE updates would result in an SDNode present in the worklist is being
removed as a side-effect of morphing a prio user in the worklist.

To examplify such a scenario, imagine that we have these nodes in
the DAG
   t12: i64 = add t8, t11
   t13: i64 = add t12, t8
   t14: i64 = add t11, t11
   t15: i64 = add t14, t8
   t16: i64 = sub t13, t15
and that the t8 uses should be replaced by t11. An initial worklist
(listing the users that should be morphed) could be [t12, t13, t15].
When updating t12 we get
   t12: i64 = add t11, t11
which results in a CSE update that replaces t14 by t12, so we get
   t15: i64 = add t12, t8
which results in a CSE update that replaces t13 by t12, so we get
   t16: i64 = sub t12, t15
and then t13 is removed given that it was the last use of t13.

So when being done with the updates triggered by rewriting the use
of t8 in t12 the t13 node no longer exist. And we used to end up
hitting an assertion when continuing with the worklist aiming at
replacing the t8 uses in t13.

The solution is based on using a DAGUpdateListener, making sure that
we prune a user from the worklist if it is removed during the
recursive CSE updates.

The bug was found using an OOT target. I think the problem is quite
old, even if the particular intree target reproducer added in this
patch seem to pass when using LLVM 13.0.0.

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




More information about the All-commits mailing list