[PATCH] D118181: [InstrProf][Correlate] Verify debug info with llvm-profdata show

Wed Jan 26 10:47:08 PST 2022

ellis added inline comments.

================
Comment at: compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c:2
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
+// RUN: llvm-profdata show --debug-info-correlation --detailed-summary --show-prof-sym-list %t | FileCheck %s
+
----------------
phosek wrote:
> kyulee wrote:
> > It appears there are many similar names, which is confusing to remember.
> > The compilation uses `-debug-info-correlate`.
> > For `llvm-profdata merge`, use `-debug-info=<debug file>`.
> > Now, `llvm-profdata show` use `-debug-info-correlation`. This usage actually specifies `<debug file>` in the place of `<profile file>` which is a required argument to `llvm-profdata show`. 
> > One thing I can think about is to use `llvm-profdata show -debug-info=<debug file>` while allowing to omit the required `<profile file>`, which becomes optional. But this doesn't look ideal, either. 
> > I don't have a strong suggestion, but I wish some more consistent naming. 
> I agree and in particular I'd prefer the last suggestion, that is making `<profile file>` optional when `-debug-info=<debug file>` is specified. I think that changing the meaning of the positional argument (that is either a profile or a binary) depending on other arguments is more confusing.
I'm happy with using `-debug-info=<debug file>`. Originally I wanted to avoid removing the `cl::Required` in the `Filename` option, but this really isn't a big deal.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118181