[PATCH] D68056: [Polly][NFC][ScopBuilder] Move RecordedAssumptions vector to ScopBuilder.

[Dominik Adamski via Phabricator via llvm-commits]

domada added inline comments.


================
Comment at: polly/lib/Support/SCEVAffinator.cpp:120
   this->BB = BB;
+  this->RecordedAssumptions = RecordedAssumptions;
 
----------------
Meinersbur wrote:
> [serious] Assigning the field in one of the methods instead of in the ctor makes the lifetime issue even worse. Is there a reason not to pass `RecordedAssumptions` by function arguments?
Some `RecordedAssumptions` are added inside `visit` and other `visit_type_of_scev functions` functions (for example `visitTruncateExpr` ).  `visit` function and other `visit_type_of_scev functions` are part of` llvm::SCEVVisitor` struct. I cannot modify number of parameters for these functions, because they are part of LLVM infrastructure.

That's why I am trying to add pointer of RecordedAssumption vector to SCEVAffinator. This pointer is visible inside all functions inside SCEVAffinator class and I don't need to pass it as function argument.

I am wondering if it is worth to split SCEVAffinator into two classes? The first class will be responsible only for delivering PWACtx, and the second will be responsible for recording assumptions. The class for collecting assumptions will be instantiated only in ScopBuilder so the problem with object lifetime will be solved. The drawback of this approach is duplicated traversing of some SCEVs.


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

https://reviews.llvm.org/D68056