On 2 June 2016 at 23:22, Rafael Espíndola <rafael.espindola(a)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
On 2 June 2016 at 20:49, Rafael Espindola via llvm-commits
<llvm-commits(a)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
* Off on Friday [2/10]
# Progress #
* TCWG-518, V1 got reviewed, and working on V2. Two existing bugs in
GDBserver are found, and fixed. Testing them... [4/10]
* Upstream patches review, all of them are about GDB python. [2/10]
* Respond to Jim Wilson's question on AIX GDB build failure caused by
his binutils patch for AArch64. Confirm that GDB build fail after
his patch. [1/10]
* Misc, [1/10]
# Plan #
* Bank holiday on Monday,
* TCWG-518, get V2 ready for review,
* Ping TCWG-561 and TCWG-547,
--
Yao
=== This Week ===
Resolved TCWG-231 (LLDB bring up and bug fixing on HiKey 96Board)
[TCWG-231] [1/10]
-- Ran tests on AArch64 LLDB tests on HiKey and compared with Nexus 9
test results.
-- Less than 1% difference in test results and good pass rate found.
LLDB ARM: Bug fixes, integration and testing on various ARM platforms
[TCWG-228] [2/10]
-- Progressed towards resolution.
-- Prepared close out setting up RaspberryPi2, RaspberryPi3, Hikey and
Chromebook as testers.
-- Compiled armhf test results.
LLDB Chromebook Test Stability [TCWG-563] [2/10]
-- Some further investigation on llvm-chrome-06 test failures.
-- Marked and committed some xfails and reported relevant bugs if not
already logged.
Support for watchpoint un-alligned watchpoint addresses on LLDB
AArch64 [TCWG-367] [1/10]
-- Debugging of watchpoint handling code and tried out instruction decding.
LLDB buildbot updates and maintenance [TCWG-241] [2/10]
-- Improved tester script to initiate lldb-server inside a chroot.
-- Monitored buildslave stability and
Miscellaneous [2/10]
-- Meetings, emails, discussions etc.
-- Migrated laptop to Ubuntu 16.04
=== Next Week ===
LLDB Chromebook Test Stability [TCWG-563]
-- Final words on watchpoint issue on llvm-chrome-06
-- Submit updated makefile.rulez for LLDB tests.
-- Report remaining bugs and commit xfails upstream.
Support for watchpoint un-alligned watchpoint addresses on LLDB
AArch64 [TCWG-367]
-- Implement instruction decoding for watchpoint hit detection.
A look into kernel code used by Nexus android devices failing
watchpoint support [TCWG-622]
Miscellaneous
-- Out of office on Tuesday 31st 2016 for visa documents preparation.
* ! day off (2/10)
== Progress ==
o Extended validation (2/10)
- Benchmarking comparison script on-going
- Reproduced armhf schroot slowness on AArch64 Debian Jessie
No issues when using Ubuntu (Trusty and Xenial).
o Upstream GCC (4/10)
- Started ARMv8.1 libatomic support
(environment setup, code understanding, ifunc support analysis)
o Misc (2/10)
* Various meetings and discussions.
== Plan ==
o Continue libatomic work
== This Week ==
* TCWG-72 (6/10)
- Patch iterations based on Richard's suggestions
- Reworked most of convert_to_divmod()
- Fixing regressions with the patch and added test-cases.
- ICE with -m32 on x86_64 for DImode division/mod:
https://gcc.gnu.org/ml/gcc-patches/2016-05/msg02302.html
* LTO (2/10)
a) TCWG-548 (1/10)
- Benchmarking jobs submitted twice and failed.
b) TCWG-535 (1/10)
- Figured out why ICE happens with my patch.
* TCWG-319 (1/10)
- Validated patch and posted upstream.
* Misc (1/10)
- Meetings
== Next Week ==
- TCWG-72: Try to workaround issue with -m32
- TCWG-548: start relooking at section anchors.
- TCWG-535: Post patch upstream.
== Progress ==
PR71252 - ICE in rewrite_expr_tree
- Tried various options to fix this and settled on an implementation
- Patches sent for upstream review
- tested with cpu2006 too
- One patch approved
PR66726 - Convert expr
- Newlib test case is failing due to mi compiled lib
- Trying to reduce test case
== Plan ==
Follow upon remaining upstream patches
IPA VRP
== Progress ==
* LLD Port:
- Got hello world running.
-- Needed a horrible hack to make lld output PLT and GOT entries for
unresolved weak references with default visibility.
- Wrote up in Jira the major areas of work that would be needed for a
useful port.
- Started putting in changes cleanly and writing tests for them.
- Currently hitting a few problems with llvm-mc. I can't generate the
relocations I need to test the linker. The hello world test case used
mainly GCC library objects.
-- Most serious is not emitting R_ARM_BASE_PREL for .word
_GLOBAL_OFFSET_TABLE - (label+8)
== Plan ==
* On holiday Monday and Tuesday
* LLVM MC
See how easy it would be to add missing features to llvm-mc as it
would allow me to write more tests.
* LLD
Complete the test cases for the code I've added.
Take stock of where the lld port is and decide where to go from there.
Options are:
- Send an RFC upstream with what I have. It is not sufficient to run
hello world but should be relatively uncontroversial.
- Cleanly implement the emit the unresolved weak references in the PLT
and GOT so hello world will work out of the box on ARM.
- Keep going until I've got Thumb2 support so I don't have to use ARM
only static libraries to make hello world working.
There is obviously a trade off between having a useful linker and the
patch size getting out of hand.
== Progress==
* Remove exit-on-error flag from CodeGen tests [TCWG-604] [5/10]
- This is a follow-up of TCWG-592: when changing the diag handler,
some of the tests started to fail, so we had to add an exit-on-error
flag to preserve the old behaviour until we can fix the tests.
- Last week's patch fixing the MIR tests (PR27770) - still in upstream review
- Last week's patch fixing an AMDGPU test (PR27762) - committed upstream
- Last week's patch fixing a BPF test (PR27766) - committed upstream
- Submitted another patch fixing a BPF test (PR27767) - committed upstream
- Submitted another patch fixing two other BPF tests (PR27768/9) -
in upstream review
- Investigating one of the AMDGPU tests (PR27761)
* Use git worktree in llvm helper scripts [TCWG-587] [4/10]
- Started a RFC and a wiki page about the new workflow [1]
- People seem to be in favour of it, but there are still some TODOs
left before we can merge
* Misc [1/10]
- Meetings, buildbots
== Plan ==
* Remove exit-on-error flag from CodeGen tests [TCWG-604]
- Submit a patch for the AMDGPU test (PR27761)
- More investigations on the ARM test (PR27765)
* Use git worktree in llvm helper scripts [TCWG-587]
- Finish the final touches and have a proper review
[1] https://collaborate.linaro.org/display/LLVM/Git+Worktree+Proposal
== Progress ==
* Validation
- cleanup
- Fixed regressions in abe (gcc_update, make install, gdb timestamps)
- updated abe stable branch (now matches master)
- various Jenkins jobs updates, prepared support for gcc-6 based toolchains
- compared results of armv8l vs arm toolchains: few, expected
differences due to different default tuning
* GCC
- regressions reports on trunk
* Misc (conf-calls, meetings, emails, ....)
== Next ==
* Validation
- actually update the Backport job
- prepare switch to docker for buildfarm job
- cleanup
* Backports
* GCC
- trunk monitoring
- more on AdvSIMD intrinsics
