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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

I did not have a chance to debug it, but my comment is below.



================
Comment at: ELF/InputSection.cpp:969
+      if (!tryRelaxTocPPC64(File, Type, Rel, Expr, BufLoc, AddrLoc))
+        Target->relocateOne(BufLoc, Type, TargetVA);
       break;
----------------
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.




Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54720