[PATCH] D131467: [LoongArch] Define the new-style reloc types

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n marked 2 inline comments as done.
xen0n added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/ELFRelocs/LoongArch.def:64-65
+
+// And finally, the new-style relocs whose processing do not require a stack
+// machine.
+//
----------------
jhenderson wrote:
> The "And finally" risks more relocs being added in the future, thus meaning this set is no longer the final set, and the comment going stale. Similarly, in 10+ years time, "new-style" may not sound particularly right. I'd probably just go simply with "Relocs whose processing do not require a stack machine."
Fair point. I was thinking by the time a third batch of reloc types are added the maintainer could adjust the comment while at it, but of course the previous wording is a bit biased (the LoongArch open-source community almost unanimously hated the previous reloc design with a passion, including me of course).

Adjusted.


================
Comment at: llvm/include/llvm/BinaryFormat/ELFRelocs/LoongArch.def:104
+ELF_RELOC(R_LARCH_TLS_GD_HI20,      98)
+ELF_RELOC(R_LARCH_RELAX,            99)
----------------
xen0n wrote:
> SixWeining wrote:
> > https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html#_relocations
> > Is `R_LARCH_32_PCREL (99)`  removed?
> I have to self-censor a few swear words. They added this `R_LARCH_32_PCREL` in [[ https://github.com/bminor/binutils-gdb/commit/9801120721c3a702ce3bd50433ef920f92a83502 | a later commit but with irrelevant commit message subject ]], so I ignored it.
> 
> I'll fix shortly.
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131467