From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by sourceware.org (Postfix) with ESMTPS id 514A23858D28 for ; Tue, 14 Dec 2021 11:10:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 514A23858D28 Received: from [10.10.3.232] (unknown [10.10.3.232]) by mail.ispras.ru (Postfix) with ESMTPSA id 6AC2440D403E; Tue, 14 Dec 2021 11:10:30 +0000 (UTC) Subject: Re: [RFC PATCH] tree-ssa-sink: do not sink to in front of setjmp To: Alexander Monakov , Richard Biener Cc: gcc-patches@gcc.gnu.org References: <4493B84A-DB6D-468B-86BF-DA5383D8CFE4@gmail.com> <794111d4-a64c-17e5-a4fe-f96e5182ee1@ispras.ru> From: =?UTF-8?B?0JDQu9C10LrRgdC10Lkg0J3Rg9GA0LzRg9GF0LDQvNC10YLQvtCy?= Message-ID: <9252c923-c86a-7c98-ec5f-97410fbca8ed@ispras.ru> Date: Tue, 14 Dec 2021 14:10:30 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <794111d4-a64c-17e5-a4fe-f96e5182ee1@ispras.ru> Content-Type: multipart/mixed; boundary="------------8D1E9C71164426FCF6451E19" Content-Language: en-US X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Dec 2021 11:10:35 -0000 This is a multi-part message in MIME format. --------------8D1E9C71164426FCF6451E19 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 13.12.2021 18:20, Alexander Monakov wrote: > On Mon, 13 Dec 2021, Richard Biener wrote: > >> On December 13, 2021 3:25:47 PM GMT+01:00, Alexander Monakov wrote: >>> Greetings! >>> >>> While testing our patch that reimplements -Wclobbered on GIMPLE we found >>> a case where tree-ssa-sink moves a statement to a basic block in front >>> of a setjmp call. >>> >>> I am confident that this is unintended and should be considered invalid >>> GIMPLE. >> Does CFG validation not catch this? That is, doesn't setjmp force the start of >> a new BB? > Oh, good point. There's stmt_start_bb_p which returns true for setjmp, but > gimple_verify_flow_info doesn't check it. I guess we can try adding that > and collect the fallout on bootstrap/regtest. Bootstrap looks good, but testsuite has some regression (the applied patch is below). The overall number of unexpected failures and unresolved testcases is around 100. The diff is in attachment. >> I think sinking relies on dominance and post dominance here but post dominance >> may be too fragile with the abnormal cycles which are likely not backwards >> reachable from exit. >> >> That said, checking for abnormal preds is OK, I just want to make sure we >> detect the invalid CFG - do we? > As above, no, otherwise it would have been caught much earlier than ICE'ing > our -Wclobbered patch :) > > Thank you. > Alexander The patch for CFG validation: diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index ebbd894ae03..92b08d1d6d8 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -5663,6 +5663,7 @@ gimple_verify_flow_info (void)         }        /* Verify that body of basic block BB is free of control flow.  */ +      gimple *prev_stmt = NULL;        for (; !gsi_end_p (gsi); gsi_next (&gsi))         {           gimple *stmt = gsi_stmt (gsi); @@ -5674,6 +5675,14 @@ gimple_verify_flow_info (void)               err = 1;             } +         if (prev_stmt && stmt_starts_bb_p (stmt, prev_stmt)) +           { +             error ("setjmp in the middle of basic block %d", bb->index); +             err = 1; +           } +         if (!is_gimple_debug (stmt)) +           prev_stmt = stmt; +           if (stmt_ends_bb_p (stmt))             found_ctrl_stmt = true; --------------8D1E9C71164426FCF6451E19 Content-Type: text/x-patch; charset=UTF-8; name="test_summary.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="test_summary.diff" 7a8,9 > FAIL: gcc.c-torture/compile/930111-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) > FAIL: gcc.c-torture/compile/930111-1.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) 85a88,148 > FAIL: c-c++-common/tm/cancel-1.c (internal compiler error) > FAIL: c-c++-common/tm/cancel-1.c (test for excess errors) > FAIL: gcc.dg/tm/data-2.c (internal compiler error) > FAIL: gcc.dg/tm/data-2.c (test for excess errors) > FAIL: gcc.dg/tm/debug-1.c (internal compiler error) > FAIL: gcc.dg/tm/debug-1.c (test for excess errors) > FAIL: gcc.dg/tm/memopt-10.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-10.c (test for excess errors) > FAIL: gcc.dg/tm/memopt-11.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-11.c (test for excess errors) > FAIL: gcc.dg/tm/memopt-12.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-12.c (test for excess errors) > FAIL: gcc.dg/tm/memopt-15.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-15.c (test for excess errors) > UNRESOLVED: gcc.dg/tm/memopt-15.c scan-assembler _ITM_LM128 > FAIL: gcc.dg/tm/memopt-16.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-16.c (test for excess errors) > FAIL: gcc.dg/tm/memopt-3.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-3.c (test for excess errors) > FAIL: gcc.dg/tm/memopt-4.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-4.c (test for excess errors) > UNRESOLVED: gcc.dg/tm/memopt-4.c scan-tree-dump-times tmedge "tm_save.[0-9_]+ = lala.x\\\\[55\\\\]" 1 > UNRESOLVED: gcc.dg/tm/memopt-4.c scan-tree-dump-times tmedge "lala.x\\\\[55\\\\] = tm_save" 1 > FAIL: gcc.dg/tm/memopt-5.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-5.c (test for excess errors) > UNRESOLVED: gcc.dg/tm/memopt-5.c scan-tree-dump-times tmedge "ITM_LU[0-9] \\\\(&lala.x\\\\[55\\\\]" 1 > FAIL: gcc.dg/tm/memopt-6.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-6.c (test for excess errors) > UNRESOLVED: gcc.dg/tm/memopt-6.c scan-tree-dump-times tmedge "memcpyRtWn \\\\(.*, &lacopy" 1 > FAIL: gcc.dg/tm/memopt-7.c (internal compiler error) > FAIL: gcc.dg/tm/memopt-7.c (test for excess errors) > UNRESOLVED: gcc.dg/tm/memopt-7.c scan-tree-dump-times tmedge "tm_save.[0-9_]+ = lala" 1 > UNRESOLVED: gcc.dg/tm/memopt-7.c scan-tree-dump-times tmedge "lala = tm_save" 1 > FAIL: gcc.dg/tm/opt-1.c (internal compiler error) > FAIL: gcc.dg/tm/opt-1.c (test for excess errors) > FAIL: gcc.dg/tm/pr55401.c (internal compiler error) > FAIL: gcc.dg/tm/pr55401.c (test for excess errors) > UNRESOLVED: gcc.dg/tm/pr55401.c scan-tree-dump-times optimized "ITM_WU[0-9] \\\\(&george," 2 > FAIL: gcc.dg/tm/pr95569.c (internal compiler error) > FAIL: gcc.dg/tm/pr95569.c (test for excess errors) > FAIL: gcc.dg/torture/pr100053.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) > FAIL: gcc.dg/torture/pr100053.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (test for excess errors) > UNRESOLVED: gcc.dg/torture/pr100053.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects compilation failed to produce executable > FAIL: gcc.dg/torture/pr103458.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) > FAIL: gcc.dg/torture/pr103458.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) > FAIL: gcc.dg/torture/pr103596.c -O1 (internal compiler error) > FAIL: gcc.dg/torture/pr103596.c -O1 (test for excess errors) > FAIL: gcc.dg/torture/pr103596.c -O2 (internal compiler error) > FAIL: gcc.dg/torture/pr103596.c -O2 (test for excess errors) > FAIL: gcc.dg/torture/pr103596.c -O3 -g (internal compiler error) > FAIL: gcc.dg/torture/pr103596.c -O3 -g (test for excess errors) > FAIL: gcc.dg/torture/pr103596.c -Os (internal compiler error) > FAIL: gcc.dg/torture/pr103596.c -Os (test for excess errors) > FAIL: gcc.dg/torture/pr103596.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (internal compiler error) > FAIL: gcc.dg/torture/pr103596.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) > FAIL: gcc.dg/torture/pr103596.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (internal compiler error) > FAIL: gcc.dg/torture/pr103596.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (test for excess errors) > FAIL: gcc.dg/torture/pr79432.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) > FAIL: gcc.dg/torture/pr79432.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) > FAIL: gcc.dg/torture/pr82276.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (internal compiler error) > FAIL: gcc.dg/torture/pr82276.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) 89,90c152,153 < # of expected passes 175813 < # of unexpected failures 64 --- > # of expected passes 175777 > # of unexpected failures 118 92a156 > # of unresolved testcases 9 104,105d167 < FAIL: g++.dg/modules/xtreme-header-3_b.C -std=c++17 (test for excess errors) < FAIL: g++.dg/modules/xtreme-header-3_c.C -std=c++17 (test for excess errors) 109,110d170 < FAIL: g++.dg/modules/xtreme-header-3_b.C -std=c++2a (test for excess errors) < FAIL: g++.dg/modules/xtreme-header-3_c.C -std=c++2a (test for excess errors) 113a174,175 > FAIL: g++.dg/modules/xtreme-header-3_b.C -std=c++17 (test for excess errors) > FAIL: g++.dg/modules/xtreme-header-3_b.C -std=c++2a (test for excess errors) 114a177,178 > FAIL: g++.dg/modules/xtreme-header-3_c.C -std=c++17 (test for excess errors) > FAIL: g++.dg/modules/xtreme-header-3_c.C -std=c++2a (test for excess errors) 119,120d182 < FAIL: g++.dg/modules/xtreme-header-5_b.C -std=c++2a (test for excess errors) < FAIL: g++.dg/modules/xtreme-header-5_c.C -std=c++2a (test for excess errors) 123a186 > FAIL: g++.dg/modules/xtreme-header-5_b.C -std=c++2a (test for excess errors) 124a188 > FAIL: g++.dg/modules/xtreme-header-5_c.C -std=c++2a (test for excess errors) 129d192 < FAIL: g++.dg/modules/xtreme-header_b.C -std=c++17 (test for excess errors) 133d195 < FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2a (test for excess errors) 136a199,200 > FAIL: g++.dg/modules/xtreme-header_b.C -std=c++17 (test for excess errors) > FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2a (test for excess errors) 137a202,233 > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++98 (internal compiler error) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++98 (test for excess errors) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++14 (internal compiler error) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++14 (test for excess errors) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++17 (internal compiler error) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++17 (test for excess errors) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++2a (internal compiler error) > FAIL: c-c++-common/tm/cancel-1.c -std=gnu++2a (test for excess errors) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++98 (internal compiler error) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++98 (test for excess errors) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++14 (internal compiler error) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++14 (test for excess errors) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++17 (internal compiler error) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++17 (test for excess errors) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++2a (internal compiler error) > FAIL: g++.dg/tm/fatomic-1.C -std=gnu++2a (test for excess errors) > FAIL: g++.dg/tm/nested-3.C -std=gnu++98 (internal compiler error) > FAIL: g++.dg/tm/nested-3.C -std=gnu++98 (test for excess errors) > FAIL: g++.dg/tm/nested-3.C -std=gnu++14 (internal compiler error) > FAIL: g++.dg/tm/nested-3.C -std=gnu++14 (test for excess errors) > FAIL: g++.dg/tm/nested-3.C -std=gnu++17 (internal compiler error) > FAIL: g++.dg/tm/nested-3.C -std=gnu++17 (test for excess errors) > FAIL: g++.dg/tm/nested-3.C -std=gnu++2a (internal compiler error) > FAIL: g++.dg/tm/nested-3.C -std=gnu++2a (test for excess errors) > FAIL: g++.dg/tm/template-1.C -std=gnu++98 (internal compiler error) > FAIL: g++.dg/tm/template-1.C -std=gnu++98 (test for excess errors) > FAIL: g++.dg/tm/template-1.C -std=gnu++14 (internal compiler error) > FAIL: g++.dg/tm/template-1.C -std=gnu++14 (test for excess errors) > FAIL: g++.dg/tm/template-1.C -std=gnu++17 (internal compiler error) > FAIL: g++.dg/tm/template-1.C -std=gnu++17 (test for excess errors) > FAIL: g++.dg/tm/template-1.C -std=gnu++2a (internal compiler error) > FAIL: g++.dg/tm/template-1.C -std=gnu++2a (test for excess errors) 141,142c237,238 < # of expected passes 224294 < # of unexpected failures 38 --- > # of expected passes 224278 > # of unexpected failures 70 168a265,279 > FAIL: libitm.c/cancel.c (internal compiler error) > FAIL: libitm.c/cancel.c (test for excess errors) > UNRESOLVED: libitm.c/cancel.c compilation failed to produce executable > FAIL: libitm.c/dropref-2.c (internal compiler error) > FAIL: libitm.c/dropref-2.c (test for excess errors) > UNRESOLVED: libitm.c/dropref-2.c compilation failed to produce executable > FAIL: libitm.c/priv-1.c (internal compiler error) > FAIL: libitm.c/priv-1.c (test for excess errors) > UNRESOLVED: libitm.c/priv-1.c compilation failed to produce executable > FAIL: libitm.c/reentrant.c (internal compiler error) > FAIL: libitm.c/reentrant.c (test for excess errors) > UNRESOLVED: libitm.c/reentrant.c compilation failed to produce executable > FAIL: libitm.c++/eh-4.C (internal compiler error) > FAIL: libitm.c++/eh-4.C (test for excess errors) > UNRESOLVED: libitm.c++/eh-4.C compilation failed to produce executable 172,173c283,286 < # of expected passes 44 < # of expected failures 3 --- > # of expected passes 35 > # of unexpected failures 10 > # of expected failures 2 > # of unresolved testcases 5 --------------8D1E9C71164426FCF6451E19--