Hopefully this reduction is right. Originally reported downstream in Gentoo at https://bugs.gentoo.org/941208 where IM would give a black PNG when built with -O3 with GCC 13. ``` #include <stdbool.h> #include <stdlib.h> static int count = 0; double PerceptibleReciprocal(const double x) { double sign; sign = x < 0.0 ? -1.0 : 1.0; return (sign / 1.0e-12); } bool CompositeImage(int compose) { double Sa = count; double pixel = 65535; double gamma = 1.0; for (int i = 0; i < 3; i++) { switch (compose) { case 1: case 61: { pixel = 65535 * gamma * Sa; break; } case 39: { gamma = PerceptibleReciprocal(0); break; } case 17: { /* When things are OK, we reach here. */ exit(0); break; } default: { break; } } /* This must be a switch to break. */ switch (compose) { case 20: case 30: case 50: { gamma = PerceptibleReciprocal(0); break; } default: { break; } } if (pixel < 0 || pixel >= 65535) { __builtin_abort(); } } count++; return true; } int main() { CompositeImage(1); CompositeImage(17); } ```
The testcase works for me with 12+14+15, but the issue might be latent. 13.3 was the first bad release, bisected to r13-7841-g70639014a69cf5 (for PR110315, cherrypick of PR109695). 14 was never broken so I couldn't bisect there.
(FWIW, the bisect result makes no sense to me, but I did revert it on releases/13 and it worked?)
/app/example.cpp:19:21: optimized: unswitching loop 3 on 'switch' with condition: compose_13(D) == 17 That seems to be missing for GCC 13.
auto_vec<int_range_max> edge_range; ... edge_range.safe_grow_cleared (EDGE_COUNT (gimple_bb (stmt)->succs), true); Hmmm, is this even valid to do? Seems like that might be broken ... Someone who knows the range API better can tell if not running the constructor on int_range_max and just having it be all 0s will be fine. That might be the reason why bisecting to r13-7841-g70639014a69cf5 makes sense. It might also be a latent bug on the trunk too now. Anyways confirmed.
Created attachment 59325 [details] Patch which might fix the issue Can you test this patch? It might fix the issue by using an allocated array of int_range_max instead of a auto_vec<int_range_max> . vec does not call the constructor or deconstructor so we might even be leaking memory.
Simplified a bit: ``` int a; int c (int f) { int d = 1; for (int e = 0; e < 3; e++) { switch (f) { case 1: d = 0; case 2: a = 1; break; case 3: __builtin_exit(0); } switch (f) { case 4: case 6: case 8: a = 1; } if (d) __builtin_abort(); } return 1; } int main() { c(3); } ``` I can't get it to fail on 14+ though.
(In reply to Sam James from comment #6) > Simplified a bit: Just some debug: When we create one of the unswitch_predicate for the second switch, we have: true_range: [irange] int [4, 4][6, 6][8, 8] NONZERO 0xf false_range: [irange] int [4, +INF] NONZERO 0x7fffffff That false_range looks wrong because there is no . It was created by doing: false_range = true_range; if (!false_range.varying_p () && !false_range.undefined_p ()) false_range.invert (); The code looks correct. Let me try something.
(In reply to Andrew Pinski from comment #7) > (In reply to Sam James from comment #6) > > Simplified a bit: > > Just some debug: > > When we create one of the unswitch_predicate for the second switch, we have: > > true_range: > [irange] int [4, 4][6, 6][8, 8] NONZERO 0xf > false_range: > [irange] int [4, +INF] NONZERO 0x7fffffff > > That false_range looks wrong because there is no . > > It was created by doing: > false_range = true_range; > if (!false_range.varying_p () > && !false_range.undefined_p ()) > false_range.invert (); > > The code looks correct. I did a quick hack: static int t = 0; t++; if (t == 4) false_range.set_undefined(); Since this was the 4th creation of unswitch_predicate and that worked.
(In reply to Andrew Pinski from comment #7) > (In reply to Sam James from comment #6) > > Simplified a bit: > > Just some debug: > > When we create one of the unswitch_predicate for the second switch, we have: > > true_range: > [irange] int [4, 4][6, 6][8, 8] NONZERO 0xf > false_range: > [irange] int [4, +INF] NONZERO 0x7fffffff > > That false_range looks wrong because there is no . > > It was created by doing: > false_range = true_range; > if (!false_range.varying_p () > && !false_range.undefined_p ()) > false_range.invert (); > > The code looks correct. This is exactly the same debug as what was done in PR 109934 :).
Created attachment 59326 [details] The change backported I don't have time to do a full bootstrap/test on the backported change but I attached it. And YES it does fix the issue here.
I'll finish it off -- thank you again!
The same code is present on the 12 branch, so better fix it there as well than waiting for somebody to run into another miscompile.
https://inbox.sourceware.org/gcc-patches/ab19ccc3ba9b1d6559039e0c6d37f2b9280bb870.1728822647.git.sam@gentoo.org/ I'll test 12 next.
13 done in r13-9114-ga987affa2b10cd, will test 12 tonight.
Ah, I'd already tested this one for 12, just not the queued IPA stuff yet. Done in r12-10766-gea7d7818fdc7a4, hence fixed for 12.5 and 13.3.