[PATCH] D73801: [LoopFission]: Loop Fission Interference Graph (FIG)

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFission/FIG.cpp:106
+
+        DDG.getDependencies(*Src, Dst, Deps);
+        if (all_of(Deps, [](const std::unique_ptr<Dependence> &D) {
----------------
Meinersbur wrote:
> [serious] This calls `DependenceInfo::depends` (a really expensive call) again on each edge after DDG already has done so. Maybe cache in the DDG?
The DDG would have to search through every pair of instructions in the loop. The set of instructions being iterated here is much more reduced than what the DDG goes through, because we only consider instructions that actually have a memory dependence between them (previously detected).  On the other hand, `depends` is more expensive to compute when there is actual memory dependencies so the concern you raise is still valid. 

When designing the DDG we opted for lazy evaluation of dependencies instead of caching the results due to memory consumption concerns. I think we should revisit these trade offs at some point. I'm just not sure this is the right time, given that the algorithms and the implementation is being changed, so any data we collect could go stale and become irrelevant very quickly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73801