[PATCH] D130358: [llvm-objdump,ARM] Add PrettyPrinters for Arm and AArch64.

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:688
+      size_t Pos = 0, End = Bytes.size();
+      if (STI.checkFeatures("+thumb-mode")) {
+        for (; Pos + 2 <= End; Pos += 2)
----------------
DavidSpickett wrote:
> You could write this as:
> ```
>       size_t insn_bytes = STI.checkFeatures("+thumb-mode") ? 2 : 4;
>       for (; Pos + insn_bytes <= End; Pos += insn_bytes)
>             OS << ' '
>                << format_hex_no_prefix(
>                       llvm::support::endian::read<uint16_t>(
>                           Bytes.data() + Pos, llvm::support::little),
>                       insn_bytes*2);
>       }
> ```
I don't think so, because you need to `read<uint16_t>` or `read<uint32_t>` depending on `insn_bytes`. If there had been an `llvm::support::endian::read` function that took the number of bytes of the integer as a runtime parameter, then yes, I could fold those branches together.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:693
+                      llvm::support::endian::read<uint16_t>(
+                          Bytes.data() + Pos, llvm::support::little),
+                      4);
----------------
DavidSpickett wrote:
> Does the endian here have to match the endian of the object file?
As per the comment in D130357, no, in all versions of the Arm architecture supported by LLVM MC, instructions are stored little-endian regardless of data endianness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130358