Page MenuHomePhabricator

[RISCV] Support llvm-objdump -M no-aliases and -M numeric
ClosedPublic

Authored by lenary on Aug 13 2019, 6:03 AM.

Details

Summary

Now that llvm-objdump allows target-specific options, we match the
no-aliases and numeric options for RISC-V, as supported by GNU objdump.

This is done by overriding the variables used for the command-line options, so
that the command-line options are still supported.

This patch updates all tests using llvm-objdump -riscv-no-aliases to use
llvm-objdump -M no-aliases.

Diff Detail

Event Timeline

lenary created this revision.Aug 13 2019, 6:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 6:03 AM

LGTM.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
43

The patch should probably mention that it builds upon D65950, which introduces ArchRegNames.

llvm/test/CodeGen/RISCV/compress-inline-asm.ll
2

Is it worth keeping at least one test using the old -riscv-no-aliases, to be sure that option also keeps working?

lenary marked 4 inline comments as done.Aug 15 2019, 2:21 AM
lenary added inline comments.
llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
43

This is noted in the parent revisions (in "Stack" under "Revision Contents" above)

llvm/test/CodeGen/RISCV/compress-inline-asm.ll
2

All the llc and llvm-mc tests still have to use the -riscv-no-aliases flag as they don't have -M.

I don't feel we need to keep compatibility this flag compatibility for llvm-objdump, given I don't think GNU objdump supports the -riscv-no-aliases flag.

luismarques accepted this revision.Aug 15 2019, 6:48 AM
This revision is now accepted and ready to land.Aug 15 2019, 6:48 AM
asb accepted this revision.Aug 19 2019, 5:32 AM

LGTM, but wouldn't it be worth moving arch-reg-names.s to D65950, and also expanding it to handle FP regs?

lenary updated this revision to Diff 219385.Sep 9 2019, 10:10 AM
lenary marked 2 inline comments as done.

Address review feedback in D65950

  • That patch now adds numeric-reg-names.s, and we now just update the LLC invocation at the top of the file.
  • This patch now adds no tests of its own, it just updates all tests to use the -M options.
lenary updated this revision to Diff 219388.Sep 9 2019, 10:18 AM
  • Use -M no-aliases in new tests
lenary updated this revision to Diff 219557.Sep 10 2019, 9:11 AM

Rebase after landing D65950

This revision was automatically updated to reflect the committed changes.