[PATCH] D57215: [CodeExtractor] Clear function's assumption cache after extracting blocks from it

[Vedant Kumar via Phabricator via llvm-commits]

vsk accepted this revision.
vsk added reviewers: davidxl, sanjoy.
vsk added a comment.
This revision is now accepted and ready to land.

LGTM, but, please wait for a +1 from someone more familiar with AssumptionCache before committing.



================
Comment at: lib/Transforms/IPO/HotColdSplitting.cpp:202
     AU.addRequired<TargetTransformInfoWrapperPass>();
+    AU.addUsedIfAvailable<AssumptionCacheTracker>();
   }
----------------
sdmitriev wrote:
> vsk wrote:
> > The splitting pass does not explicitly preserve AssumptionCacheTracker. Why does this not invalidate AssumptionCacheTracker correctly? As the splitting pass does not "really" need AssumptionCacheTracker, it seems preferable to just not require it here.
> AssumptionCacheTracker is implemented as immutable pass, so, as I understand, it cannot be invalidated.
I see, thanks for explaining. It looks like it's well-understood that AssumptionCache may need to be invalidated (there's already a clear() method). As most passes do not invalidate it, making AssumptionCacheTracker an ImmutablePass might be the right tradeoff. @hfinkel does that sound right?


================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:184
   struct OutlineRegionInfo {
-    OutlineRegionInfo(SmallVector<BasicBlock *, 8> Region,
+    OutlineRegionInfo(const SmallVector<BasicBlock *, 8> &Region,
                       BasicBlock *EntryBlock, BasicBlock *ExitBlock,
----------------
nit, as this seems unrelated to the main change, perhaps it could be a followup?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57215