This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Default to --mattr=+all for AArch64
ClosedPublic

Authored by MaskRay on Jun 16 2022, 10:45 PM.

Details

Summary

GNU objdump disassembles all unknown instructions by default. Match this user
friendly behavior with the target feature "all" (D128029) designed for disassemblers.

Diff Detail

Event Timeline

MaskRay created this revision.Jun 16 2022, 10:45 PM
MaskRay requested review of this revision.Jun 16 2022, 10:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2022, 10:45 PM
jhenderson accepted this revision.Jun 17 2022, 12:37 AM

Seems good to me, but might want to give a chance for AArch64 contributors to have a say in the matter.

This revision is now accepted and ready to land.Jun 17 2022, 12:37 AM
MaskRay added inline comments.Jun 17 2022, 1:06 AM
llvm/docs/CommandGuide/llvm-objdump.rst
32

typo: will change one to mcpu

nickdesaulniers added inline comments.
llvm/tools/llvm-objdump/llvm-objdump.cpp
1696–1698

Adding context to my comment, this is where I'd expect us to handle per-arch enabling of various target features.

...
if (MAttrs.empty() && MCPU.empty())
  enableAllArchFeatures()
...

static void enableAllArchFeatures() {
  switch (arch) {
  case aarch64:
    Features.AddFeature("+sve", etc etc);
    break;
  case x86:
...
}
Matt added a subscriber: Matt.Jun 28 2022, 2:34 PM
MaskRay marked an inline comment as done.Jun 30 2022, 11:10 AM
MaskRay added inline comments.
llvm/tools/llvm-objdump/llvm-objdump.cpp
1696–1698

Resolved in D128029 ((a) Use TableGen to derive the source of truth. (b) The disassembler side just forwards "+all")

MaskRay updated this revision to Diff 441461.Jun 30 2022, 11:10 AM
MaskRay marked an inline comment as done.

fix a typo. rebase

MaskRay edited the summary of this revision. (Show Details)Jun 30 2022, 11:11 AM
This revision was landed with ongoing or failed builds.Jun 30 2022, 11:18 AM
This revision was automatically updated to reflect the committed changes.
MaskRay added a subscriber: kongyi.Jun 30 2022, 2:28 PM

In case folks (@kongyi) want to cherry pick D128029 and D128030 into an old release branch. f4b7b66c4c75d40c099ef41ecea0d3be27bdb6c1 ([AArch64][test] Add --mattr=-{sve,sve2,sme} to SVE/SVE2/SME MC tests) is also needed
(it likely does not apply cleanly since there has been lots of SVE development).