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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

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.


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

https://reviews.llvm.org/D70228