llvm dev - Jun 2016 - [lld] r271569 - Start adding tlsdesc support for aarch64.

On 2 June 2016 at 20:49, Rafael Espindola via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: rafael
> Date: Thu Jun  2 14:49:53 2016
> New Revision: 271569
>
> URL: http://llvm.org/viewvc/llvm-project?rev=271569&view=rev
> Log:
> Start adding tlsdesc support for aarch64.
>
> This is mostly extracted from http://reviews.llvm.org/D18960.
Rafael,

Why commit part of Adhemerval's patch without reviewing his request?
This is a really serious breach of community trust.

Not only we're waiting for reviews on the TLS set of patches and
having to rebase every two weeks, but now you implemented in a way
that wasn't discussed on the review, didn't mention authorship, nor
asked Adhemerval for any input.

If you had technical input on his patch, you should have done on the
review. If you wanted him to split in smaller patches, you should have
asked on the review and let *him* do it.

Even if you were the code owner (which you're not), it would still be
a *serious* breach of trust and respect.

I hereby respectfully request that you revert your patch and let
Adhemerval finish the work that he started in the way that we normally
do in the LLVM community.

regards,
--renato

> Rafael,
>
> Why commit part of Adhemerval's patch without reviewing his request?
> This is a really serious breach of community trust.
Because the patch includes way too much and doesn't explain what it is
doing.
That is a general problem with aarch64, the documentation is missing
and comments have to make due. I had a lot of work to rewrite the
original aarch64 patches to be in line with the rest of lld and I
didn't want to have to do the same for tls.
> Not only we're waiting for reviews on the TLS set of patches and
> having to rebase every two weeks, but now you implemented in a way
> that wasn't discussed on the review, didn't mention authorship, nor
> asked Adhemerval for any input.
The delay was because of the above mentioned issues. I wanted to make
sure there was a solid foundation.

Realistically the only way to get an understanding of how tls works on
aarch64 is to look at what musl does. For example, the patch sets DT_*
entries that are not documented on any aarch64 document. I had to go
read unofficial arm info and guess aarch64 was similar. That is how I
found this was only for dlopen and should not be in the first patch.
Once I had a patch, I may as well commit it.

> If you had technical input on his patch, you should have done on the
> review. If you wanted him to split in smaller patches, you should have
> asked on the review and let *him* do it.
>
> Even if you were the code owner (which you're not), it would still be
> a *serious* breach of trust and respect.
>
> I hereby respectfully request that you revert your patch and let
> Adhemerval finish the work that he started in the way that we normally
> do in the LLVM community.
Sorry, no.

Compare the two changes. They have almost nothing in common. I
mentioned it to give credit, but I wrote almost the entire patch. I
didn't even apply it, I just added the R_* expressions and noticed the
patch was not even creating them. From that point I just wrote the
rest.

Cheers,
Rafael

On 2 June 2016 at 23:22, Rafael Espíndola <rafael.espindola at gmail.com>
wrote:
> Because the patch includes way too much and doesn't explain what it is
doing.
So let me get this straight: someone publishes a patch, you don't like
it, you do some private investigations and commit whatever you want
without even notifying the original authors?

I don't know how you work at your company, but this is not how open
source development works.

This is not the first time either that you step over people's toes
with your "design decisions" that you don't share with anyone.
Last
year, Adhemerval has worked for three months to get the LLD AArch64
back-end working and out of the blue, no warning, the whole back-end
was yanked.

It doesn't matter if it was the right decision or not in the long
term, we don't just yank things, especially not before some
deliberation on the list. See how long is taking for the new pass
manager to be enabled, or FastIsel or the new Selection, or the new
register allocators, etc.

That's not how open source works and I assumed you knew that.

> That is a general problem with aarch64, the documentation is missing
> and comments have to make due. I had a lot of work to rewrite the
> original aarch64 patches to be in line with the rest of lld and I
> didn't want to have to do the same for tls.
You shouldn't be rewriting *any* patch, but asking the original
authors to do that themselves.

There is a pattern that I'm seeing and that's that *you* refuse or
dismiss more patches than most other people. There are many of your
comments on reviews that are just personal, and then you step over
people's toes and commits yourself.

This does not scale. But more importantly, it puts into doubt the
validity of the tool you're so hardly defending.

You see, 3 years ago, I was asked to choose between MCLinker and LLD.
MCLinker was a linker for all purposes, but Chris Lattner convinced me
that LLD is the LLVM linker, and we should be focusing all efforts
there.

It goes against the commercial interests of Linaro members to choose
such a premature technology, and it did put them back years of
development, because MCLinker was very close to ready, and MediaTek,
despite what people said, was very willing to accept our help.

But in the interest of the community, and the open source nature of my
work, I have decided to pursue LLD and managed to convince Linaro to
put two people working on it. But now, I'm re-evaluating all my
strategy, and sincerely, I do not trust the LLD community anymore.

> The delay was because of the above mentioned issues. I wanted to make
> sure there was a solid foundation.
Some patches are quick to review, others take 6 months. If you work in
open source you have *got* to understand that. If you're not willing
to take that cost, than please, refrain from working open source.

> Sorry, no.
I understand your position, but you have to understand mine. I
therefore call into question your ability to care about such an
important project of the LLVM community.

I sincerely believe that your actions are harming the project, and the
people trying to help. I appreciate the value of your contribution, I
really do, but if you don't change your way to handle open source
contributions, LLD will, whether you like it or not, become irrelevant
and be replaced.

Such is the nature of open source.

cheers,
--renato