Message ID | 13oq9o43-6q30-orp1-281q-4q2859sn7r49@fhfr.qr |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 76AB73858408 for <patchwork@sourceware.org>; Thu, 25 Nov 2021 13:23:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 76AB73858408 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1637846633; bh=V/gHcHAKUE6hqcartvdMuqfzy/xbFbd2WkOf8ytBCZo=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=afdnJwni1QEf2bWwfU7jvMyH/hxE9O2u9tihi7yCoId3cLNvgcGLa8hlJ9wiQncwr 3rrdOzJlm/Dj7Cyw+uQb5mXCFeHz7iz93CuGAfLXIHGj8BsbQXBFj/mB7ryillCL/0 Kj5kWPoDcb90H3kj/CJ/U65/i113SrizZW3iiAIE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 980C73858D35 for <gcc-patches@gcc.gnu.org>; Thu, 25 Nov 2021 13:23:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 980C73858D35 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 9F34E2114D for <gcc-patches@gcc.gnu.org>; Thu, 25 Nov 2021 13:23:23 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 8784B13C3A for <gcc-patches@gcc.gnu.org>; Thu, 25 Nov 2021 13:23:23 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id r8J0H0uOn2FYTwAAMHmgww (envelope-from <rguenther@suse.de>) for <gcc-patches@gcc.gnu.org>; Thu, 25 Nov 2021 13:23:23 +0000 Date: Thu, 25 Nov 2021 14:23:23 +0100 (CET) To: gcc-patches@gcc.gnu.org Subject: [PATCH] [RFC] unreachable returns Message-ID: <13oq9o43-6q30-orp1-281q-4q2859sn7r49@fhfr.qr> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Richard Biener <rguenther@suse.de> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[RFC] unreachable returns
|
|
Commit Message
Richard Biener
Nov. 25, 2021, 1:23 p.m. UTC
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 <rguenther@suse.de> gcc/c/ * c-typeck.c (c_tree_equal): Remove unreachable return. --- gcc/c/c-typeck.c | 2 -- 1 file changed, 2 deletions(-)
Comments
> 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. I think this is just remat of times we did not have __builtin_unreachable. I like the idea of removing the redundant code. Honza
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 <rguenther@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
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index b71358e1821..7524304f2bd 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -15984,8 +15984,6 @@ c_tree_equal (tree t1, tree t2) default: gcc_unreachable (); } - /* We can get here with --disable-checking. */ - return false; } /* Returns true when the function declaration FNDECL is implicit,