From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by sourceware.org (Postfix) with ESMTPS id A73343858022 for ; Thu, 15 Jun 2023 06:17:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A73343858022 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=marvell.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=marvell.com Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35EJuvLX004340 for ; Wed, 14 Jun 2023 23:17:22 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=JJsU2YQwTclcfK4OfZaUZXGxreEucpET0TsasGjFR5w=; b=Wg9ESitLNo92j2DjK9VkyYZ6PWkVtwmICfcs5x+7iKedRwhT/NqBgYhD5NJy9qgf3Jlq BTTHASrTVGMabXG/AaZ1Eqe48xgzYD3atvyj0XcwSgVOFZROMVYx0PVZT7bHHU0j8L3C HIu0078h5oDvqv8DbPzTHcLaRIMSaV7h3Q2e0P1rkdf2o2bvyu0ILVAZKJgU/W1DCTGb qMeGir8LpulsPswNL/oJluqwYFF3irnYR5LKHBO1e0SZlCJt+EMFBzbjZX+vcg647534 UwIfOjdQJGGpaWmoCA0a7AJzjgQA3y5ggE+Ii1Z/ruGymdbnnF0Vr0FtCJTCdYZN83YB Yg== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0b-0016f401.pphosted.com (PPS) with ESMTPS id 3r7ky1sq4p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT) for ; Wed, 14 Jun 2023 23:17:21 -0700 Received: from DC5-EXCH02.marvell.com (10.69.176.39) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.48; Wed, 14 Jun 2023 23:17:20 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server id 15.0.1497.48 via Frontend Transport; Wed, 14 Jun 2023 23:17:20 -0700 Received: from vpnclient.wrightpinski.org.com (unknown [10.69.242.187]) by maili.marvell.com (Postfix) with ESMTP id D8B773F70DD; Wed, 14 Jun 2023 23:17:16 -0700 (PDT) From: Andrew Pinski To: CC: Andrew Pinski Subject: [PATCH] Fix tree-opt/110252: wrong code due to phiopt using flow sensitive info during match Date: Wed, 14 Jun 2023 23:17:04 -0700 Message-ID: <20230615061704.3514834-1-apinski@marvell.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-GUID: c2NM8V1hTBIJdVIerEqKG1q5Lo82X646 X-Proofpoint-ORIG-GUID: c2NM8V1hTBIJdVIerEqKG1q5Lo82X646 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-15_03,2023-06-14_02,2023-05-22_02 X-Spam-Status: No, score=-14.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Match will query ranger via tree_nonzero_bits/get_nonzero_bits for 2 and 3rd operand of the COND_EXPR and phiopt tries to do create the COND_EXPR even if we moving one statement. That one statement could have some flow sensitive information on it based on the condition that is for the COND_EXPR but that might create wrong code if the statement was moved out. So the solution here is to temporarily remove the flow sensitive info of the statements phiopt might move and try match only then. This is done via the new class auto_flow_sensitive. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. PR tree-optimization/110252 gcc/ChangeLog: * tree-ssa-phiopt.cc (match_simplify_replacement): Temporarily remove the flow sensitive info on the two statements that might be moved. * tree-ssanames.cc (auto_flow_sensitive::auto_flow_sensitive): New method. (auto_flow_sensitive::~auto_flow_sensitive): New method. * tree-ssanames.h (class auto_flow_sensitive): New class. gcc/testsuite/ChangeLog: * gcc.c-torture/execute/pr110252.c: New test. * gcc.c-torture/execute/pr110252-1.c: New test. * gcc.dg/tree-ssa/phi-opt-25b.c: Updated as __builtin_parity loses the nonzerobits info. --- .../gcc.c-torture/execute/pr110252-1.c | 15 +++++++ .../gcc.c-torture/execute/pr110252.c | 10 +++++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25b.c | 6 +-- gcc/tree-ssa-phiopt.cc | 12 ++++-- gcc/tree-ssanames.cc | 40 +++++++++++++++++++ gcc/tree-ssanames.h | 21 ++++++++++ 6 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110252-1.c create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110252.c diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110252-1.c b/gcc/testsuite/gcc.c-torture/execute/pr110252-1.c new file mode 100644 index 00000000000..4ae93ca0647 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr110252-1.c @@ -0,0 +1,15 @@ +/* This is reduced from sel-sched.cc which was noticed was being miscompiled too. */ +int g(int min_need_stall) __attribute__((__noipa__)); +int g(int min_need_stall) +{ + return min_need_stall < 0 ? 1 : ((min_need_stall) < (1) ? (min_need_stall) : (1)); +} +int main(void) +{ + for(int i = -100; i <= 100; i++) + { + int t = g(i); + if (t != (i!=0)) + __builtin_abort(); + } +} diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110252.c b/gcc/testsuite/gcc.c-torture/execute/pr110252.c new file mode 100644 index 00000000000..7f1a7dbf134 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr110252.c @@ -0,0 +1,10 @@ +signed char f() __attribute__((__noipa__)); +signed char f() { return 0; } +int main() +{ + int g = f() - 1; + int e = g < 0 ? 1 : ((g >> (8-2))!=0); + asm("":"+r"(e)); + if (e != 1) + __builtin_abort(); +} diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25b.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25b.c index 7298da0c96e..0fd9b004a03 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25b.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25b.c @@ -65,8 +65,6 @@ int test_popcountll(unsigned long long x, unsigned long long y) return x ? __builtin_popcountll(y) : 0; } -/* 3 types of functions (not including parity), each with 3 types and there are 2 goto each */ -/* { dg-final { scan-tree-dump-times "goto " 18 "optimized" } } */ +/* 4 types of functions, each with 3 types and there are 2 goto each */ +/* { dg-final { scan-tree-dump-times "goto " 24 "optimized" } } */ /* { dg-final { scan-tree-dump-times "x_..D. != 0" 12 "optimized" } } */ -/* parity case will be optimized to x!=0 & parity(y) . */ -/* { dg-final { scan-tree-dump-times " & " 3 "optimized" } } */ diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index d9a1153086f..5f871e019e4 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -782,9 +782,15 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb, } tree type = TREE_TYPE (gimple_phi_result (phi)); - result = gimple_simplify_phiopt (early_p, type, stmt, - arg_true, arg_false, - &seq); + { + auto_flow_sensitive s1(stmt_to_move); + auto_flow_sensitive s_alt(stmt_to_move_alt); + + result = gimple_simplify_phiopt (early_p, type, stmt, + arg_true, arg_false, + &seq); + } + if (!result) return false; diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc index 5fdb6a37e9f..765d343ba49 100644 --- a/gcc/tree-ssanames.cc +++ b/gcc/tree-ssanames.cc @@ -812,6 +812,46 @@ reset_flow_sensitive_info_in_bb (basic_block bb) } } +/* Constructor for auto_flow_sensitive. Saves + off the ssa name's flow sensitive information + that was defined by gimple statement S and + resets it to be non-flow based ones. */ +auto_flow_sensitive::auto_flow_sensitive (gimple *s) +{ + if (!s) + return; + + name = gimple_get_lhs (s); + + if (!POINTER_TYPE_P (TREE_TYPE (name))) + { + range_info = SSA_NAME_RANGE_INFO (name); + SSA_NAME_RANGE_INFO (name) = NULL; + } + else if (SSA_NAME_PTR_INFO (name)) + { + get_ptr_info_alignment (SSA_NAME_PTR_INFO (name), &align, &misalign); + null = SSA_NAME_PTR_INFO (name)->pt.null; + } +} + +/* Deconstructor, restores the flow sensitive information + for the SSA name. */ +auto_flow_sensitive::~auto_flow_sensitive () +{ + if (!name) + return; + + if (!POINTER_TYPE_P (TREE_TYPE (name))) + SSA_NAME_RANGE_INFO (name) = range_info; + else if (SSA_NAME_PTR_INFO (name)) + { + if (align) + set_ptr_info_alignment (SSA_NAME_PTR_INFO (name), align, misalign); + SSA_NAME_PTR_INFO (name)->pt.null = null; + } +} + /* Release all the SSA_NAMEs created by STMT. */ void diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h index f3fa609208a..28b83b779fa 100644 --- a/gcc/tree-ssanames.h +++ b/gcc/tree-ssanames.h @@ -137,4 +137,25 @@ make_temp_ssa_name (tree type, gimple *stmt, const char *name) } +/* RAII style class to temporarily remove flow sensitive + from a ssa name defined by a gimple statement. + An usage example is inside phiopt, where using match-and-simplify + but since the statement is from inside a conditional bb, the + flow sensitive information can't be used. */ +class auto_flow_sensitive +{ +public: + auto_flow_sensitive (gimple *s); + ~auto_flow_sensitive (); +private: + tree name = nullptr; + /* The range info for non pointers */ + vrange_storage *range_info; + /* Flow sensitive pointer information. */ + unsigned int align = 0; + unsigned int misalign = 0; + bool null = false; +}; + + #endif /* GCC_TREE_SSANAMES_H */ -- 2.31.1