[lld] r250101 - [ELF2] Add a base set of PPC64 relocations

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Rafael Espíndola" <rafael.espindola at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>
> Sent: Tuesday, October 13, 2015 9:04:54 AM
> Subject: Re: [lld] r250101 - [ELF2] Add a base set of PPC64 relocations
> 
> > +  // FIXME: This obviously does not do the right thing when there
> > is no .got
> > +  // section, but there is a .toc or .tocbss section.
> > +  uint64_t TocVA = Out<ELF64BE>::Got->getVA();
> > +  if (!TocVA)
> > +    TocVA = Out<ELF64BE>::Plt->getVA();
> 
> There is always a got if there is a plt, so this could be an assert,
> no?

I imagine this is true currently because we create both sections together. Does anything prevent a .got from existing in an input file? When we support the lazy-binding .bss-style .plt sections, we might not have a .got (that's my current understanding).

> 
> > +  switch (Type) {
> > +  default: return false;
> 
> git-clang-format.
> 
> 
> 
> > +
> >  bool PPC64TargetInfo::relocNeedsPlt(uint32_t Type, const
> >  SymbolBody &S) const {
> > -  return false;
> > +  if (Type != R_PPC64_REL24)
> > +    return false;
> > +
> > +  // These are function calls that need to be redirected through a
> > PLT stub.
> > +  return S.isShared() || (S.isUndefined() && S.isWeak());
> >  }
> 
> Why the extra "(S.isUndefined() && S.isWeak())"?
> 
> Is this here for the same reason as X86_64 and i386? If so, you only
> need the S.isShared(), no?

No, I added this specifically because it came up, even in my hello-world test case (because there was some symbol, __gmon_start__ as I recall, that has weak linkage and needs a .plt stub). Maybe I'm just not understanding what is going on, but what would locally resolving such an undefined symbol mean?

> 
> 
> > +  // For a TOC-relative relocation, adjust the addend and proceed
> > in terms of
> > +  // the corresponding ADDR16 relocation type.
> >    switch (Type) {
> > -  case R_PPC64_ADDR64:
> > -    write64be(Loc, SymVA + Rel.r_addend);
> > +  case R_PPC64_TOC16:       Type = R_PPC64_ADDR16;       A -= TB;
> > break;
> > +  case R_PPC64_TOC16_DS:    Type = R_PPC64_ADDR16_DS;    A -= TB;
> > break;
> > +  case R_PPC64_TOC16_LO:    Type = R_PPC64_ADDR16_LO;    A -= TB;
> > break;
> > +  case R_PPC64_TOC16_LO_DS: Type = R_PPC64_ADDR16_LO_DS; A -= TB;
> > break;
> > +  case R_PPC64_TOC16_HI:    Type = R_PPC64_ADDR16_HI;    A -= TB;
> > break;
> > +  case R_PPC64_TOC16_HA:    Type = R_PPC64_ADDR16_HA;    A -= TB;
> > break;
> > +  default: break;
> 
> git-clang-format.

No, thank you.

Thanks again,
Hal

> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory