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

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked an inline comment as done.
sfertile added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:158
+  R_TOC = 0x03,
+  R_TRL = 0x12,
+  R_TRLA = 0x13,
----------------
DiggerLin wrote:
> sfertile wrote:
> > This is 0x4 in the [[ https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/filesreference/XCOFF.html#XCOFF__sua3i125jbau | XCOFF Object Format Docs]], do they need to be updated or is the patch wrong?
> I think it is Doc wrong, The value in the patch come from aix OS header file . I  opened ticket to confirm it.
Thanks.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:165
+  R_RL = 0x0c,
+  R_RLA = 0x0d,
+  R_REF = 0x0f,
----------------
DiggerLin wrote:
> sfertile wrote:
> > Minor nit: IIUC `R_RL` and 'R_RLA` are handled the same as `R_POS`, maybe we should keep them together.
> the enum is keep in value order, I am prefer to keep them as now. 
There are already relocations grouped together by functionality as opposed to value order: eg `R_TOC', 'R_TRL','R_TRLA` or 'R_BA`,'R_RBA','R_RBR' . Why is important to keep these grouped in value order as opposed these others that are grouped by functionality? 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67008