[PATCH] D81585: [AIX][XCOFF][Patch1] Provide decoding trace back table information API for xcoff object file for llvm-objdump -d

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:328
+  static constexpr uint32_t IsBackChainStoredMask = 0x8000'0000;
+  static constexpr uint32_t FixupMask = 0x4000'0000;
+  static constexpr uint32_t FPSavedMask = 0x3F00'0000;
----------------
This is a boolean and the value expresses whether the function is fixup code, so `IsFixupMask` is appropriate.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:329
+  static constexpr uint32_t FixupMask = 0x4000'0000;
+  static constexpr uint32_t FPSavedMask = 0x3F00'0000;
+  static constexpr uint32_t FPSavedShift = 24;
----------------
Here and below: Please use "FPR" to denote floating-point registers instead of "FP".


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:335
+  static constexpr uint32_t HasVectorInfoMask = 0x0040'0000;
+  static constexpr uint32_t GPSavedMask = 0x003F'0000;
+  static constexpr uint32_t GPSavedShift = 16;
----------------
Here and below: Please use "GPR" to denote general-purpose registers instead of just "GP".


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:339
+  // Byte 7
+  static constexpr uint32_t NumberOfFixedParaMask = 0x0000'FF00;
+  static constexpr uint8_t NumberOfFixedParaShift = 8;
----------------
Here and below: Please use "Parms" or "Params" ("Parms" does appear below). "Para" is not used by any of the three references I have and I am not aware of it being a common short form for "parameter".


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:343
+  // Byte 8
+  static constexpr uint32_t NumberOfFloatPointParaMask = 0x0000'00FE;
+  static constexpr uint32_t ParmsOnStackMask = 0x0000'0001;
----------------
Noting more instances of "FloatPoint" to replace here.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:344
+  static constexpr uint32_t NumberOfFloatPointParaMask = 0x0000'00FE;
+  static constexpr uint32_t ParmsOnStackMask = 0x0000'0001;
+  static constexpr uint8_t NumberOfFloatPointParaShift = 1;
----------------
This is a boolean value: `HasParmsOnStackMask`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585