On Mon, 3 Jul 2023, YunQiang Su via Gcc-cvs wrote:
> https://gcc.gnu.org/g:42d6b905c454e8f1b59d9248465d62a489b64972
>
> commit r14-2248-g42d6b905c454e8f1b59d9248465d62a489b64972
> Author: Jie Mei <jie....@oss.cipunited.com>
> Date: Mon Jun 19 16:29:53 2023 +0800
>
> MIPS: Add bitwise instructions for mips16e2
>
> There are shortened bitwise instructions in the mips16e2 ASE,
> for instance, ANDI, ORI/XORI, EXT, INS etc. .
This change was committed without any public review and it breaks the
build of the compiler for the `mipsel-linux-gnu' target:
.../gcc/config/mips/mips-msa.md:435:26: error: 'and' of mutually exclusive
equal-tests is always 0 [-Werror]
435 | DONE;
.../gcc/config/mips/mips-msa.md:435:26: error: 'and' of mutually exclusive
equal-tests is always 0 [-Werror]
.../gcc/config/mips/mips.md:822:1: error: 'and' of mutually exclusive
equal-tests is always 0 [-Werror]
822 | ;; conditional-move-type condition is needed.
| ^
.../gcc/config/mips/mips.md:822:1: error: 'and' of mutually exclusive
equal-tests is always 0 [-Werror]
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:2928: build/gencondmd.o] Error 1
Moreover the error messages indicate dead code has been introduced. This
shows the lack of due dilligence in any review this patch series may have
received off-list. At the very least the maintainer is supposed to verify
that code builds before it has been committed.
I volunteered to review this patch series for a reason, not only to
ensure meeting the GNU Coding Standards, but because I spotted code design
problems. I realise I did not make the review right away as I could not
afford the time required for such a medium large sized patch series last
month. This does not mean the changes should have been committed without
verification or any review at all.
I raised my concerns back when YunQiang volunteered to become a MIPS
target maintainer. He was accepted by the GCC Steering Committee
regardless, with a word of advice to be vigilant about GCC Coding
Standards. I would expect a responsible person to follow such advice.
Back when I became a MIPS target maintainer for binutils and GDB I worked
for Imagination Technologies and I told my manager not to expect me to be
more relaxed when reviewing changes submitted by my coworkers. He replied
he actually expected me to be stricter about such changes, which was
exactly the reply I expected. Here it seems to me the patch series was
fast-tracked despite having been submitted from a person working for the
same company. This leaves me with a bad feeling. Also from what I have
seen written I have a feeling that the new maintainer has assumed the
position of authority rather than duty. This doesn't feel right to me.
Given the concerns I previously raised were in my reception not correctly
addressed I have a feeling my voice has not been listened to. This makes
me less than motivated to strive and find time for MIPS reviews, which I
find a significant mental burden, especially when code is far below the
standard expected. Therefore given the turn of events I am not going to
review any further MIPS changes submitted and will limit myself to the VAX
backend.
Considering my continued interest in keeping my MIPS hardware alive I
will probably regret that as things continue breaking and bad design
decisions are propagated, but without the lack of support from the
community I need to draw a line somewhere.
NB it cost me disruption and half a working day lost in tracking down
what has happened here and consequently I haven't completed what I meant
to and which I expected to be a trivial task of rebuilding the compiler
and trying it on a piece of code I wanted to check.
Maciej
