[PATCH] D47624: [SimpleLoopUnswitch] Invalidate SCEV after making structural changes to a loop

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks for looking at this!



================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2015
+  // invariants in SCEV for this and any outer loop.
+  // FIXME: Is it OK that unswitching produces invalid loops?!
+  if (Changed && !L->isInvalid())
----------------
Definitely isn't. Feel free to make the FIXME more explicit. Also, do you have a minimal test case? Good to put that in a PR.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2017
+  if (Changed && !L->isInvalid())
+    if (auto *SEWP = getAnalysisIfAvailable<ScalarEvolutionWrapperPass>())
+      SEWP->getSE().forgetTopmostLoop(L);
----------------
We need this to happen for both pass managers. I would suggest passing SCEV into unswitchLoop for both and sinking this invalidation into that common code.


================
Comment at: test/Transforms/SimpleLoopUnswitch/invalidate_scev.ll:1
+; RUN: opt -iv-users -simple-loop-unswitch -indvars -S < %s | FileCheck %s
+; PR37651
----------------
Can you instead just require SCEV before and verify it after unswitch? Would seem more focused.


https://reviews.llvm.org/D47624