From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id C712A3857804 for ; Sat, 5 Dec 2020 10:22:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C712A3857804 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id DA4A4AB63; Sat, 5 Dec 2020 10:22:25 +0000 (UTC) Date: Sat, 05 Dec 2020 11:22:24 +0100 User-Agent: K-9 Mail for Android In-Reply-To: <20201205091449.GY3788@tucnak> References: <20201205091449.GY3788@tucnak> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [PATCH] phiopt: Handle bool in two_value_replacement [PR796232] To: Jakub Jelinek CC: gcc-patches@gcc.gnu.org From: Richard Biener Message-ID: <991616BD-554D-4D9F-BA6C-AE5B16361618@suse.de> X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Sat, 05 Dec 2020 10:22:28 -0000 On December 5, 2020 10:14:49 AM GMT+01:00, Jakub Jelinek wrote: >Hi! > >The following patch improves code generation on the included testcase >by >enabling two_value_replacement on booleans=2E It does that only for >arg0/arg1 >values that conditional_replacement doesn't handle, and only does it if >not >in the early phiopt pass, because conditional_replacement isn't done >early >either=2E >I must say I'm not sure about that, in PR87105 / PR87608 you've added >the >early phiopt pass and specifically excluded conditional_replacement and >a >few others from being optimized early, but then 3 months later I've >added >two_value_replacement and didn't restrict it to !early_p=2E Shall we >instead >do two_value_replacement only in late phiopt? Yeah, I guess that would make sense=2E I don't remember exactly but I saw = regressions with early doing if conversion besides min/max/abs detection=2E= =20 >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok=2E=20 Richard=2E=20 >2020-12-05 Jakub Jelinek > > PR tree-optimization/96232 > * tree-ssa-phiopt=2Ec (two_value_replacement): Add early_p argument=2E > If false, optimize even boolean lhs cases as long as arg0 has wider > precision and conditional_replacement doesn't handle that case=2E > > * gcc=2Edg/tree-ssa/pr96232-2=2Ec: New test=2E > >--- gcc/tree-ssa-phiopt=2Ec=2Ejj 2020-12-04 17:27:53=2E472837921 +0100 >+++ gcc/tree-ssa-phiopt=2Ec 2020-12-04 17:54:56=2E070689584 +0100 >@@ -51,7 +51,7 @@ along with GCC; see the file COPYING3=2E >=20 > static unsigned int tree_ssa_phiopt_worker (bool, bool, bool); >static bool two_value_replacement (basic_block, basic_block, edge, gphi >*, >- tree, tree); >+ tree, tree, bool); > static bool conditional_replacement (basic_block, basic_block, > edge, edge, gphi *, tree, tree); >static gphi *factor_out_conditional_conversion (edge, edge, gphi *, >tree, tree, >@@ -337,7 +337,7 @@ tree_ssa_phiopt_worker (bool do_store_el > } >=20 > /* Do the replacement of conditional if it can be done=2E */ >- if (two_value_replacement (bb, bb1, e2, phi, arg0, arg1)) >+ if (two_value_replacement (bb, bb1, e2, phi, arg0, arg1, early_p)) > cfgchanged =3D true; > else if (!early_p > && conditional_replacement (bb, bb1, e1, e2, phi, >@@ -614,7 +614,7 @@ factor_out_conditional_conversion (edge >=20 > static bool > two_value_replacement (basic_block cond_bb, basic_block middle_bb, >- edge e1, gphi *phi, tree arg0, tree arg1) >+ edge e1, gphi *phi, tree arg0, tree arg1, bool early_p) > { > /* Only look for adjacent integer constants=2E */ > if (!INTEGRAL_TYPE_P (TREE_TYPE (arg0)) >@@ -635,7 +635,7 @@ two_value_replacement (basic_block cond_ >=20 > if (TREE_CODE (lhs) !=3D SSA_NAME > || !INTEGRAL_TYPE_P (TREE_TYPE (lhs)) >- || TREE_CODE (TREE_TYPE (lhs)) =3D=3D BOOLEAN_TYPE >+ || (early_p && TREE_CODE (TREE_TYPE (lhs)) =3D=3D BOOLEAN_TYPE) > || TREE_CODE (rhs) !=3D INTEGER_CST) > return false; >=20 >@@ -648,9 +648,25 @@ two_value_replacement (basic_block cond_ > return false; > } >=20 >+ /* Defer boolean x ? 0 : {1,-1} or x ? {1,-1} : 0 to >+ conditional_replacement=2E */ >+ if (TREE_CODE (TREE_TYPE (lhs)) =3D=3D BOOLEAN_TYPE >+ && (integer_zerop (arg0) >+ || integer_zerop (arg1) >+ || TREE_CODE (TREE_TYPE (arg0)) =3D=3D BOOLEAN_TYPE >+ || (TYPE_PRECISION (TREE_TYPE (arg0)) >+ <=3D TYPE_PRECISION (TREE_TYPE (lhs))))) >+ return false; >+ > wide_int min, max; >- if (get_range_info (lhs, &min, &max) !=3D VR_RANGE >- || min + 1 !=3D max >+ if (TREE_CODE (TREE_TYPE (lhs)) =3D=3D BOOLEAN_TYPE) >+ { >+ min =3D wi::to_wide (boolean_false_node); >+ max =3D wi::to_wide (boolean_true_node); >+ } >+ else if (get_range_info (lhs, &min, &max) !=3D VR_RANGE) >+ return false; >+ if (min + 1 !=3D max > || (wi::to_wide (rhs) !=3D min > && wi::to_wide (rhs) !=3D max)) > return false; >--- gcc/testsuite/gcc=2Edg/tree-ssa/pr96232-2=2Ec=2Ejj 2020-12-04 >17:56:22=2E180730234 +0100 >+++ gcc/testsuite/gcc=2Edg/tree-ssa/pr96232-2=2Ec 2020-12-04 >17:57:27=2E973997239 +0100 >@@ -0,0 +1,18 @@ >+/* PR tree-optimization/96232 */ >+/* { dg-do compile } */ >+/* { dg-options "-O2 -fdump-tree-optimized" } */ >+/* { dg-final { scan-tree-dump " 38 - " "optimized" } } */ >+/* { dg-final { scan-tree-dump " \\+ 97;" "optimized" } } */ >+/* { dg-final { scan-tree-dump-not "PHI <" "optimized" } } */ >+ >+int >+foo (_Bool x) >+{ >+ return x ? 37 : 38; >+} >+ >+int >+bar (_Bool x) >+{ >+ return x ? 98 : 97; >+} > > Jakub