Issue 499 in webp: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11

[jz… via monorail]

Status: Accepted
Owner: jz...@google.com
Labels: Type-Defect Priority-Medium

New issue 499 by jz...@google.com: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499

v1.1.0-69-g289757fe

There are two visible issues, but they may have the same root cause:
1) a mismatch in x86 between the default & -noasm
1a) gcc: with -O0
1b) clang: with -O2

2) a mismatch between x86 and x86-64
2a) gcc: with -O2
2b) clang: with -O2

Versions tested:
# Only (2) applies
gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516

# Both (1) & (2) apply
gcc (Debian 8.3.0-6) 8.3.0
i686-linux-gnu-gcc-8 (Debian 8.4.0-1) 8.4.0
i686-linux-gnu-gcc-9 (Debian 9.3.0-18) 9.3.0
i686-linux-gnu-gcc-10 (Debian 10.2.0-9) 10.2.0
Debian clang version 11.0.0-5+build1

# or CC='gcc -m32', CC='clang -m32'
$ ../libwebp/configure --host i686-linux-gnu --build x86_64-pc-linux-gnu \
--disable-shared CC='i686-linux-gnu-gcc-8' CFLAGS='-g -O0'

$ ./examples/cwebp -noasm -lossless -q 0 -m 4 ../libwebp-test-data/test.webp \
-o - 2>/dev/null | md5sum \
&& ./examples/cwebp -lossless -q 0 -m 4 ../libwebp-test-data/test.webp \
-o - 2>/dev/null | md5sum
7465351e9a838a245c6e79cfb602f3f4 -
382b2625417c6376480e2b3f16365154 -

# 32-bit -O2, all gcc
355342de0f46c8a4440f8e7b8d4e48bf -
355342de0f46c8a4440f8e7b8d4e48bf -

# 32-bit -O2, clang
355342de0f46c8a4440f8e7b8d4e48bf -
2e0d6693c495280d0a8b639b5531b144 -

# -O0 & -O2 x86-64
7465351e9a838a245c6e79cfb602f3f4 -
7465351e9a838a245c6e79cfb602f3f4 -

I tested some decodes, the bitstream was always lossless.

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[jz… via monorail]

Comment #1 on issue 499 by jz...@google.com: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c1

Disabling CombinedShannonEntropy_SSE2 (which returns a float) clears (1a) and (1b), but (2) remains. So there are other float issues around.

[jz… via monorail]

Updates:
Cc: vince...@gmail.com pasca...@gmail.com

Comment #2 on issue 499 by jz...@google.com: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c2

(No comment was entered for this change.)

[jz… via monorail]

Comment #3 on issue 499 by jz...@google.com: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c3

I didn't really look into it, but for what it's worth the first occurrence of x86 != x86-64 seems to be v0.4.0-57-gc16cd99a [1]. Bisecting from 0.4.0..0.4.4 and 0.4.0..master land at the same place. It's worth noting too that in v1.1.0-69-g289757fe we'll generate a floating point exception (FE_INEXACT) in the FastLog function. Changing the double precision to 64 didn't change the result, so it may have to do with the float precision itself as we were suspecting.

[1]
c16cd99abab37042aa1bc89e10f50aa4f86f348c is the first bad commit
commit c16cd99abab37042aa1bc89e10f50aa4f86f348c
Author: Vikas Arora <vik...@google.com>
Date: Fri Feb 21 11:41:38 2014 -0800

Speed up lossless encoder.

Speedup lossless encoder by 20-25% by optimizing:
- GetBestColorTransformForTile: Use techniques like binary search and
local minima search to reduce the search space.
- VP8LFastSLog2Slow & VP8LFastLog2Slow: Adding the correction factor for
log(1 + x) and increase the threshold for calling the approximate
version of log_2 (compared to costly call to log()).

Change-Id: Ia2444c914521ac298492aafa458e617028fc2f9d

src/dsp/lossless.c | 295 ++++++++++++++++++++++++++++++++++++-----------------
src/dsp/lossless.h | 2 +-
src/enc/vp8l.c | 3 +-
3 files changed, 203 insertions(+), 97 deletions(-)
bisect run success

[jz… via monorail]

Comment #4 on issue 499 by jz...@google.com: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c4

From Intel's SDE [1][2] we can see a potential culprit (x87) which was somewhat expected:

$ for f in sse-check*; do echo "==> $f <=="; awk '{print $3}' $f |sort |uniq; done
==> sse-check-x86_64-asm.txt <==

AVX2
BASE
BLOCK
LONGMODE
SSE
SSE2
==> sse-check-x86_64-noasm.txt <==

AVX2
BASE
BLOCK
LONGMODE
SSE
SSE2
==> sse-check-x86-asm.txt <==

BASE
BLOCK
SSE
SSE2
SSE4
X87
==> sse-check-x86-noasm.txt <==

BASE
BLOCK
SSE
SSE2
SSE4

[1] https://software.intel.com/content/www/us/en/develop/articles/intel-software-development-emulator.html
[2] sde -demangle -sse-sde -sse-check -- ./examples/cwebp -lossless -q 0 -m 4 libwebp-test-data/test.webp

[bugdroid via monorail]

Updates:
Labels: merge-merged-1.2.0

Comment #5 on issue 499 by bugdroid: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c5

The following revision refers to this bug:
https://chromium.googlesource.com/webm/libwebp/+/8599571935b08104b666695987ba663db76f461e

commit 8599571935b08104b666695987ba663db76f461e
Author: James Zern <jz...@google.com>
Date: Tue Jan 19 00:41:44 2021

disable CombinedShannonEntropy_SSE2 on x86

this function produces different results from the C code due to
use of double/float resulting in output differences when compared to
-noasm.

Bug: webp:499
Change-Id: Ia039b168c0a66da723fb434656657ba1948db8ae

[modify] https://crrev.com/8599571935b08104b666695987ba663db76f461e/src/dsp/lossless_enc_sse2.c

[jz… via monorail]

Updates:
Status: Fixed

Comment #6 on issue 499 by jz...@google.com: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c6

(1) from comment #1 is fixed with the change in comment #5. Converting to floats may help as well, but that is beyond the scope of the bug.

[Git Watcher via monorail]

Comment #7 on issue 499 by Git Watcher: [x86] lossless asm/C mismatch w/gcc 8-10, clang 11
https://bugs.chromium.org/p/webp/issues/detail?id=499#c7

The following revision refers to this bug:

https://chromium.googlesource.com/webm/libwebp/+/dde11574b0ddf7fa94e91f120e88ac937b226910

commit dde11574b0ddf7fa94e91f120e88ac937b226910
Author: Vincent Rabaud <vra...@google.com>
Date: Thu Jul 11 13:15:33 2024

Remove TODO now that log is using fixed point.

Bug: webp:499

Change-Id: I39ab340ec6b5932db7535c6b7f31843c28de8415

[modify] https://crrev.com/dde11574b0ddf7fa94e91f120e88ac937b226910/src/dsp/lossless_enc_sse2.c