[PATCH] D71649: [PPC32] Emit R_PPC_PLTREL24 for calls to dso_local ifunc

Fri Dec 20 11:31:10 PST 2019

MaskRay added a comment.

In D71649#1793088 <https://reviews.llvm.org/D71649#1793088>, @sfertile wrote:

> In D71649#1792275 <https://reviews.llvm.org/D71649#1792275>, @MaskRay wrote:
>
> > 🤡🤡🤡(Any chance this is reviewed before I watch star wars? (I felt that if I did not say, due to vacation schedules, this might took a lot longer..))
>
>
> Yep, sorry for taking so long. It seems the fact that upgrading a function to an ifunc isn't always safe for the 32-bit PowerPC ABI is soemthing we just have to live with. I am good with this patch as is, with 2 minor comments:
>
> 1. Is it possible to diagnose when a call to an ifunc uses  R_PPC_LOCAL24PC or R_PPC_REL24  but should have used a R_PPC_PLTREL instead? It would be really nice to issue a Error with a descriptive diagnostic at link time for this.

This looks like a linker suggestion, right? Technically lld can do that... But GNU ld silently generates incorrect code, and clang does not generate `R_PPC_LOCAL24PC` (except `_GLOBAL_OFFSET_TABLE` which is hidden and unrelated to ifunc). I think the usefulness of the detection may not be that much. I guess we can probably live with this deficiency...

> 2. We should consider not marking ifunc as dso_local when generating the IR. At least for ppc32, but I think conceptually it might make sense in general.

This will be an alternative. I have a weak preference for making this general. Since this seems to work on all other targets (and ppc64), the scope of this hack should probably be kept as narrow as possible. It seems this place is the most appropriate place before we found more issues.

Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71649