[PATCH] D64060: [LoopInfo] Extend getExitEdges API

Fri Jul 5 03:05:07 PDT 2019

skatkov marked an inline comment as done.
skatkov added inline comments.

================
Comment at: include/llvm/Analysis/LoopInfo.h:282
+  template <class ParamBlockT>
+  void getExitEdges(SmallVectorImpl<std::pair<ParamBlockT *, ParamBlockT *> > &
+                        ExitEdges) const {
----------------
fhahn wrote:
> skatkov wrote:
> > fhahn wrote:
> > > Is there a reason why the implementation was moved to here? Could it stay in LoopInfoImpl.h?
> > Yes, there is a reason. To support both const and non-const ParamBlockT in SmallVector argument I introduced the new template parameter ParamBockT. As a result if I keep the implementation in LoopInfoImpl.h I have a compilation error due to users includes this header. So the movement to this header is required.
> > 
> On second thought, it looks like getExitEdges has only a single user, so maybe it would be better to just change the original API from const to non-const and update LoopPredication?
> 
>  This would be in line with most of the other APIs, that already return non-const BlockT*, exactly for the reason that most APIs expect non-const blocks.
I can do this.
I just wonder what wrong with movement of this code from LoopInfoImpl.h to LoopInfo.h and support both const and non-const cases?

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

https://reviews.llvm.org/D64060