[PATCH] D67008: [llvm-readobj][XCOFF]implement parsing relocation information for 32-bit xcoff objectfile

[Digger via Phabricator via llvm-commits]

DiggerLin marked 6 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:193
+               ///< the refrenced symbol and the address of the refrenced branch
+               ///< instruction. References a non modifiable instruction.
+  R_RBA = 0x18, ///< Branch absolute relocation. Similar to the R_BA but
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Still missing the hyphen for "non-modifiable".
> Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616694
changed


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:194
+               ///< instruction. References a non modifiable instruction.
+  R_RBA = 0x18, ///< Branch absolute relocation. Similar to the R_BA but
+                ///< references a modifiable instruction.
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Either remove the "the" for this line or add "relocation" after "R_BA".
> Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616692
changed


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:165
+};
 class XCOFFObjectFile : public ObjectFile {
 private:
----------------
hubert.reinterpretcast wrote:
> Blank line between the class definitions please.
added


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:597
+
+  uint32_t RelocEntNum = RelocEntNumOrErr.get();
+
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Suggestion: `NumRelocEntries`
> Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616704
changed 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67008