[PATCH] D20560: [new PM] port LoopAccessAnalysis to new pass manager (part-1)

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

At a meta level, if I've started reviewing a patch, please wait until I have a chance to see your updates before submitting it. I'm sorry I didn't get to it in the early morning, but its still well under 24h turn around and I don't know that its reasonable to expect lower latency.

That's not to say that I don't appreciate all of Adam and Davide's comments -- they're help reviewing is much appreciated -- but as it happens I don't actually think this is the right approach.

See below for more details.


================
Comment at: llvm/trunk/include/llvm/Analysis/LoopAccessAnalysis.h:704-706
@@ +703,5 @@
+private:
+  /// \brief LoopAccessInfo is created on demand. This map caches
+  /// the computed results.
+  DenseMap<Loop *, std::unique_ptr<LoopAccessInfo>> LoopAccessInfoMap;
+
----------------
This doesn't seem like the right design. It leads to the `getInfo().getInfo()` awkward pattern that IMO is only marginally less awkward spelled as `getResult().getInfo()`.

Instead, I think this pretty clearly wants to be a Loop analysis in the new pass manager. I think that the caching and map should be the new pass manager's caching and map, rather than rolling our own in a function analysis manager.

As a consequence, I don't think you need this result type at all. I think the LoopAccessInfo is already the correct result type. I think you'll need the DenseMap and logic around it in the legacy pass manager's function analysis, and you'll want a different query path to use the LoopAnalysisManager directly in the new pass manager.

Does that make sense?


Repository:
  rL LLVM

http://reviews.llvm.org/D20560