[PATCH] D143673: [lld][RISCV] Implement GP relaxation for R_RISCV_HI20/R_RISCV_LO12_I/R_RISCV_LO12_S.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lld/ELF/Writer.cpp:1861
       OutputSection *sec = findSection(".sdata");
-      addOptionalRegular("__global_pointer$", sec ? sec : Out::elfHeader, 0x800,
-                         STV_DEFAULT);
+      Defined *globalPointer = addOptionalRegular(
+          "__global_pointer$", sec ? sec : Out::elfHeader, 0x800, STV_DEFAULT);
----------------
MaskRay wrote:
> craig.topper wrote:
> > MaskRay wrote:
> > > Joe wrote:
> > > > If the global pointer has been defined in the linkerscript, addOptionalRegular will return nullptr and ElfSym::riscvGlobalPointer won't be set. Is it worth adding a check for this and setting it if the symbol is defined?
> > > See @Joe 's comment.
> > I don't know my way around lld's APIs. Can you tell me what I need to do?
> Sure. (Sorry I didn't make comments earlier. I had been travelling for most days in the past month and just returned and started to read more patches.)
> 
> For library use cases (prevent `riscvGlobalPointer` from referencing a dangling object), ensure `riscvGlobalPointer` is always initialized even if `config->shared` is true.
> 
> ```
> if (config->emachine == EM_RISCV) {
>   ElfSym::riscvGlobalPointer = nullptr;
>   if (!config->shared) {
>     ...
>     ElfSym::riscvGlobalPointer = symtab.find(...);
>     addOptionalRegular(...);
>   }
> }
> ```
Am I still supposed to check config->relaxGP too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143673