From patchwork Wed Sep 13 10:44:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 813333 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-462020-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="hS5AyIuU"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xsdZs15MWz9s5L for ; Wed, 13 Sep 2017 20:44:36 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=moRIigHF54aIJguQp4uoXbi4xqp1Z +zqTrM1LXS2Vqxf97APrgXrjBwrB+/a6roTR5qoGjGcGyShUtWenHb6DMJgyLJV9 i/FUWM7wgcM74kaJ/IOKytiOJuV8RlYHy1FWz0ZbXPePoyAmVbeLJt+O3UkcIE0O MS0NBsz89uNYyw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=FZjH/idAb83cekPZXJxpHbIpMaM=; b=hS5 AyIuU9gSJrvSxnot0pOtHR1THL/VTqpYvwyqEV/gcZaTpvxaAIblQUrroPE7LqK7 6SPDlbJNlThH78DuDyED7wyAdqMWLFh8lmeIAkMd46xdBE8Jv/HsS5BelRalrAx6 hEdOiXIeM5A4uE2MNFwwKgfPvOFdFcDdUjLxs3UA= Received: (qmail 79163 invoked by alias); 13 Sep 2017 10:44:26 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 78875 invoked by uid 89); 13 Sep 2017 10:44:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=gathered X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Sep 2017 10:44:24 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 04F307E424; Wed, 13 Sep 2017 10:44:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 04F307E424 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jakub@redhat.com Received: from tucnak.zalov.cz (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8BE9A5FCA9; Wed, 13 Sep 2017 10:44:22 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v8DAiJTw018678; Wed, 13 Sep 2017 12:44:19 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v8DAiIks018677; Wed, 13 Sep 2017 12:44:18 +0200 Date: Wed, 13 Sep 2017 12:44:17 +0200 From: Jakub Jelinek To: Richard Biener , Jeff Law , Alexandre Oliva Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix -fcompare-debug issues in find_many_sub_basic_blocks (PR target/81325) Message-ID: <20170913104417.GW1701@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes Hi! The following testcase fails on powerpc64le with -fcompare-debug. The problem is that find_many_sub_basic_blocks is called during expansion on a basic block where originally two __atomic_fetch_add calls have some debug stmts in between. The rs6000 backend expands those atomic calls into a loop that starts with a CODE_LABEL and ends with a JUMP_INSN, so find_many_sub_basic_blocks must end a basic block after the JUMP_INSN from the first call and start a new basic block at the start of the second call. If there are no DEBUG_INSNs in between, it creates just those 2 basic blocks, which is correct, but it doesn't ignore DEBUG_INSNs when deciding what to do and thus when there are DEBUG_ISNSs in between the flow_transfer_insn (JUMP_INSN) and CODE_LABEL, it creates yet another basic block that has just the DEBUG_INSNs in it. That means different cfg between -g and -g0 and more differences later on. As I wrote in the PR, if we have 2 loops and some code in between at GIMPLE, and in that code in between DCE all the non-debug stmts and have only debug stmts in there, we consider it a forwarder block and delete it with all those debug stmts (it is one of the unfortunate cases where we drop them on the floor, but there isn't really much we can do with those without adding new extensions like conditional debug stmts or debug phis). The following patch attempts to do the same thing on RTL during find_many_sub_basic_blocks, i.e. ignore debug stmts when deciding what to do, trying to preserve debug insns somewhere if that is easily possible (if there is say a conditional jump with a fallthrough into debug insns followed by some real insn (not a CODE_LABEL nor BARRIER), the debug insns can go at the start of the fallthrough bb that would be created anyway). If we have an insn that must end a bb and another one that must start a bb (CODE_LABEL) and only debug insns in between, we remove them. Bootstrapped/regtested on x86_64-linux and i686-linux (in these two cases I've also gathered statistics and no debug insns during the bootstraps/regtests were actually removed) and powerpc64le-linux (forgot to apply the statistics patch, so no details, but it surely triggers at least on the testcase). Ok for trunk? 2017-09-13 Jakub Jelinek PR target/81325 * cfgbuild.c (find_bb_boundaries): Ignore debug insns in decisions if and where to split a bb, except for splitting before debug insn sequences followed by non-label real insn. Delete debug insns in between basic blocks. * g++.dg/cpp0x/pr81325.C: New test. Jakub --- gcc/cfgbuild.c.jj 2017-09-12 21:57:53.084514168 +0200 +++ gcc/cfgbuild.c 2017-09-13 09:15:57.701521179 +0200 @@ -442,9 +442,10 @@ find_bb_boundaries (basic_block bb) rtx_insn *end = BB_END (bb), *x; rtx_jump_table_data *table; rtx_insn *flow_transfer_insn = NULL; + rtx_insn *debug_insn = NULL; edge fallthru = NULL; - if (insn == BB_END (bb)) + if (insn == end) return; if (LABEL_P (insn)) @@ -455,22 +456,43 @@ find_bb_boundaries (basic_block bb) { enum rtx_code code = GET_CODE (insn); + if (code == DEBUG_INSN) + { + if (flow_transfer_insn && !debug_insn) + debug_insn = insn; + } /* In case we've previously seen an insn that effects a control flow transfer, split the block. */ - if ((flow_transfer_insn || code == CODE_LABEL) - && inside_basic_block_p (insn)) + else if ((flow_transfer_insn || code == CODE_LABEL) + && inside_basic_block_p (insn)) { - fallthru = split_block (bb, PREV_INSN (insn)); + rtx_insn *prev = PREV_INSN (insn); + + /* If the first non-debug inside_basic_block_p insn after a control + flow transfer is not a label, split the block before the debug + insn instead of before the non-debug insn, so that the debug + insns are not lost. */ + if (debug_insn && code != CODE_LABEL && code != BARRIER) + prev = PREV_INSN (debug_insn); + fallthru = split_block (bb, prev); if (flow_transfer_insn) { BB_END (bb) = flow_transfer_insn; + rtx_insn *next; /* Clean up the bb field for the insns between the blocks. */ for (x = NEXT_INSN (flow_transfer_insn); x != BB_HEAD (fallthru->dest); - x = NEXT_INSN (x)) - if (!BARRIER_P (x)) - set_block_for_insn (x, NULL); + x = next) + { + next = NEXT_INSN (x); + /* Debug insns should not be in between basic blocks, + drop them on the floor. */ + if (DEBUG_INSN_P (x)) + delete_insn (x); + else if (!BARRIER_P (x)) + set_block_for_insn (x, NULL); + } } bb = fallthru->dest; @@ -480,6 +502,7 @@ find_bb_boundaries (basic_block bb) bb->frequency = 0; bb->count = profile_count::uninitialized (); flow_transfer_insn = NULL; + debug_insn = NULL; if (code == CODE_LABEL && LABEL_ALT_ENTRY_P (insn)) make_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun), bb, 0); } @@ -502,17 +525,23 @@ find_bb_boundaries (basic_block bb) /* In case expander replaced normal insn by sequence terminating by return and barrier, or possibly other sequence not behaving like ordinary jump, we need to take care and move basic block boundary. */ - if (flow_transfer_insn) + if (flow_transfer_insn && flow_transfer_insn != end) { BB_END (bb) = flow_transfer_insn; /* Clean up the bb field for the insns that do not belong to BB. */ - x = flow_transfer_insn; - while (x != end) + rtx_insn *next; + for (x = NEXT_INSN (flow_transfer_insn); ; x = next) { - x = NEXT_INSN (x); - if (!BARRIER_P (x)) + next = NEXT_INSN (x); + /* Debug insns should not be in between basic blocks, + drop them on the floor. */ + if (DEBUG_INSN_P (x)) + delete_insn (x); + else if (!BARRIER_P (x)) set_block_for_insn (x, NULL); + if (x == end) + break; } } --- gcc/testsuite/g++.dg/cpp0x/pr81325.C.jj 2017-09-12 22:17:54.662002067 +0200 +++ gcc/testsuite/g++.dg/cpp0x/pr81325.C 2017-09-13 09:21:44.536059474 +0200 @@ -0,0 +1,84 @@ +// PR target/81325 +// { dg-do compile { target c++11 } } +// { dg-options "-O2 -fcompare-debug" } + +struct A { A(const char *, const int & = 0); }; +template struct B; +template struct C { + int _M_i; + void m_fn1() { __atomic_fetch_add(&_M_i, 0, __ATOMIC_RELAXED); } +}; +struct D { + int *Data; + long Length = 0; + D(int) : Data() {} +}; +template <> struct B : C {}; +struct F { + B RefCount; + void m_fn2() { RefCount.m_fn1(); } +}; +struct G { + F *Obj; + G(const G &p1) : Obj(p1.Obj) { + if (Obj) { + F *a = 0; + a->m_fn2(); + } + } +}; +struct H { + int CPlusPlus : 1; +}; +struct I { + enum {} KindId; +}; +template struct J { + void operator()(); + ResultT operator()(ArgT) {} +}; +struct K { + int AllowBind; + I SupportedKind; + I RestrictKind; + G Implementation; +}; +struct L { + L(int) : Implementation(Implementation) {} + K Implementation; +}; +struct M { + int Param1; +}; +struct N { + N(int, L &p2) : Param2(p2) {} + L Param2; +}; +struct O { + L m_fn3(); +}; +L ignoringImpCasts(L); +J b; +L hasName(const A &); +M hasOverloadedOperatorName(D); +J c; +struct P { + void m_fn4(L, int); +}; +struct Q { + void m_fn5(P *); +}; +H d; +void Q::m_fn5(P *p1) { + if (!d.CPlusPlus) { + c(); + L e = 0, f = ignoringImpCasts(e); + b(ignoringImpCasts(f)).m_fn3(); + } + hasOverloadedOperatorName(0); + hasName(""); + L g = 0; + N(0, g); + L h(0); + p1->m_fn4(h, 0); +}