Created attachment 37377 [details] Assembly and preprocessed output of miscompiled file http://fate.ffmpeg.org/report.cgi?time=20160116195307&slot=x86_64-archlinux-gcc-experimental https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/svq1enc.c;h=1e1745e7b10ed24e8a600d643b5aa951317f2aba;hb=HEAD FFmpeg's svq1 encoder is being miscompiled by GCC 6 starting with revision r232361. I'm attaching example assembly of the last good commit and the first bad, alongside the preprocessed output. To reproduce the issue you can download ffmpeg and run the fate svq1 tests (no external samples needed to run), which will fail and create output video files full of artifacts from the revision mentioned above onward: git clone https://git.videolan.org/git/ffmpeg.git && cd ffmpeg && ./configure && make fate-vsynth{1,2,3}-svq1
THanks for the .i and .s files. The one thing that is missing that would help a lot is the compiler flags used. Without that information we have to guess and with the number of flags, it's easy to guess wrong. Thankfully something seems to have recorded the flags and embedded them into your assembly output files. So was able to use those. Please if you file future reports make sure to include the compiler flags too. Anyway, it's a mis-compilation of svq1`_encode_plane. It's quite strange though. So we have: _373 = _370 <= _372; best_374 = (int) _373; That establishes best_374 as having a boolean range, even though it's an integer variable. That's important because later we see a conditional on best_374 and infer its value on both arms of the conditional. So if we look further we have: ;; basic block 444, loop depth 2, count 0, freq 1, maybe hot ;; prev block 443, next block 445, flags: (NEW, REACHABLE) ;; pred: 441 [71.0%] (FALSE_VALUE,EXECUTABLE) ;; 440 [50.0%] (FALSE_VALUE,EXECUTABLE) if (best_374 == 1) goto <bb 445>; else goto <bb 452>; If we follow the TRUE arm where we know best_374 will have the value 1 we have this path (there's others, this is just one): 445->445->446->461->451->462->453 Where BB453 looks like: # best_3287 = PHI <0(443), best_374(462), 0(429)> After my patch that statement looks like: # best_3287 = PHI <0(443), 0(461), 0(429)> Yow! That's clearly not right. Still digging, but that looks like a real problem, and hopefully it's the mis-compilation we're looking for. Certainly if I block the propagation of best_374 via a dbg-counter the resulting code passes its internal tests.
Ha! Got it. On the path noted in the earlier comment we have the following: <bb 444>: pred_x ={v} {CLOBBER}; pred_y ={v} {CLOBBER}; _423 = s_46(D)->rd_total; _424 = score[best_374]; _425 = (long int) _424; _426 = _423 + _425; s_46(D)->rd_total = _426; if (best_374 != 2) goto <bb 445>; else goto <bb 449>; I'd looked at that block a few times and even traced where that test came from, the propagation of best_374 into that test and determined that while suboptimal, it was correct (the test originally used a different variable, one which had the range [0..2]. The path by which the "2" appears in that range eventually gets proved unexecutable and removed. Once that happens the original object becomes equivalent to best_374 and the copy propagation occurs in copyprop (which AFAICT doesn't try to use range information to simplify things). Anyway, all that is irrelevant to the problem. ssa_name_has_boolean_range does something like this edge_info->rhs = (integer_zerop (op1) ? true_val : false_val); When it only had to deal with booleans, that was fine because op1 in this context would always be the constant 0 or 1 (it's the RHS of the conditional). But once we start handling integers with a narrow range, that test is wrong. Fixing is trivial. It'll probably take longer to boil down a suitable testcase -- it's late. I'll poke at this further over the weekend.
Perhaps look at PR69325 or PR69326?
*** Bug 69325 has been marked as a duplicate of this bug. ***
*** Bug 69326 has been marked as a duplicate of this bug. ***
*** Bug 69342 has been marked as a duplicate of this bug. ***
*** Bug 69322 has been marked as a duplicate of this bug. ***
Jakub. I'll certainly backtest against the referenced BZs While the fix is trivial, I'm seriously considering just turning the exploitation of ranges off for gcc-6. It didn't help as much as I had wanted in the path splitting bits, so it's not terribly important for this release.
I've backtested against all 4 duplicates and my change fixes all of them.
Author: law Date: Tue Jan 19 06:43:54 2016 New Revision: 232548 URL: https://gcc.gnu.org/viewcvs?rev=232548&root=gcc&view=rev Log: 2016-01-18 Jeff Law <law@redhat.com> PR tree-optimization/69320 * tree-ssa-dom.c (record_edge_info): For comparisons against a boolean ranged object, do nothing if the RHS constant is not [0..1]. (optimize_stmt): Comparing a boolean ranged object against a constant outside [0..1] results in a compile-time constant. * tree-ssanames.c (ssa_name_has_boolean_range): Remove unnecessary test. PR tree-optimization/69320 * gcc.c-torture/pr69320-1.c: New test. * gcc.c-torture/pr69320-2.c: New test. * gcc.c-torture/pr69320-3.c: New test. * gcc.c-torture/pr69320-4.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr69320-1.c trunk/gcc/testsuite/gcc.c-torture/execute/pr69320-2.c trunk/gcc/testsuite/gcc.c-torture/execute/pr69320-3.c trunk/gcc/testsuite/gcc.c-torture/execute/pr69320-4.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-dom.c trunk/gcc/tree-ssanames.c
Fixed by commit on the trunk.
Fixed on trunk.