[PATCH] D54864: Introduce llvm-objdump man page

[Ed Maste via Phabricator via llvm-commits]

emaste marked 5 inline comments as done.
emaste added a comment.

In D54864#1307053 <https://reviews.llvm.org/D54864#1307053>, @Higuoxing wrote:

> lgtm, but seems that some hidden/alias options are not listed here (I could help add them). I am not expert in this area, so I will leave the decision to @kristina


I just based this on the usage output. I will update with added/removed options and we can iterate on it from there.

In D54864#1307061 <https://reviews.llvm.org/D54864#1307061>, @MaskRay wrote:

> BTW, `usr.bin/clang/llvm-objdump/llvm-objdump.1` here is not FreeBSD phabricator :)


Sorry about that :)

Next upload will be from the proper llvm location - since the content is the same I just uploaded the same diff here.



================
Comment at: usr.bin/clang/llvm-objdump/llvm-objdump.1:12
+.Nm llvm-objdump
+.Op Fl -aarch64-neon-syntax Ns = Ns Ar value
+.Op Fl -arch Ns = Ns Ar value
----------------
MaskRay wrote:
> It does not seem very necessary to list all options in SYNOPSIS.
Same comment was raised in a separate review for FreeBSD in https://reviews.freebsd.org/D18309. Will update.


================
Comment at: usr.bin/clang/llvm-objdump/llvm-objdump.1:69
+.Op Fl -unwind-info
+.Op Fl -verify-debug-info
+.Op Fl -verify-dom-info
----------------
MaskRay wrote:
> There is no `-verify-debug-info`
Will delete.


================
Comment at: usr.bin/clang/llvm-objdump/llvm-objdump.1:70
+.Op Fl -verify-debug-info
+.Op Fl -verify-dom-info
+.Op Fl -version
----------------
MaskRay wrote:
> This option is unfortunately pulled in from a static constructor defined in`lib/IR/Dominators.cpp`. It is not an llvm-objdump option.
Will delete.


================
Comment at: usr.bin/clang/llvm-objdump/llvm-objdump.1:218
+.Fl -macho .
+.It Fl -print-after-all
+Print IR after each pass.
----------------
MaskRay wrote:
> `-print-after-all` is a legacy PM option defined in `lib/IR/LegacyPassManager.cpp`. llvm-objdump doesn't need it.
> 
> These `cl::opt` options should at some point be refactored to use tablegen as llvm-objcopy does...
Will delete.


================
Comment at: usr.bin/clang/llvm-objdump/llvm-objdump.1:238
+Seed for the random number generator.
+.It Fl s
+Display the content of each section.
----------------
MaskRay wrote:
> It should be accompanied by `-full-sections`. My preference is that these options are sorted and aligned by their full names and the single-letter aliases follow (not following GNU convention), as what `lld/docs/ld.lld1.1` does:
> 
> ```
> .It Fl -emit-relocs , Fl q
> Generate relocations in the output.
> .It Fl -enable-new-dtags
> Enable new dynamic tags.
> ```
> 
It was pointed out in some ld.lld.1 review that preference should be for sorting by short option, but I believe that some of the short options are still not listed in ld.lld.1.


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

https://reviews.llvm.org/D54864