[PATCH] D111533: [SCEV] Invalidate user SCEVs along with operand SCEVs to avoid cache corruption

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:12849
   SmallPtrSet<const SCEV *, 8> ToForget(SCEVs.begin(), SCEVs.end());
+  SmallVector<const SCEV *, 8> Worklist(SCEVs.begin(), SCEVs.end());
+
----------------
nikic wrote:
> Might make sense to initialize this from `ToForget` to avoid duplicates in the initial worklist?
Good catch!


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:12877
           if (any_of(ToForget,
                      [&BEInfo](const SCEV *S) { return BEInfo.hasOperand(S); }))
             Map.erase(I++);
----------------
nikic wrote:
> nikic wrote:
> > Hm ... we don't really need to consider user SCEVs here, as the used SCEV will be part of the operands as well.
> > 
> > Alternatively, if we're already considering user SCEVs, we no longer need the hasOperand() approach, we could directly check whether `ENT.ExactNotTaken == S`.
> To finish the thought, we could do something like `for (const auto &EI : BEInfo.ExitNotTaken) if (ToForget.contains(EI.ExactNotTaken)) erase()`, which makes this approximately O(exits) rather than O(loops * scevs). In any case, that's for a followup...
Thanks for the idea. I'll try to follow-up on that.


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

https://reviews.llvm.org/D111533