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

[Hongtao Yu via Phabricator via llvm-commits]

hoy 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; }
----------------
modimo wrote:
> The implicit operator here seems unnecessary given you have `getContextFrames` below providing the same thing. Is this needed?
It's needed for an implicit type cast where a `SampleContext` object is used as a `SampleContextFrames` object.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:547
+
+  bool operator!=(const SampleContext &That) const { return !(*this == That); }
+
----------------
modimo wrote:
> 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 ==?
Can you elaborate more? Why would `!=` return true for two objects having the same State/Name/FullContext?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108433