[all-commits] [llvm/llvm-project] ad3b1f: [SCEV] Do not erase LoopUsers. PR53969

[Max Kazantsev via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: ad3b1fe47273f15c721e7d0b125a2c6f0a8283cb
      https://github.com/llvm/llvm-project/commit/ad3b1fe47273f15c721e7d0b125a2c6f0a8283cb
  Author: Max Kazantsev <mkazantsev at azul.com>
  Date:   2022-02-22 (Tue, 22 Feb 2022)

  Changed paths:
    M llvm/lib/Analysis/ScalarEvolution.cpp
    M llvm/test/Transforms/LoopDeletion/pr53969.ll

  Log Message:
  -----------
  [SCEV] Do not erase LoopUsers. PR53969

This patch fixes a logical error in how we work with `LoopUsers` map.
It maps a loop onto a set of AddRecs that depend on it. The Addrecs
are added to this map only once when they are created and put to
the UniqueSCEVs` map.

The only purpose of this map is to make sure that, whenever we forget
a loop, all (directly or indirectly) dependent SCEVs get forgotten too.

Current code erases SCEVs from dependent set of a given loop whenever
we forget this loop. This is not a correct behavior due to the following scenario:

1. We have a loop `L` and an AddRec `AR` that depends on it;
2. We modify something in the loop, but don't destroy it. We still call forgetLoop on it;
3. `AR` is no longer dependent on `L` according to `LoopUsers`. It is erased from
    ValueExprMap` and `ExprValue map, but still exists in UniqueSCEVs;
4. We can later request the very same AddRec for the very same loop again, and get existing
    SCEV `AR`.
5. Now, `AR` exists and is used again, but its notion that it depends on `L` is lost;
6. Then we decide to delete `L`. `AR` will not be forgotten because we have lost it;
7. Just you wait when you run into a dangling pointer problem, or any other kind of problem
   because an active SCEV is now referecing a non-existent loop.

The solution to this is to stop erasing values from `LoopUsers`. Yes, we will maybe forget something
that is already not used, but it's cheap.

This fixes a functional bug and potentially may have negative compile time impact on methods with
huge or numerous loops.

Differential Revision: https://reviews.llvm.org/D120303
Reviewed By: nikic