[PATCH] D60927: [WIP][llvm-objdump] Switch between ARM/Thumb based on mapping symbols.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 19 17:11:45 PDT 2019


MaskRay added a comment.

> I'm not really happy about putting the switching logic into llvm-objdump.cpp; it seems like there should be a better way to refactor the interface so the target-specific logic is somewhere inside the target.

I do feel the monolithic `disassembleObject` is hard to reason about. Do you have idea how to simplify it? On the other hand, I think the new addition of `ThumbMappingSymsAddr` `ARMMappingSymsAddr` should be ok, because the ARM specific logic is already there.



================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:576
 
+static bool isArm32Elf(const ObjectFile *Obj) {
+  return (Obj->isELF() &&
----------------
Since you are adding `isArm32Elf`, can you add `isArm64Elf` and delete `isArmElf` if that orthogonal property is desired?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60927





More information about the llvm-commits mailing list