[PATCH] D58589: [ConstantHoisting] Inline releaseMemory() into ConstantHoistingPass::runImpl to avoid dangling elements in ConstIntInfoVec for new PM

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:950
 
+  releaseMemory();
+
----------------
wmi wrote:
> MaskRay wrote:
> > ormris wrote:
> > > ormris wrote:
> > > > If this only happens in the new pass manager, could this call be moved to ::run? That way, the legacy pass manager doesn't call this function twice.
> > > s/::run/ConstantHoistingPass::run/g
> > Thanks! I think maybe it makes more sense to delete releaseMemory() and inline it into the end of runImpl. I've updated the patch to do that.
> nit: why you feel it is better to inline it? It seems better to me to extract those cleanups into a function.
`releaseMemory()` is called by legacy PM but not by new PM. This `releaseMemory()` doesn't do what people expected: this is not only related to release unused memory but also related to correctness. The data structure should be destroyed after applying on one function as otherwise it would become dangling.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D58589