From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) by sourceware.org (Postfix) with ESMTPS id 1E58C3945C2C for ; Tue, 13 Jul 2021 07:56:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1E58C3945C2C Received: by mail-ej1-x62e.google.com with SMTP id gn32so39908159ejc.2 for ; Tue, 13 Jul 2021 00:56:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8Ee4Mm+tgtMynCHXSRIBU8QGmAh+61zyk6O/RIM1NeY=; b=Lp0+aF9xK7AlCJivo0YaFgQ5JLpJCzqZutQZSrBBl2q6QoxjxtWWPnDL4MuTAevvmO wBPjd5nthQgsNPUmj+0BVVP5kCxB6QUk9etWzFn8/5iupWyCyALT8Xo7MXsuNSx1GIYK j+syCAjknaYXmgrINHC17+siZwwhDn5xP8MSXpu+QuFrjJzeY7TVNFdizpfbzx5u0x58 b6CcFc+bGRAKmVBwhTn/ADphQ+nO/wbPxzdT8nzIHCGSJCZ1P+NMFGduWXmu0abFZR7Y BM45DoK0LiaAVJAO6IZXEvu93NgDGJ8TLNcT+7+7JnH8ikRF/9OCzPPwzu+oAgwMu/x8 zhlQ== X-Gm-Message-State: AOAM532+Wl2i7qfHQMDAoVP4ZKoVYZlL2B+5Dmcr7uBsx3Wc+dwMzVKA P9bHAhiP25bZLEZQ/OeF7pV08Zsc+EbRFGa/i14= X-Google-Smtp-Source: ABdhPJwI5vpBi+kkVRi/RNym0X+JFZsUgIj3HskuXu98duq32wRIqjRb0HAGwfOWYFKEycwFP29kqJXSL6DxIY/EAoo= X-Received: by 2002:a17:906:e1a:: with SMTP id l26mr4084942eji.129.1626163012188; Tue, 13 Jul 2021 00:56:52 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Tue, 13 Jul 2021 09:56:41 +0200 Message-ID: Subject: Re: move unreachable user labels to entry point To: Alexandre Oliva , Jakub Jelinek Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Jul 2021 07:56:55 -0000 On Tue, Jul 13, 2021 at 5:11 AM Alexandre Oliva wrote: > > > pr42739.C, complicated by some extra wrappers and cleanups from a > feature I'm working on, got me very confused because a user label > ended up in a cleanup introduced by my pass, where it couldn't > possibly have been initially. > > The current logic may move such an unreachable user label multiple > times, if it ends up in blocks that get removed one after the other. > Since it doesn't really matter where it lands (save for omp > constraints), I propose we move it once and for all to a stable, final > location, that we currently use only as a last resort: the entry > point. > > Regstrapped on x86_64-linux-gnu. Ok to install? > > > for gcc/ChangeLog > > * tree-cfg.c (remove_bb): When preserving an unreachable user > label, use the entry block as the preferred choice for its > surviving location, rather than as a last resort. > --- > gcc/tree-cfg.c | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) > > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c > index 1f0f4a2c6eb2c..f6f005f10a9f5 100644 > --- a/gcc/tree-cfg.c > +++ b/gcc/tree-cfg.c > @@ -2300,13 +2300,11 @@ remove_bb (basic_block bb) > FORCED_LABEL (gimple_label_label (label_stmt)) = 1; > } > > - new_bb = bb->prev_bb; > - /* Don't move any labels into ENTRY block. */ > - if (new_bb == ENTRY_BLOCK_PTR_FOR_FN (cfun)) > - { > - new_bb = single_succ (new_bb); > - gcc_assert (new_bb != bb); > - } > + /* We have to move the unreachable label somewhere. > + Moving it to the entry block makes sure it's moved at > + most once, and avoids messing with anonymous landing > + pad labels. */ > + new_bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)); > if ((unsigned) bb->index < bb_to_omp_idx.length () > && ((unsigned) new_bb->index >= bb_to_omp_idx.length () > || (bb_to_omp_idx[bb->index] > @@ -2316,7 +2314,6 @@ remove_bb (basic_block bb) > into the right OMP region. */ The comment above says /* During cfg pass make sure to put orphaned labels into the right OMP region. */ and the full guard is if ((unsigned) bb->index < bb_to_omp_idx.length () && ((unsigned) new_bb->index >= bb_to_omp_idx.length () || (bb_to_omp_idx[bb->index] != bb_to_omp_idx[new_bb->index]))) The right OMP region suggests something wrt correctness (though the original choice of bb->prev_bb doesn't put the initial new_bb in an semantic relation to the original block). So I wonder what the code was for an whether we still need it. The ENTRY_BLOCK successor can be in some OMP region I guess but we're still falling back to a "mismatched" OMP region in case it was. Then we could also insert on the ENTRY_BLOCK single successor edge... That said, the patch doesn't look incorrect - it just tweaks heuristics - but the OMP code looks odd. Richard. > unsigned int i; > int idx; > - new_bb = NULL; > FOR_EACH_VEC_ELT (bb_to_omp_idx, i, idx) > if (i >= NUM_FIXED_BLOCKS > && idx == bb_to_omp_idx[bb->index] > @@ -2325,11 +2322,6 @@ remove_bb (basic_block bb) > new_bb = BASIC_BLOCK_FOR_FN (cfun, i); > break; > } > - if (new_bb == NULL) > - { > - new_bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)); > - gcc_assert (new_bb != bb); > - } > } > new_gsi = gsi_after_labels (new_bb); > gsi_remove (&i, false); > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Disinformation flourishes because many people care deeply about injustice > but very few check the facts. Ask me about