[PATCH] D59869: [NewPM] Fix a nasty bug with analysis invalidation in the new PM.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h:832-835
+        // FIXME: This is redundant in one case -- the top of the worklist may
+        // *also* be the same SCC we just ran over (and invalidated for). In
+        // that case, we'll end up doing a redundant invalidation here as
+        // a consequence.
----------------
@chandlerc, was it intentional to have overlap/redundancy between `UR.UpdatedC` and `CWorklist`?

Asking because we ran into a pathological case where that redundancy and the one extra pass over a SCC caused significant compile time increase (by couple hours), when we switch a service to use new pass manager. There was no SCC mutation between the last two runs, and we run over that same SCC twice only because of the redundancy. (The one extra last run over it caused its size to grow exponentially, and that slowed down downstream passes)

I tried to tweak this to avoid that redundancy between the two - that resolved the compile time regression from newpm, and all llvm tests passed too (except the analysis counts for CGSCCPassManagerTest needs update, which is expected I think). But wanted to check if that's a sensible approach..




Repository:
  rL LLVM

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

https://reviews.llvm.org/D59869