[PATCH] D102246: [SampleFDO] New hierarchical discriminator for Flow Sensitive SampleFDO

[Rong Xu via Phabricator via llvm-commits]

xur added a comment.

In D102246#2754153 <https://reviews.llvm.org/D102246#2754153>, @wmi wrote:

> Can you split the changes under ProfileData into a separate patch?

OK. I will do that.



================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:65
 
+extern cl::opt<bool> EnableFSDiscriminator;
+
----------------
wmi wrote:
> This may introduce undefined variable problem if llvm library user only use LLVMCore and not LLVMProfileData. The locations of definition and declaration need to be swapped.
OK. This makes sense. I'll fix this.


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1771
+  /// (i.e. zero out the B-th and above bits for D (B is 0-based and inclusive).
+  // Example: an input of (0x1FF, 7) returns 0xFF.
+  static unsigned getMaskedDiscriminator(unsigned D, unsigned B) {
----------------
wmi wrote:
> If the first bit is the 0th bit, the 1 in 0x1FF is in the 8th bit and why input B = 7 can clear it?
The comment is not accurate. It should be "Zero out the (B+1)-th and above bits for D. 


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

https://reviews.llvm.org/D102246