On 5/3/21 10:41 AM, Linus Torvalds wrote:

On Fri, Apr 30, 2021 at 1:46 PM Vineet Gupta [off-list ref] wrote:

quoted

I've hit a mainline gcc 10.2 (also gcc 9.3) bug which triggers at -O3
causing wrong codegen.

So it does seem to be a gcc bug or at least mis-feature where gcc gets
the aliasing decision wrong when vectorizing the code.

That said, the fact that gcc even tries to vectorize the code shows
how pointless it was for us to (years ago) try to make it use 16-bit
accesses in the first place.

So can you try this patch that basically reverts some of those
kernel-specific changes to zlib's inffast.c - and does so by just
upgrading it to a newer version from a more modern zlib (which in this
case still means "from 2017", but that's the most recent version there
is).

This is a fairly quick hack, and I really tried to keep it to just
inffast.c and inftrees.c with a few minimal fixups to inflate.c
itself.

Most of the changes are for naming (which seems to be at least partly
for C++ namespace reasons, ie "this" -> "here"), but there's some
cleanup wrt maximum table sizes etc too.

NOTE! I have not tested this patch in any other way than "it compiles
with allmodconfig". The diff looks reasonable to me, but that's all
I'm really going to say.

Does this avoid the gcc -O3 problem for you?

Yes it does. I built the following:

2021-05-03 b93bedcf8fa4 Update zlib inffast code to more modern zlib
2021-02-26 ea680985468f ARC: fix CONFIG_HARDENED_USERCOPY
2021-04-23 f7118f8ada1b ARC: entry: fix off-by-one error in syscall 
number validation
2021-04-21 1cb7eefda7ed ARC: kgdb: add 'fallthrough' to prevent a warning
2021-03-22 163630b2d95b arc: Fix typos/spellos
2021-04-11 d434405aaab7 Linux 5.12-rc7

And it seems to be DTRT

| Linux version 5.12.0-rc7-00005-gb93bedcf8fa4 
(vineetg@vineetg-Latitude-7400) (arc-linux-gcc.br_real (Buildroot 
2021.02-6-g5e29ba7bf732) 10.2.0, GNU ld (GNU Binutils) 2.36.1) #678 
PREEMPT Mon May 3 11:29:32 PDT 2021
| Memory @ 80000000 [1024M]
| ...
|   with environment:
|     HOME=/
|     TERM=linux
| Starting syslogd: OK
| Starting klogd: OK
| Running sysctl: OK
| $
| $ zcat /proc/config.gz | egrep "(OPTIM|COMPRESSION_GZIP)"
| CONFIG_INITRAMFS_COMPRESSION_GZIP=y
| # CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
| CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3=y
|
| $ free -m
|               total        used        free      shared buff/cache   
available
| Mem:           1012           3         978          31 31         972
| Swap:
|

It would be lovely if somebody also took a look at some of the other
zlib code, like inflate.c itself. But some of that code has rather
subtle changes for things like s390 hardware support, and we have
thihngs like our fallthrough rules etc, so it gets a bit hairier.

I took a quick look, but there some to be subtle state machine changes 
in inflate.c which I'm not comfortable mucking around with.

Which is why I did just this fairly minimal part.

Note that the commit message has a "Not-yet-signed-off-by:" from me.
That's purely because I wanted it to be obvious that this needs a lot
more testing - I'm not comfy with this patch unless somebody takes it
upon themselves to actually test it under different loads (and
different architectures).

Maybe give it some time to bake in linux-next for a 5.14 inclusion ?
Thx again for jumping in and steering gcc folks to right conclusions.

-Vineet