This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Set --print-imm-hex by default.
ClosedPublic

Authored by mysterymath on Oct 28 2022, 12:22 PM.

Details

Summary

This was previously attempted in 2016 by colinl's D18770, but LLD tests
were missed, which caused the change to be reverted.

Setting --print-imm-hex by default brings llvm-objdump's behavior closer
in line with objdump, and it makes it easier to read addresses and
alignment from the disassembly. It may make non-address immediates
harder to interpret, but it still seems the better default, barring more
context-sensitive base selection logic.

--no-print-imm-hex was added to the affected tests in the monorepo in a
semi-automated fashion to preserve their existing semantics.

Diff Detail

Event Timeline

mysterymath created this revision.Oct 28 2022, 12:22 PM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added a reviewer: rafauler. · View Herald Transcript
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
mysterymath requested review of this revision.Oct 28 2022, 12:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2022, 12:23 PM
MaskRay accepted this revision.Oct 28 2022, 1:20 PM

LGTM. Please push the --no-print-imm-hex change separately and land the default-toggle change in its own so that it is more easily reverted in case there is anything not caught.

This revision is now accepted and ready to land.Oct 28 2022, 1:20 PM
MaskRay added inline comments.Oct 28 2022, 1:22 PM
lld/test/COFF/armnt-blx23t.test
4 ↗(On Diff #471624)

Don't wrap lines. For tests we don't stick with the 80-column rule.

If someone decides to fix the tests, they will need to join the lines.

bcain added a subscriber: bcain.Oct 28 2022, 2:16 PM
mysterymath marked an inline comment as done.

Leave the original lines formatting alone; just insert --no-print-imm-hex after
llvm-objdump.

This revision was landed with ongoing or failed builds.Oct 30 2022, 1:36 PM
This revision was automatically updated to reflect the committed changes.