[PATCH] D26389: [SCEV] limit recursion depth of CompareSCEVComplexity

[Daniil Fukalov via llvm-commits]

dfukalov added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:704
+
+  SmallSet<std::pair<const SCEV *, const SCEV *>, 8> EqCache;
   if (Ops.size() == 2) {
----------------
sanjoy wrote:
> IMO a slightly better pattern would have been to have `EqCache` be a `SmallSet` of `std::pair<PointerUnion<Value *, const SCEV *>, PointerUnion<Value *, const SCEV *>>` and to re-use the same cache in `CompareValueComplexity`.
> 
> If you agree, can you do that as a followup change?
I'm not sure this will actually be a re-use: any correct `SCEV *` is not equal to any correct `Value *` (except nullptr values), so the combined cache will contain all of items from both current caches, without memory usage reducing. And if it so, any search in a combined cache will be the same or slower.
E.g. there is an edge case: we can try to find actual value typed `std::pair<const SCEV*, const SCEV*>` in a set that contains `std::pair<Value*, Value*>` only. Do you agree?


https://reviews.llvm.org/D26389