[PATCH] D75131: [XCOFF][AIX] Enable -r option for llvm-objdump

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:280
 
   void moveRelocationNext(DataRefImpl &Rel) const override;
+  /// Returns relocation offset.
----------------
Please add a blank line before the comment block.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:281
   void moveRelocationNext(DataRefImpl &Rel) const override;
+  /// Returns relocation offset.
+  /// When received relocation offset is invalid, returns UINT64_MAX.
----------------
Suggestion:
Returns the relocation offset with the base address of the containing section as zero.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:282
+  /// Returns relocation offset.
+  /// When received relocation offset is invalid, returns UINT64_MAX.
   uint64_t getRelocationOffset(DataRefImpl Rel) const override;
----------------
Suggestion:
Returns <named constant> on errors (such as a relocation that does not refer to an address in any section).


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:22
+enum : uint16_t { NO_REL_MASK = 0x0001 };
+static constexpr uint64_t InvalidReloc = std::numeric_limits<uint64_t>::max();
+} // namespace
----------------
My understanding of @jhenderson's comment is that this should be an interface constant in a header.


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

https://reviews.llvm.org/D75131