[PATCH] D103041: [SampleFDO] New hierarchical discriminator for FS SampleFDO (ProfileData part)

[Rong Xu via Phabricator via llvm-commits]

xur marked 3 inline comments as done.
xur added inline comments.


================
Comment at: llvm/include/llvm/Support/Discriminator.h:55
+// Define the High bit for each FS pass.
+const unsigned char FSPassHighBit[] = {7, 13, 19, 25, 31};
+
----------------
wmi wrote:
> Can we use unsigned int instead so no implicit conversion when FSPassHighBit is used? And how about using std::array so we can use FSPassHighBit.at(i) in FSPassBitBegin and FSPassBitEnd which will have array bounds check.
OK. This sounds good to me. Will change.


================
Comment at: llvm/include/llvm/Support/Discriminator.h:91-93
+  if (N == 31)
     return 0xFFFFFFFF;
   return (1 << (N + 1)) - 1;
----------------
wmi wrote:
> Nit: For N==31, (1U << (N + 1)) - 1 is 0xFFFFFFFF so no if (N == 31) is needed.
Yes. using 1U is a good idea. I will change.


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

https://reviews.llvm.org/D103041