[PATCH] D70593: [LCSSA] Don't use VH callbacks to invalidate SCEV when creating LCSSA phis

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:322
     // nodes for values used in the no-longer-nested loop.
-    formLCSSA(*OldContainingL, DT, &LI, nullptr);
+    formLCSSA(*OldContainingL, DT, &LI, SE);
 
----------------
DaniilSuchkov wrote:
> This change is required to preserve current behavior in cases when it's valid: sometimes invalidation of SCEV through VH actually worked as intended.
Do you have a test case that breaks when passing nullptr? After a quick scan of the code, it looks like SimpleLoopUnswitch forgets the whole loop when making changes (but I am not sure if that holds for all paths that use formLCSSA). But the SimpleLoopUnswitch tests all seem to pass when passing nullptr.


================
Comment at: llvm/test/Transforms/LCSSA/pr44058.ll:1
+; RUN: opt -passes="verify<scalar-evolution>,lcssa,verify<scalar-evolution>" -verify-scev-strict -S %s
+
----------------
if you're not checking the output, maybe pass `-disable-output`?

Also, can you also run using the legacy pass manager? It's still the default.....


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

https://reviews.llvm.org/D70593