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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay 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);
----------------
craig.topper wrote:
> 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?
Yes. `ElfSym::riscvGlobalPointer` can be nullptr if `config->relaxGP` is false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143673