https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476
Thomas Schwinge <tschwinge at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tschwinge at gcc dot gnu.org --- Comment #20 from Thomas Schwinge <tschwinge at gcc dot gnu.org> --- (In reply to Richard Biener from comment #18) > /home/rguenther/src/trunk/libgomp/oacc-plugin.c: In function > 'GOMP_PLUGIN_acc_default_dim': > /home/rguenther/src/trunk/libgomp/oacc-plugin.c:65:7: error: statement is > not reachable [-Werror] > 65 | return -1; > | ^~~~~~ (That's correct, and you do address that in the patch posted.) It feels strange to not have a 'return' in a non-'void' function, but that's fine, given 'gomp_fatal' being 'noreturn'. For posterity (only; these "bad" cases have not made it into the patch posted): > /home/rguenther/src/trunk/libgomp/oacc-profiling.c: In function > 'acc_prof_register': > /home/rguenther/src/trunk/libgomp/oacc-profiling.c:354:7: error: statement > is not reachable [-Werror] > 354 | __builtin_unreachable (); > | ^~~~~~~~~~~~~~~~~~~~~ > /home/rguenther/src/trunk/libgomp/oacc-profiling.c: In function > 'acc_prof_unregister': > /home/rguenther/src/trunk/libgomp/oacc-profiling.c:475:7: error: statement > is not reachable [-Werror] > 475 | __builtin_unreachable (); > | ^~~~~~~~~~~~~~~~~~~~~ > > the latter two are an issue with inital CFG construction I think, where > group_case_labels turns > > void bar (foo x) > { > <bb 2> : > switch (x) <default: <L2>, case 0: <L0>, case 1: <L1>> > > <bb 3> : > <L0>: > goto <L2>; > > <bb 4> : > <L1>: > __builtin_unreachable (); > > <bb 5> : > <L2>: > return; > > into the following with BB 4 now unreachable. > > void bar (foo x) > { > <bb 2> : > switch (x) <default: <L2>, case 0: <L0>> > > <bb 3> : > <L0>: > goto <L2>; > > <bb 4> : > <L1>: > __builtin_unreachable (); > > <bb 5> : > <L2>: > return; The source-level situation here is: [...] 256 /* Special cases. */ 257 if (reg == acc_toggle) [...] 274 else if (reg == acc_toggle_per_thread) 275 { [...] 284 /* Silently ignore. */ 285 gomp_debug (0, " ignoring bogus request\n"); 286 return; 287 } [...] 302 switch (reg) 303 { [...] 353 case acc_toggle_per_thread: 354 __builtin_unreachable (); 355 } [...] ..., and similar for the other instance. Here, the point is to (a) enumerate all possible 'enum' values in the 'switch (reg)', but (b) make it clear ('__builtin_unreachable') that we're not expecting 'acc_toggle_per_thread' here, as it has already been handled (plus early 'return') above. In my opinion, we shouldn't diagnose these cases (and you don't, per the patch posted).