[PATCH] D78458: [LAA] Move CheckingPtrGroup/PointerCheck outside class (NFC).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:416
-  /// function.  FIXME: once check-generation is moved inside this class (after
-  /// the PtrPartition hack is removed), we could drop const.
-  typedef std::pair<const CheckingPtrGroup *, const CheckingPtrGroup *>
----------------
fhahn wrote:
> Ayal wrote:
> > The const's are not dropped, but the comment is?
> Unfortunately I am not really sure if there's a benefit from dropping the const, unless there is a need to have them non-const.
> 
> I did not spot any const_casts or similar related to that pair and thought we can just drop the FIXME all together. If there's a benefit for them to not be const I am missing, I'll drop it.
Not sure either; the "since checks are generated from CheckingPtrGroups in LAI::addRuntimeChecks which is a const member function" - still holds. This traces back to D11205; @anemet - do you recall this FIXME and/or agree it is now obsolete?


================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:330
+/// two groups.
+struct RuntimePointerCheck {
+  /// Create a new pointer checking group containing a single
----------------
Ah, suggested this struct be renamed RuntimeCheckingPtrGroup instead of RuntimePointerChecking::CheckingPtrGroup, rather than RuntimePointerCheck,


================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:358
+typedef std::pair<const RuntimePointerCheck *, const RuntimePointerCheck *>
+    PointerCheck;
+
----------------
and this typedef be renamed RuntimePointerCheck instead of RuntimePointerChecking::PointerCheck, rather than only PointerCheck.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78458