[PATCH] D125008: [llvm-objdump] Print Mnemonic Histogram

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 5 21:31:12 PDT 2022


MaskRay added a comment.

> I don't know what bar there is to add stuff like this but I assume it's quite low given that it's self contained and not trying to match any equivalent GNU options. I can see the use for comparing files certainly.

I think there are two bars:

- whether we can add an option that GNU objdump doesn't have: in my understanding this is low, but we really want to avoid potential conflict with GNU. This means we usually cannot add a short option alias as GNU objdump may take a short option for anything in the future. A descriptive long option usually has no semantic conflict concern.
- whether we should add an option: I think the bar is actually quite high. An option needs to be sufficiently useful to justify its addition.

> I am probably interested in printing more information, like the encoding width, but this seems a good first step.

If we add an option, we want to provide some stability guarantee. Removing an option has an extremely high bar as that may break some users.
So when adding an option we need to very careful. If there is a plan to extend the option, it's best to design it well upfront.
I know that sometimes iterative process may be unavoidable due to engineering reality, in that case perhaps we can call an option experimental.
A new option needs a documentation update in `llvm/docs/CommandGuide/llvm-objdump.rst`

That said, for this case I wonder why we can't just use

  llvm-objdump -d --no-show-raw-insn --no-leading-addr =cat | grep '^ ' | awk '{freq[$1]++; tot++} END{ for(i in freq) print i "\t" freq[i] "\t" (freq[i]/tot) }'

My reasoning is that different users may have vastly different statistics needs.
If it is difficult to come up with something that meets many's needs, I'd more like the user to compose utilities, but I think I can be persuade the other way if you can come up with something useful which seems to benefit a lot from built-in support.


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

https://reviews.llvm.org/D125008



More information about the llvm-commits mailing list