[regression] maybe a runtime issue related to D154953

[vfdff]

This is reported with @mstorsjo , so I'm checking:

This commit caused misoptimizations in the WMA decoder in ffmpeg, observed on all architectures. The misoptimization can be observed with https://martin.st/temp/wma-preproc.c, compiled with clang -target aarch64-linux-gnu -c -O3 wma-preproc.c -o libavcodec/wma.o

For a full runtime reproducible case:

$ git clone git://source.ffmpeg.org/ffmpeg
$ mkdir ffmpeg-build
$ cd ffmpeg-build
$ ../configure --cc=clang --samples=../fate-samples
$ make -j$(nproc)
$ make fate-rsync
$ make fate-wmapro-2ch

[vfdff]

• because I can't get the rsync://fate-suite.ffmpeg.org/fate-suit, so I directly comparative analysis the output of above file wma-preproc.c, and get the differences for function ff_wma_run_level_decode between with and without the change of D154953.

~/FFmpeg-base/ffmpeg-build(master) » make -j fate-rsync                                                  zhongyunde@A191240619
rsync -vrltLW --timeout=60 --contimeout=60 rsync://fate-suite.ffmpeg.org/fate-suite/ ../fate-samples
rsync error: timeout waiting for daemon connection (code 35) at socket.c(282) [Receiver=3.1.3]
make: *** [src/tests/Makefile:281: fate-rsync] Error 35

• related code in function ff_wma_run_level_decode
  

[mstorsjo]

• because I can't get the rsync://fate-suite.ffmpeg.org/fate-suit, so I directly comparative analysis the output of above file wma-preproc.c, and get the differences for function ff_wma_run_level_decode between with and without the change of D154953.

~/FFmpeg-base/ffmpeg-build(master) » make -j fate-rsync                                                  zhongyunde@A191240619
rsync -vrltLW --timeout=60 --contimeout=60 rsync://fate-suite.ffmpeg.org/fate-suite/ ../fate-samples
rsync error: timeout waiting for daemon connection (code 35) at socket.c(282) [Receiver=3.1.3]
make: *** [src/tests/Makefile:281: fate-rsync] Error 35

If you're unable to use rsync to get the whole set of samples, you can manually download the specific one needed for this testcase - download http://fate-suite.ffmpeg.org/wmapro/Beethovens_9th-1_small.wma and place it in fate-samples/wmapro. Then you should be able to skip the make fate-rsync step and go directly to make fate-wmapro-2ch.

[vfdff]

I think we missing a condition to check PowerC->isNonNegative()

(gdb) p PowerC->isNegative()
$8 = true
(gdb) p Known.getMaxValue().getActiveBits()
$9 = 1
(gdb) p Op1->dump()
i32 -2147483648
$10 = void
(gdb) p Op0->dump()
  %sub3 = add nsw i32 %conv9.i, -1

[vfdff]

I'll submit a commit similar to following, would you help me check the issue end to end? @mstorsjo
I have checked the single file wma-preproc.c, it seems fixed the issue.

+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -2119,7 +2119,7 @@ static Value *simplifyAndInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
   // and 2^x-1, 2^C --> 0 where x <= C.
   const APInt *PowerC;
   Value *Shift;
-  if (match(Op1, m_Power2(PowerC)) &&
+  if (match(Op1, m_Power2(PowerC)) && PowerC->isNonNegative() &&
       match(Op0, m_Add(m_Value(Shift), m_AllOnes())) &&
       isKnownToBeAPowerOfTwo(Shift, Q.DL, /*OrZero*/ true, 0, Q.AC, Q.CxtI,
                              Q.DT)) {

[vfdff]

candidate MR： https://reviews.llvm.org/D156881

[mstorsjo]

I'll submit a commit similar to following, would you help me check the issue end to end? @mstorsjo I have checked the single file wma-preproc.c, it seems fixed the issue.

Thanks! The fix does seem to make all issues in this testsuite go away, when rebuilt with this patch.

[vfdff]

hi @mstorsjo, the fixing is land now, sorry for blocking your work

[zmodem]

Chromium also hit test failures after D154953, in BoringSSL. It appears that 68ea002 has fixed it, thanks!

(For reference, our bug is https://crbug.com/1470223)