[PATCH] D143725: [llvm-objdump][ARM] support --symbolize-operands for ARM/ELF

Fri Feb 17 13:50:23 PST 2023

covanam added a comment.

In D143725#4127923 <https://reviews.llvm.org/D143725#4127923>, @Esme wrote:

> Please include as much context as possible with your diff. https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Thank you. Will read that and will add more context.

================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1145
+  // So far only supports ARM/Thumb, PowerPC and X86.
+  Triple triple = STI->getTargetTriple();
+  if (!triple.isPPC() && !triple.isX86() && !triple.isARM() &&
----------------
Esme wrote:
> Please capitalize the first letter of variable names.
Sorry I am newcomer. clang-format didn't say anything, so I thought this is okay.
Will be fixed. Same for the other one.

================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1169-1171
+      // On PowerPC and ARM/Thumb, if the address of a branch is the same as
+      // the target, it means that it's a function call. Do not mark the label
+      // for this case.
----------------
hoy wrote:
> Is this always true for linked binary?
Actually I don't know, from my practical experience, it seems to be true. However, I checked ARM's documentation on ELF files and they do not say anything about this.

I will think about this and will come back later.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143725/new/

https://reviews.llvm.org/D143725