[PATCH] D110390: [SCEV] Establish control over disposition caches

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev reclaimed this revision.
mkazantsev added a comment.
This revision now requires changes to proceed.

Reclaiming the revision just to explain what happens. I don't have any other (nearly cheap) idea how to fix this. Maybe you guys do.

The problem is with hypothetical SCEVs, and existing mechanism of forgetting cached results bound to instructions doesn't work. Let me describe what happens when we run test `test/Transforms/LCSSA/pr44058.ll`.

Right before we run `formLCSSAForInstructions`, some block dispositions are already made. The state of cached `BlockDispositions` is following:

  SCEV = ((-1 * %tmp) + ((%tmp + %tmp4) smax %tmp)) ---> bb13 : 2
  SCEV = ((%tmp + %tmp4) smax %tmp) ---> bb13 : 2
  SCEV = (-1 * %tmp) ---> bb13 : 2
  SCEV = (%tmp + %tmp4) ---> bb13 : 2
  SCEV = -1 ---> bb13 : 2
  SCEV = %tmp ---> bb13 : 2
  SCEV = %tmp4 ---> bb13 : 2
  SCEV = 1 ---> bb13 : 2
  SCEV = 0 ---> bb13 : 2

And here is `analyze -scalar-evolution` for the same method:

  Classifying expressions for: @foo
    %tmp = load i32, i32* %arg, align 4
    -->  %tmp U: full-set S: full-set             Exits: <<Unknown>>              LoopDispositions: { %bb3: Variant, %bb13: Invariant }
    %tmp4 = load i32, i32* %arg1, align 4
    -->  %tmp4 U: full-set S: full-set            Exits: <<Unknown>>              LoopDispositions: { %bb3: Variant, %bb13: Invariant }
    %tmp5 = add i32 %tmp4, %tmp
    -->  (%tmp + %tmp4) U: full-set S: full-set           Exits: <<Unknown>>              LoopDispositions: { %bb3: Variant, %bb13: Invariant }
    %tmp9 = add nsw i32 %tmp, 1
    -->  (1 + %tmp) U: full-set S: full-set
    %tmp12 = phi i32 [ 0, %bb3 ], [ %tmp4, %bb10 ]
    -->  ((-1 * %tmp) + ((%tmp + %tmp4) smax %tmp)) U: full-set S: full-set               Exits: <<Unknown>>              LoopDispositions: { %bb3: Variant, %bb13: Invariant }
    %tmp14 = phi i32 [ %tmp15, %bb13 ], [ 0, %bb11 ]
    -->  {0,+,1}<nuw><nsw><%bb13> U: [0,2147483647) S: [0,2147483647)             Exits: (-1 + (1 smax ((-1 * %tmp) + ((%tmp + %tmp4) smax %tmp))))<nsw>          LoopDispositions: { %bb13: Computable, %bb3: Variant }
    %tmp15 = add nuw nsw i32 %tmp14, 1
    -->  {1,+,1}<nuw><nsw><%bb13> U: [1,-2147483648) S: [1,-2147483648)           Exits: (1 smax ((-1 * %tmp) + ((%tmp + %tmp4) smax %tmp)))              LoopDispositions: { %bb13: Computable, %bb3: Variant }

Note that we have this small and amazing `((%tmp + %tmp4) smax %tmp)` that does not correspond to any instruction. However, it still gets into cache.

At some point, `formLCSSAForInstructions` calls `forgetValue`. The following values and SCEVs get wiped out of the map:

  forgetValue called on   %tmp = load i32, i32* %arg, align 4
          Erasing from map:   %tmp = load i32, i32* %arg, align 4 (SCEV = %tmp)
          Erasing from map:   %tmp5 = add i32 %tmp4, %tmp (SCEV = (%tmp + %tmp4))

Note that `(%tmp + %tmp4)` is now dead. However, it is still an argument of our amazing unkillable hypothetical `((%tmp + %tmp4) smax %tmp)`. There is no way that it will ever be removed from `BlocksDisposition` because it is only done for SCEVs that are bound for instructions. The only way to delete it is to wipe out the entire map.

It also seems that the same bug affects *all* other maps we have, and the only legal option for `forgetValue` is to completely invalidate all SCEV.

I see only one potential solution: when we forget memoized results, we should also forget them for all users of a given SCEV. AFAIK there is no API for this currently. And this is potentially, well... Costly.

Any other good ideas? @nikic @reames


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110390/new/

https://reviews.llvm.org/D110390