From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb30.google.com (mail-yb1-xb30.google.com [IPv6:2607:f8b0:4864:20::b30]) by sourceware.org (Postfix) with ESMTPS id 55A6E3861838 for ; Wed, 26 May 2021 11:07:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 55A6E3861838 Received: by mail-yb1-xb30.google.com with SMTP id f9so1449930ybo.6 for ; Wed, 26 May 2021 04:07:49 -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:content-transfer-encoding; bh=WQtRueKrLyvsiO8wCXB7arYUp7gMtxWdsqYALkfx510=; b=GZlnrEVLxzG29D30e8Pv/DcNmvNcBBgT10rzKbq45V+BTEpKaFdvkvMhzfkueOeEtM F+8IA4eYGX6QJ6PQbeYThCGlYo0XpsIOm0Yfcc5JTqOb75DkrYgyAcAqoiXFk559YbzW oobEHevrhqx+KjoUAc3bBmp8BQCYU4qk9cZyXEQB6zEcS+tDxbPXhbW2Hk3/zNNqLArM Chpp4+NOJLWnGxI/W3mYVyP2B3QAbHXnqZIEOf9wzuQ6sIGXI4DCmp4M910um/akp8Wc GpjOx72xGZxhuOTtU0QKLuo47hzIF6Zvx93ppHmv1DZgdQC+q6nVBzG/RmPsjV5ZQFPm qG+g== X-Gm-Message-State: AOAM531nrISYhuieZ5Wc88BsZN4K0XEXF8cqmZ81luhOpKCBr6IOyn2D dIYWR9bqvCgM0IfCVAvVbqOeH7iolxpMZwMuNPo= X-Google-Smtp-Source: ABdhPJzVQVn0tsvYwtqFACiQxMU84q/81HMUbksEnP2bMWUhlK/s5u6CCyR7X/5/5iPV5oelA1ohQbOdwuIeAaENzlQ= X-Received: by 2002:a25:31c3:: with SMTP id x186mr45865728ybx.382.1622027268758; Wed, 26 May 2021 04:07:48 -0700 (PDT) MIME-Version: 1.0 References: <1621762863-26665-1-git-send-email-apinski@marvell.com> In-Reply-To: From: Andrew Pinski Date: Wed, 26 May 2021 04:07:36 -0700 Message-ID: Subject: Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations To: Bernd Edlinger , Andrew MacLeod Cc: Richard Biener , Andrew Pinski , GCC Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, 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: Wed, 26 May 2021 11:07:53 -0000 On Wed, May 26, 2021 at 2:01 AM Andrew Pinski wrote: > > On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger > wrote: > > > > On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote: > > > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches > > > wrote: > > >> > > >> From: Andrew Pinski > > >> > > >> Instead of some of the more manual optimizations inside phi-opt, > > >> it would be good idea to do a lot of the heavy lifting inside match > > >> and simplify instead. In the process, this moves the three simple > > >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications. > > >> > > >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions. > > >> > > >> Differences from V1: > > >> * Use bit_xor 1 instead of bit_not to fix the problem with boolean t= ypes > > >> which are not 1 bit precision. > > > > > > OK. > > > > > > Thanks, > > > Richard. > > > > > > > Hmm, sorry, no luck. > > > > I think this caused: > > If anything it is a bad interaction with changes between r12-1046 and > r12-1053; I am suspecting a bug in those changes rather than my > changes causing the bug. Debugging it right now. (gdb) p debug_tree(name) unit-size align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff6b45b28 precision:1 min max > def_stmt _19 =3D ~_8; version:19> So what is happening is evrp converted: ct_12 =3D ct_5 + -1; Into ct_12 =3D ct_5 =3D=3D 1 ? 0 : 1; (this was done before my patch) And then it gets simplified to: _8 =3D ct_5 =3D=3D 1; _19 =3D ~_8; ct_12 =3D (int) _19; (after my match.pd patch) Which is correct, but the range code is not expecting new SSA names to be added ..... It looks like the issue was introduced with r12-1048 (Add imports and strengthen the export definition in range_def and gori_map). I suspect there are other match.pd patterns which would also hit this issue where a new ssa name is introduced. I have no idea how to get this fixed because gimple-range-* is new to me; I tried looking into it but calling has_def_chain/get_def_chain inside register_dependency seems wrong, maybe Andrew MacLeod can help here. This happens even with a stage 1 gcc so it is not miscompiling. Also sorry for the breakage, I was not expecting it this time around as I ran bootstrap like three times and I did not expect an interaction with other parts of the compiler like this. Thanks, Andrew Pinski > > Thanks, > Andrew > > > > > > > > home/ed/gnu/gcc-build/./gcc/xgcc -B/home/ed/gnu/gcc-build/./gcc/ -B/hom= e/ed/gnu/install/x86_64-pc-linux-gnu/bin/ -B/home/ed/gnu/install/x86_64-pc-= linux-gnu/lib/ -isystem /home/ed/gnu/install/x86_64-pc-linux-gnu/include -i= system /home/ed/gnu/install/x86_64-pc-linux-gnu/sys-include -fchecking=3D= 1 -c -g -O2 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototyp= es -Wold-style-definition -Wmissing-format-attribute -Wno-overlength-string= s -pedantic -Wno-long-long -DHAVE_CONFIG_H -I. -I../../gcc-trunk/fixinclu= des -I../include -I../../gcc-trunk/fixincludes/../include ../../gcc-trunk/f= ixincludes/fixtests.c > > during GIMPLE pass: evrp > > ../../gcc-trunk/fixincludes/fixtests.c: In function =E2=80=98run_test= =E2=80=99: > > ../../gcc-trunk/fixincludes/fixtests.c:155:1: internal compiler error: = in operator[], at vec.h:890 > > 155 | } > > | ^ > > 0x824622 vec::operator[](unsigned int) > > ../../gcc-trunk/gcc/vec.h:890 > > 0x8247f0 vec::operator[](unsig= ned int) > > ../../gcc-trunk/gcc/tree.h:3366 > > 0x8247f0 vec::operator[](unsigne= d int) > > ../../gcc-trunk/gcc/vec.h:1461 > > 0x8247f0 range_def_chain::register_dependency(tree_node*, tree_node*, b= asic_block_def*) > > ../../gcc-trunk/gcc/gimple-range-gori.cc:179 > > 0x18639bc fold_using_range::range_of_range_op(irange&, gimple*, fur_sou= rce&) > > ../../gcc-trunk/gcc/gimple-range.cc:439 > > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, tr= ee_node*) > > ../../gcc-trunk/gcc/gimple-range.cc:376 > > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_nod= e*) > > ../../gcc-trunk/gcc/gimple-range.cc:1067 > > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*) > > ../../gcc-trunk/gcc/gimple-range.cc:1097 > > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*) > > ../../gcc-trunk/gcc/gimple-range.cc:980 > > 0x18637c7 fold_using_range::range_of_range_op(irange&, gimple*, fur_sou= rce&) > > ../../gcc-trunk/gcc/gimple-range.cc:431 > > 0x1866c85 fold_using_range::fold_stmt(irange&, gimple*, fur_source&, tr= ee_node*) > > ../../gcc-trunk/gcc/gimple-range.cc:376 > > 0x1866fa2 gimple_ranger::fold_range_internal(irange&, gimple*, tree_nod= e*) > > ../../gcc-trunk/gcc/gimple-range.cc:1067 > > 0x1866fa2 gimple_ranger::range_of_stmt(irange&, gimple*, tree_node*) > > ../../gcc-trunk/gcc/gimple-range.cc:1097 > > 0x186308a gimple_ranger::range_of_expr(irange&, tree_node*, gimple*) > > ../../gcc-trunk/gcc/gimple-range.cc:980 > > 0x1149961 range_query::value_of_expr(tree_node*, gimple*) > > ../../gcc-trunk/gcc/value-query.cc:86 > > 0x1871e51 hybrid_folder::value_of_expr(tree_node*, gimple*) > > ../../gcc-trunk/gcc/gimple-ssa-evrp.c:235 > > 0xff9573 substitute_and_fold_engine::replace_uses_in(gimple*) > > ../../gcc-trunk/gcc/tree-ssa-propagate.c:575 > > 0xff988c substitute_and_fold_dom_walker::before_dom_children(basic_bloc= k_def*) > > ../../gcc-trunk/gcc/tree-ssa-propagate.c:845 > > 0x183921f dom_walker::walk(basic_block_def*) > > ../../gcc-trunk/gcc/domwalk.c:309 > > 0xff8d15 substitute_and_fold_engine::substitute_and_fold(basic_block_de= f*) > > ../../gcc-trunk/gcc/tree-ssa-propagate.c:987 > > Please submit a full bug report, > > with preprocessed source if appropriate. > > Please include the complete backtrace with any bug report. > > See for instructions. > > make[2]: *** [Makefile:76: fixtests.o] Error 1 > > make[2]: Leaving directory '/home/ed/gnu/gcc-build/fixincludes' > > make[1]: *** [Makefile:3827: all-fixincludes] Error 2 > > make[1]: Leaving directory '/home/ed/gnu/gcc-build' > > make: *** [Makefile:1011: all] Error 2 > > > > > > Bernd. > > > > > > >> Thanks, > > >> Andrew Pinski > > >> > > >> gcc: > > >> * match.pd (A?CST1:CST2): Add simplifcations for A?0:+-1, A?= +-1:0, > > >> A?POW2:0 and A?0:POW2. > > >> --- > > >> gcc/match.pd | 41 +++++++++++++++++++++++++++++++++++++++++ > > >> 1 file changed, 41 insertions(+) > > >> > > >> diff --git a/gcc/match.pd b/gcc/match.pd > > >> index 1fc6b7b1557..ad6b057c56d 100644 > > >> --- a/gcc/match.pd > > >> +++ b/gcc/match.pd > > >> @@ -3711,6 +3711,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > >> (if (integer_all_onesp (@1) && integer_zerop (@2)) > > >> @0)))) > > >> > > >> +/* A few simplifications of "a ? CST1 : CST2". */ > > >> +/* NOTE: Only do this on gimple as the if-chain-to-switch > > >> + optimization depends on the gimple to have if statements in it. = */ > > >> +#if GIMPLE > > >> +(simplify > > >> + (cond @0 INTEGER_CST@1 INTEGER_CST@2) > > >> + (switch > > >> + (if (integer_zerop (@2)) > > >> + (switch > > >> + /* a ? 1 : 0 -> a if 0 and 1 are integral types. */ > > >> + (if (integer_onep (@1)) > > >> + (convert (convert:boolean_type_node @0))) > > >> + /* a ? -1 : 0 -> -a. */ > > >> + (if (integer_all_onesp (@1)) > > >> + (negate (convert (convert:boolean_type_node @0)))) > > >> + /* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */ > > >> + (if (!POINTER_TYPE_P (type) && integer_pow2p (@1)) > > >> + (with { > > >> + tree shift =3D build_int_cst (integer_type_node, tree_log2 (= @1)); > > >> + } > > >> + (lshift (convert (convert:boolean_type_node @0)) { shift; }))= ))) > > >> + (if (integer_zerop (@1)) > > >> + (with { > > >> + tree booltrue =3D constant_boolean_node (true, boolean_type_n= ode); > > >> + } > > >> + (switch > > >> + /* a ? 0 : 1 -> !a. */ > > >> + (if (integer_onep (@2)) > > >> + (convert (bit_xor (convert:boolean_type_node @0) { booltrue; = } ))) > > >> + /* a ? -1 : 0 -> -(!a). */ > > >> + (if (integer_all_onesp (@2)) > > >> + (negate (convert (bit_xor (convert:boolean_type_node @0) { bo= oltrue; } )))) > > >> + /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */ > > >> + (if (!POINTER_TYPE_P (type) && integer_pow2p (@2)) > > >> + (with { > > >> + tree shift =3D build_int_cst (integer_type_node, tree_log2 (= @2)); > > >> + } > > >> + (lshift (convert (bit_xor (convert:boolean_type_node @0) { b= ooltrue; } )) > > >> + { shift; })))))))) > > >> +#endif > > >> + > > >> /* Simplification moved from fold_cond_expr_with_comparison. It ma= y also > > >> be extended. */ > > >> /* This pattern implements two kinds simplification: > > >> -- > > >> 2.17.1 > > >> > > >