[PATCH] D103550: [SampleFDO] New hierarchical discriminator for FS SampleFDO (llvm-profdata part)

[Rong Xu via Phabricator via llvm-commits]

xur added inline comments.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:935-936
+               "override the cold threshold got from profile summary."));
+  cl::opt<int> FSDiscriminatorPass(
+      "fs-discriminator-pass", cl::init(-1), cl::Hidden,
+      cl::desc("Zero out the discriminator bits for the FS discrimiantor "
----------------
wmi wrote:
> Not sure if it is better to use cl::opt<enum>. Maybe not. But it may still be better to mention the input value correspond to the enum value of llvm::sampleprof::FSDiscriminatorPass.
Using enum, we don't need to code to check the range. Let me change to use enum. 


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:944-948
+  if (Val > static_cast<int>(getNumFSPasses()))
+    exitWithError("FS DiscriminatorPass out of the range.");
+  if (Val == -1)
+    Val = getNumFSPasses();
+  FSDiscriminatorPassVal = static_cast<sampleprof::FSDiscriminatorPass>(Val);
----------------
wmi wrote:
> May be shared with the corresponding part in show_main.
OK. I will move the option definition to a file scope static. 


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

https://reviews.llvm.org/D103550