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

Rafael EspĂ­ndola via llvm-dev llvm-dev at lists.llvm.org
Thu Jun 2 15:22:05 PDT 2016


> 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


More information about the llvm-dev mailing list