On 11/25/2021 6:23 AM, Richard Biener via Gcc-patches wrote:
We have quite a number of "default" returns that cannot be reached.
One is particularly interesting since it says (see patch below):

      default:
        gcc_unreachable ();
      }
    /* We can get here with --disable-checking.  */
    return false;

which suggests that _maybe_ the intention was to have the
gcc_unreachable () which expands to __builtin_unreachable ()
with --disable-checking and thus a fallthru to "somewhere"
be catched with a "sane" default return value rather than
falling through to the next function or so.  BUT - that
isn't what actually happens since the 'return false' is
unreachable after CFG construction and will be elided.

In fact the IL after CFG construction is exactly the same
with and without the spurious return.

Now, I wonder if we should, instead of expanding
gcc_unreachable to __builtin_unreachable () with
--disable-checking, expand it to __builtin_trap ()
(or remove the --disable-checking variant completely,
always retaining assert level checking but maybe make
it cheaper in size by using __builtin_trap () or abort ())

Thoughts?

That said, I do have a set of changes removing such spurious
returns.

2021-11-25  Richard Biener  <rguent...@suse.de>

gcc/c/
        * c-typeck.c (c_tree_equal): Remove unreachable return.
I'd bet if you dig into the history you'll find that the return was added first to make enable-checking happy, then later we added the gcc_unreachable().

I think expanding to __builtin_trap is highly preferable to __builtin_unreachable and it's probably the lowest overhead option. I can also live with removing the -disable-checking variant and instead using something that always halts execution.  Once we're always halting execution on that path I have no objection to removing the extraneous return.

jeff

Reply via email to