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

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

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



================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:65
 
+extern cl::opt<bool> EnableFSDiscriminator;
+
----------------
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.


================
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) {
----------------
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?


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

https://reviews.llvm.org/D102246