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

[Digger via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:321
+  static constexpr uint32_t IsAllocaUsedMask = 0x0000'0020;
+  static constexpr uint32_t IsOnConditionDirectiveMask = 0x0000'001C;
+  static constexpr uint32_t IsCRSavedMask = 0x0000'0002;
----------------
hubert.reinterpretcast wrote:
> This is not a boolean value. Please fix.
thanks


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:403
+  Optional<SmallVector<uint32_t, 8>> ControlledStorageInfoDisp;
+  Optional<uint16_t> FunctionNameLen;
+  Optional<StringRef> FunctionName;
----------------
hubert.reinterpretcast wrote:
> Why a separate field for the length of the function name? The `StringRef` for the function name has a length field.
good idea.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:893
+  if (!Err && ParaNum)
+    ParaType = parseParaType(DE.getU32(&Offset, &Err), ParaNum);
+
----------------
hubert.reinterpretcast wrote:
> I do not believe that it is okay for this to just parse incorrectly when `hasVectorInfo()` is true.
yes, when hasVectorInfo() is true , there is a different version of parse parameter type for the vector info , I will implement them is a following up patch, I added  comment here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585