HomePhabricator

Fix helptext for opt/llc after 14fc20ca6
Concern Raised6cc6e89c11de

Authored by nemanjai on Jan 30 2020, 6:35 AM.

Description

Fix helptext for opt/llc after 14fc20ca6

The commit https://reviews.llvm.org/rG14fc20ca6 added some options to the X86
back end that cause the help text for opt/llc to become much harder to read.
The issue is that the cl::value_desc is part of the option name and is used to
compute the indentation of the description text (i.e. the maximum length option
name is what everything aligns to). Since the commit puts a large number of
characters into that text, everything is aligned to that width.

This patch just reformats the option so that the description is contained in the
description and the list of possible values is within the angle brackets.

Note: the readability issue of the helptext was fixed in commit

70cbf8c71c510077baadcad305fea6f62e830b06, but the re-formatting wasn't
added on that commit so I am still committing this.

Differential revision: https://reviews.llvm.org/D73267

Details

Auditors
lebedev.ri
Committed
nemanjaiJan 30 2020, 6:35 AM
Differential Revision
D73267: Fix helptext for opt/llc after 14fc20ca6
Parents
rG6be9acdfa814: Drop arm triple from test/CodeGen/AArch64/global-merge-hidden-minsize.ll
Branches
Unknown
Tags
Unknown

Event Timeline

lebedev.ri raised a concern with this commit.Feb 1 2020, 11:40 AM
lebedev.ri added subscribers: llvm-commits, nemanjai, reames and 3 others.
lebedev.ri added inline comments.
/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
96

Can this particular line change be reverted please?

  1. All those values are listed literally right afterwards
  2. This option+it's cl::value_desc appear to be the longest in all of help, thus adding more padding between columns..
This commit now has outstanding concerns.Feb 1 2020, 11:40 AM
skan added inline comments.Feb 1 2020, 5:53 PM
/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
96

Do you mean removing "cl::value_desc("jcc, fused, jmp, call, ret, indirect"),"? I think it's okay to remove it.

lebedev.ri added inline comments.Feb 2 2020, 5:05 AM
/llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
96

Yes, specifically cl::value_desc("jcc, fused, jmp, call, ret, indirect"),.
Either removing, or at least roll it back to less duplicative/long cl::value_desc("(plus separated list of types)"),

skan added a comment.Feb 2 2020, 8:47 PM

Remove cl::value_desc("jcc, fused, jmp, call, ret, indirect"), by commit https://reviews.llvm.org/rGdb7d2ab03d9a