[PATCH] D70228: [LoopDataPrefetch + SystemZ] Let target decide on prefetching on a per loop basis

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

In D70228#1956859 <https://reviews.llvm.org/D70228#1956859>, @Meinersbur wrote:

> In D70228#1956686 <https://reviews.llvm.org/D70228#1956686>, @jonpa wrote:
>
> > Comments expanded. I hope multiple line doxygen comments are ok for a parameter?
>
>
> Yes; I usually indent/align them, although not done automatically by clang format. For instance, the doxygen for `getMinPrefetchStride` could be improved:
>
>   /// Some HW prefetchers can handle accesses up to a certain constant stride.
>   /// Sometimes prefetching is beneficial even below the HW prefetcher limit,
>   /// and the arguments provided are meant to serve as a basis for deciding this
>   /// for a particular loop.
>   ///
>   /// \param NumMemAccesses        Number of memory accesses in the loop.
>   /// \param NumStridedMemAccesses Number of the memory accesses that
>   ///                              ScalarEvolution could find a known stride
>   ///                              for.
>   /// \param NumPrefetches         Number of software prefetches that will be
>   ///                              emitted as determined by the addresses
>   ///                              involved and the cache line size.
>   /// \param HasCall               True if the loop contains a call.
>   ///
>   /// \return This is the minimum stride in bytes where it makes sense to start
>   ///         adding SW prefetches. The default is 1, i.e. prefetch with any
>   ///         stride.
>   
>
> However, there is a tradeoff involved in number off lines changed in this diff, consistency with the other docygen comments in the same file, number of likes for the entire doxygen comment.


I see :-)  Thanks for the review.


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

https://reviews.llvm.org/D70228