[PATCH] D37333: [ELF, draft] - Combine GOTPLT and GOT slots.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D37333#858170, @ruiu wrote:

> When you attempt to implement a documented ABI, please include a reference to the ABI. It seems you are implementing this: https://github.com/hjl-tools/x86-psABI/blob/68edce4f22070cc83ebc4a5df4b74222300dd24d/linker-optimization.tex
>
> So, IIUC, this optimization aims to reduce the number of relocations for an interruptible function from 2 to 1. I'm skeptical if that makes some noticeable difference in performance because .got.plt isn't usually that big, and processing relocations is usually fast. IIRC, in lld (modulo the difference between static and dynamic linkers), we can process millions of relocations in a few seconds, so that's not slow at least.
>
> We don't have to implement all optimizations that the ABI defines. We instead want to implement some of them that have non-marginal performance improvement. You presumably could see a difference in performance in a benchmark for the sake of benchmark, but my feeling is that I don't want to implement it unless someone comes to us with some numbers such as "we want to use this optimization because it makes our application starts up XX% faster" or something like that.
>
> So, what is the motivation of implementing it? If it is because the ABI defines it, I don't think I'm convinced that we want it.


I am agree with you.
We had 2 open issues about this. In one of them Rafael also mentioned that it is not clear how much it is useful, 
and I just wanted to check by myself how much hard to support it. If it would be few lines, we could implement it for feature completeness,
but when I faced problems mentioned in description of patch decided to tell about them first,
because I also suppose it is not very useful optimization and given the amount of changes it requires I would not 
implement it.

So I suggest to close this bug with explicit "Will not fix" statement and look at possible objections from users then.


https://reviews.llvm.org/D37333