[PATCH] D50377: [LICM] Use ICFLoopSafetyInfo in LICM

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko added a comment.

Current patch seems to have comments like "this block doesn't need invalidation". Maybe sink this decision down to the SafetyInfo implementation? It should reduce the complexity of the user: just call invalidate every time you delete/insert instructions and you would be fine.



================
Comment at: lib/Transforms/Scalar/LICM.cpp:407-409
+        SafetyInfo->dropCachedInfo(I.getParent());
         CurAST->deleteValue(&I);
         I.eraseFromParent();
----------------
I'd suggest introducing a helper for removing the instruction. We need to do some invalidation every time an instruction is removed. Having a helper would help to avoid missing invalidation in new code.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:1522-1525
+  // Drop all cached info regarding LoopUses.
+  for (auto *I : LoopUses)
+    SafetyInfo->dropCachedInfo(I->getParent());
+
----------------
Having invalidation in Promoter's callbacks (like instructionDeleted) seems more natural. We invalidate CurAST there.


https://reviews.llvm.org/D50377