[PATCH] D108433: [CSSPGO] split context string - compiler changes

[Di Mo via Phabricator via llvm-commits]

modimo added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:492
 
-  operator StringRef() const { return FullContext; }
+  operator SampleContextFrames() const { return FullContext; }
   bool hasAttribute(ContextAttributeMask A) { return Attributes & (uint32_t)A; }
----------------
The implicit operator here seems unnecessary given you have `getContextFrames` below providing the same thing. Is this needed?


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:547
+
+  bool operator!=(const SampleContext &That) const { return !(*this == That); }
+
----------------
The == and != operators are not always opposites. If another instance of this object has the same State/Name/FullContext == and != will both evaluate as true.  Should != check on all of these fields/be a direct negation of ==?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108433