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).
