[PATCH] D54720: [PPC64] toc-indirect to toc-relative relaxation.

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked an inline comment as done.
sfertile added a comment.

Related to @grimar comment, I am also playing around with doing some of the checking early and mapping toc based relocations that are not-relaxable to got based relocations during scanRelocs. Ideally I'd like to be able to add a separate step for PPC64 once all relocations have been scanned that will iterate over the .toc sections to handle large-code model where any symbols that can't be safely relaxed get added to the .got and all remaining toc based relocations get relaxed. This should  hopefully allow me to get rid of the .toc sections in the final binaries, but I'm not familiar enough with large-code model, or gcc and xlc codegen to know if this is feasible yet.



================
Comment at: ELF/InputSection.cpp:969
+      if (!tryRelaxTocPPC64(File, Type, Rel, Expr, BufLoc, AddrLoc))
+        Target->relocateOne(BufLoc, Type, TargetVA);
       break;
----------------
grimar wrote:
> I think we usually reach here (`relocateAlloc`) when we already know will we do the relaxation or not.
> I.e. for example on x86_64 for `R_RELAX_GOT_PC` we just call `Target->relaxGot` because check in the `adjustRelaxExpr` the conditions for relaxing. And here then we either have `R_RELAX_GOT_PC` which name contains `RELAX` and means we *will do* the relaxation or we would have another R_* expression if we are not going to relax.
> 
> So I think you should not have a `R_PPC64_RELAX_TOC` expression here if you are not going to relax. I think you should check the conditions somewhere earlier. Also, the platform-specific code can live into own file (like ELF\Arch\PPC64.cpp) then. I think you can put all the functions you added there.
> 
> 
The got-indirect to toc-relative relaxation is unique in this regard because we can't decide if we can relax until the data segment has been fully built. During scanRelocs we could see if the indirection is to a defined symbol, but that is only enough to know that the relocation is// potentially //relax-able. We still need the offset from the TOC base pointer to the symbols definition to fit in a signed 32-bit offset (which isn't guaranteed in the large code model), and scanRelocs is too early to determine that.  I could add the relax checking earlier than here but that would involve doing the same scan over the .toc sections relocations and replacing those that are indeed relaxable. I decided not to do that in the spirit of keeping it simple.

I like the idea of moving the platform specific code into PPC64.cpp, and will update the patch with that change, but I think here is the right place to perform the relaxation.



Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54720