HomePhabricator

[llvm-size] Switch command line parsing from llvm::cl to OptTable
Needs Verification47db32e542eb

Authored by MaskRay on Jul 9 2021, 10:26 AM.

Description

[llvm-size] Switch command line parsing from llvm::cl to OptTable

Part of https://lists.llvm.org/pipermail/llvm-dev/2021-July/151622.html
"Binary utilities: switch command line parsing from llvm::cl to OptTable"

  • --totals=false and --totals=0 cannot be used. Omit the option.
  • --help-list is removed. This is a cl:: specific option.

OptTable avoids global option collision if we decide to support multiplexing for binary utilities.

Note: because the tool is simple, and its long options are uncommon, I just drop
the one-dash forms except -arch <value> (Darwin style).

Reviewed By: jhenderson

Differential Revision: https://reviews.llvm.org/D105598

Details

Auditors
Meinersbur
Committed
MaskRayJul 9 2021, 10:26 AM
Reviewer
jhenderson
Differential Revision
D105598: [llvm-size] Switch command line parsing from llvm::cl to OptTable
Parents
rG214f63b2729d: [gn build] Port 0e09a41b415b
Branches
Unknown
Tags
Unknown

Event Timeline

Herald added a subscriber: Restricted Project. ยท View Herald TranscriptJul 9 2021, 10:27 AM
Meinersbur raised a concern with this commit.Jul 14 2021, 8:02 AM
Meinersbur added a subscriber: Meinersbur.

This patch seem to have invalidated the llvm-size command line as used by the llvm-test-suite. See for instance: https://lab.llvm.org/buildbot/#/builders/102/builds/2163

/home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/llvm.obj/bin/llvm-size -format=sysv /home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/test-suite.build/SingleSource/UnitTests/vla
Could not collect metric with <function _getCodeSize at 0x7faef3ade040>
Traceback (most recent call last):
  File "/home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/test-suite.src/litsupport/testplan.py", line 119, in _executePlan
    additional_metrics = metric_collector(context)
  File "/home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/test-suite.src/litsupport/modules/codesize.py", line 17, in _getCodeSize
    out = testplan.check_output(cmdline).decode('utf-8', errors='ignore')
  File "/home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/test-suite.src/litsupport/testplan.py", line 166, in check_output
    return subprocess.check_output(commandline, *aargs, **dargs)
  File "/usr/lib/python3.8/subprocess.py", line 415, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/llvm.obj/bin/llvm-size', '-format=sysv', '/home/buildslave/buildslave/polly-x86_64-gce2/polly-x86_64-linux-test-suite/test-suite.build/SingleSource/UnitTests/vla']' returned non-zero exit status 1.

Could you either allow the one-dash command flags again or fix the llvm-test-suite to use the two-dash options?

This commit now has outstanding concerns.Jul 14 2021, 8:02 AM

Thanks.

The title of https://reviews.llvm.org/rTd73ac5f0188cbfaf7a29981d94dcc7434b7ea66b refers to llvm-nm, but fixes the call of llvm-size?!?

MaskRay requested verification of this commit.Jul 14 2021, 10:09 AM
This commit now requires verification by auditors.Jul 14 2021, 10:09 AM

The title of https://reviews.llvm.org/rTd73ac5f0188cbfaf7a29981d94dcc7434b7ea66b refers to llvm-nm, but fixes the call of llvm-size?!?

My bad, the title should say llvm-size.