From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 02E5338582B7 for ; Fri, 31 Mar 2023 11:30:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 02E5338582B7 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680262249; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=waXkthwswFVC//r5xDhBzMNQf0NI5awEuLy4TWqDzMQ=; b=ONZAWQY+2FcJwGuG4sbZMJXjCfEiAdhOjAntkesg3Ulf3g1IWlKLHdykRNHGviJY36lZl1 aV0Yu+xWDPvepRwiiVSyW1tSKJzrgwSAln2NyisIxSzwqIJRACDhf+CIhOolWfOObmhQTb hwICkyd0GPlWeSY7CRnIn1s86YY3i2A= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-538-KbLnsenkNQW6U8fB3JRkYw-1; Fri, 31 Mar 2023 07:30:48 -0400 X-MC-Unique: KbLnsenkNQW6U8fB3JRkYw-1 Received: by mail-wr1-f70.google.com with SMTP id o13-20020adfa10d000000b002d34203df59so2402747wro.9 for ; Fri, 31 Mar 2023 04:30:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680262247; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=5iTlqMqYsrl20wsGGpbqU/0WEc+1MWj7zsB3lErq7q4=; b=EC/QOwAU5sqB5hJIJ94W1QPJ71fVlFWOT9Opb1SkAWjwTjq6fyBt6llJXj5Ub24+MF VpTQW2BsCZY+2aPbVgfrvSPXXkg1yX3l96xs44g+pwMY6gydptjsqeC7vTmRJ6ZomnfA dED4Mr+D04zPZDTWAP+oc36y/egY5h/7tvkO/D8rf1JUvBHpJX+I7ylNy0UhpBlBgAlh dQMSVvXthJjirATssA2xGHOHebPIt+IMlqeiZrBW+c5c4z3ejuZIS4jv6lu+hamUq1lg kkVD7RJTt8bTLMaznr9H3anEJoOnOhcp2A05VPSyWmQ1a7DTz19df9/nLymfVqM6VfYS rNsA== X-Gm-Message-State: AO0yUKVqOEaMhrUU5KYfmustDoE3KuQoQkqEjIe0Tm/zr0Yis1i3B9cb sppvPDyr1l8lW+zYk+eXpJKDfMns/PQIxIOiUVAnxX6sYqFgmkaPCYvxfEXiVY9cqoPA1i4zPOn 7Fzo2KREEkluOBkL7Lw== X-Received: by 2002:a7b:c44a:0:b0:3ef:8b0:dbb1 with SMTP id l10-20020a7bc44a000000b003ef08b0dbb1mr19828789wmi.7.1680262247407; Fri, 31 Mar 2023 04:30:47 -0700 (PDT) X-Google-Smtp-Source: AK7set+0c3RNYfVsn1rNAwL9F01AGEyxViakKYj6OytYmM/BE3vhAmH2BRNpv3ppM7St4pd6nSSEag== X-Received: by 2002:a7b:c44a:0:b0:3ef:8b0:dbb1 with SMTP id l10-20020a7bc44a000000b003ef08b0dbb1mr19828775wmi.7.1680262246982; Fri, 31 Mar 2023 04:30:46 -0700 (PDT) Received: from [192.168.1.201] ([139.47.42.170]) by smtp.gmail.com with ESMTPSA id s15-20020a05600c45cf00b003eb2e33f327sm17152692wmo.2.2023.03.31.04.30.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 31 Mar 2023 04:30:46 -0700 (PDT) Message-ID: <86eb9c46-339f-c170-11e9-82e4b2729728@redhat.com> Date: Fri, 31 Mar 2023 13:30:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [RFC PATCH] Use ranger in the cdce pass [PR91645] To: Jakub Jelinek , Andrew MacLeod Cc: gcc-patches@gcc.gnu.org References: From: Aldy Hernandez In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00,DKIM_INVALID,DKIM_SIGNED,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_SHORT,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,WEIRD_PORT autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 3/31/23 10:16, Jakub Jelinek wrote: > Hi! > > The cdce pass among other things replaces calls like sqrt with code > like > if (condition(s)) > ret = .IFN_SQRT (x); > else > ret = sqrt (x); > so that in the common case when we know the argument doesn't trigger > any range/domain errors we use the hardware instruction and defer to > a library call just to handle the erroneous conditions. > Now, on pr103559-3.c we already fold that condition during vrp2 or > so, but for pr103559-{2,4}.c we don't. > The following patch is an attempt to ask the ranger already during the > cdce pass. > Unfortunately, bootstrap/regtest of this patch found one regression: > FAIL: 26_numerics/headers/cmath/functions_std_c++23.cc (test for excess errors) > Excess errors: > during GIMPLE pass: cdce > /home/jakub/src/gcc/libstdc++-v3/testsuite/26_numerics/headers/cmath/functions_std_c++23.cc:26: internal compiler error: in operator[], at vec.h:890 > 0x94b822 vec::operator[](unsigned int) > ../../gcc/vec.h:890 > 0x94ba2a vec::operator[](unsigned int) > ../../gcc/value-relation.cc:705 > 0x94ba2a vec::operator[](unsigned int) > ../../gcc/vec.h:1505 > 0x94ba2a equiv_oracle::add_equiv_to_block(basic_block_def*, bitmap_head*) > ../../gcc/value-relation.cc:690 > The problem is that if the pass asks ranger about something, then splits > some basic blocks to add the above if (condition(s)) stuff and then > asks the ranger again, the ranger caches stuff based on basic blocks > but the pass adds more basic blocks. > So, if I wanted to use ranger in the pass, is it true I'd need to > do all the ranger queries in some analysis phase before changes are > made to the cfg? Most likely, but Andrew had some fixes to make sure we didn't tooo bad when simple changes to the IL happened. Perhaps he has some thoughts here. Aldy > > Now, I've just tried and apparently already the patch I've just posted > about foperator_un*::fold_range seems to fix those testcases (during vrp2 > or when), so maybe we don't need to bother with cdce here and can just > commit the testcases. Are they ok for trunk as incremental change to > the previous patch? > > 2023-03-31 Jakub Jelinek > > PR tree-optimization/91645 > * tree-call-cdce.cc: Include gimple-range.h. > (gen_one_condition): Add STMT argument. Ask ranger if the > comparison is known to be always false or always true and use > false or true instead of the actual comparison in that case. > (gen_conditions_for_domain, gen_conditions_for_pow_cst_base, > gen_conditions_for_pow_int_base): Add STMT argument, pass it > through. > (gen_conditions_for_pow): Pass pow_call as STMT to > gen_conditions_for_pow_cst_base and gen_conditions_for_pow_int_base. > (gen_shrink_wrap_conditions): Pass bi_call as STMT to > gen_conditions_for_domain. > (pass_call_cdce::execute): Disable ranger if it has been enabled. > > * gcc.target/i386/pr103559-1.c: New test. > * gcc.target/i386/pr103559-2.c: New test. > * gcc.target/i386/pr103559-3.c: New test. > * gcc.target/i386/pr103559-4.c: New test. > > --- gcc/tree-call-cdce.cc.jj 2023-01-02 09:32:45.940944935 +0100 > +++ gcc/tree-call-cdce.cc 2023-03-30 14:54:25.248544702 +0200 > @@ -36,6 +36,7 @@ along with GCC; see the file COPYING3. > #include "builtins.h" > #include "internal-fn.h" > #include "tree-dfa.h" > +#include "gimple-range.h" > > > /* This pass serves two closely-related purposes: > @@ -425,12 +426,9 @@ comparison_code_if_no_nans (tree_code co > null tree. */ > > static void > -gen_one_condition (tree arg, int lbub, > - enum tree_code tcode, > - const char *temp_name1, > - const char *temp_name2, > - vec conds, > - unsigned *nconds) > +gen_one_condition (tree arg, int lbub, enum tree_code tcode, > + const char *temp_name1, const char *temp_name2, > + vec conds, unsigned *nconds, gimple *stmt) > { > if (!HONOR_NANS (arg)) > tcode = comparison_code_if_no_nans (tcode); > @@ -451,10 +449,24 @@ gen_one_condition (tree arg, int lbub, > gimple_assign_set_lhs (stmt1, tempn); > > tempc = create_tmp_var (boolean_type_node, temp_name2); > - stmt2 = gimple_build_assign (tempc, > - fold_build2 (tcode, > - boolean_type_node, > - tempn, lbub_real_cst)); > + tree tcond = build2 (tcode, boolean_type_node, arg, lbub_real_cst); > + int_range_max r; > + range_query *q = get_range_query (cfun); > + if (q == get_global_range_query ()) > + q = enable_ranger (cfun); > + /* Ask the ranger whether it knows the condition will be always false or > + always true. */ > + if (!q->range_of_expr (r, tcond, stmt) || r.undefined_p ()) > + tcond = NULL_TREE; > + else if (r.upper_bound () == 0) > + tcond = boolean_false_node; > + else if (r.lower_bound () == 1) > + tcond = boolean_true_node; > + else > + tcond = NULL_TREE; > + if (!tcond) > + tcond = fold_build2 (tcode, boolean_type_node, tempn, lbub_real_cst); > + stmt2 = gimple_build_assign (tempc, tcond); > tempcn = make_ssa_name (tempc, stmt2); > gimple_assign_set_lhs (stmt2, tempcn); > > @@ -475,16 +487,15 @@ gen_one_condition (tree arg, int lbub, > for lower bound check, one for upper bound check. */ > > static void > -gen_conditions_for_domain (tree arg, inp_domain domain, > - vec conds, > - unsigned *nconds) > +gen_conditions_for_domain (tree arg, inp_domain domain, vec conds, > + unsigned *nconds, gimple *stmt) > { > if (domain.has_lb) > gen_one_condition (arg, domain.lb, > (domain.is_lb_inclusive > ? UNGE_EXPR : UNGT_EXPR), > "DCE_COND_LB", "DCE_COND_LB_TEST", > - conds, nconds); > + conds, nconds, stmt); > > if (domain.has_ub) > { > @@ -496,7 +507,7 @@ gen_conditions_for_domain (tree arg, inp > (domain.is_ub_inclusive > ? UNLE_EXPR : UNLT_EXPR), > "DCE_COND_UB", "DCE_COND_UB_TEST", > - conds, nconds); > + conds, nconds, stmt); > } > } > > @@ -518,9 +529,8 @@ gen_conditions_for_domain (tree arg, inp > and *NCONDS is the number of logical conditions. */ > > static void > -gen_conditions_for_pow_cst_base (tree base, tree expn, > - vec conds, > - unsigned *nconds) > +gen_conditions_for_pow_cst_base (tree base, tree expn, vec conds, > + unsigned *nconds, gimple *stmt) > { > inp_domain exp_domain; > /* Validate the range of the base constant to make > @@ -532,11 +542,9 @@ gen_conditions_for_pow_cst_base (tree ba > real_from_integer (&mv, TYPE_MODE (TREE_TYPE (base)), 256, UNSIGNED); > gcc_assert (!real_less (&mv, &bcv)); > > - exp_domain = get_domain (0, false, false, > - 127, true, false); > + exp_domain = get_domain (0, false, false, 127, true, false); > > - gen_conditions_for_domain (expn, exp_domain, > - conds, nconds); > + gen_conditions_for_domain (expn, exp_domain, conds, nconds, stmt); > } > > /* Generate error condition code for pow calls with > @@ -554,9 +562,8 @@ gen_conditions_for_pow_cst_base (tree ba > conditions. */ > > static void > -gen_conditions_for_pow_int_base (tree base, tree expn, > - vec conds, > - unsigned *nconds) > +gen_conditions_for_pow_int_base (tree base, tree expn, vec conds, > + unsigned *nconds, gimple *stmt) > { > gimple *base_def; > tree base_val0; > @@ -600,11 +607,9 @@ gen_conditions_for_pow_int_base (tree ba > /* Generate condition in reverse order -- first > the condition for the exp argument. */ > > - exp_domain = get_domain (0, false, false, > - max_exp, true, true); > + exp_domain = get_domain (0, false, false, max_exp, true, true); > > - gen_conditions_for_domain (expn, exp_domain, > - conds, nconds); > + gen_conditions_for_domain (expn, exp_domain, conds, nconds, stmt); > > /* Now generate condition for the base argument. > Note it does not use the helper function > @@ -660,9 +665,9 @@ gen_conditions_for_pow (gcall *pow_call, > bc = TREE_CODE (base); > > if (bc == REAL_CST) > - gen_conditions_for_pow_cst_base (base, expn, conds, nconds); > + gen_conditions_for_pow_cst_base (base, expn, conds, nconds, pow_call); > else if (bc == SSA_NAME) > - gen_conditions_for_pow_int_base (base, expn, conds, nconds); > + gen_conditions_for_pow_int_base (base, expn, conds, nconds, pow_call); > else > gcc_unreachable (); > } > @@ -852,7 +857,7 @@ gen_shrink_wrap_conditions (gcall *bi_ca > inp_domain domain = get_no_error_domain (fnc); > *nconds = 0; > arg = gimple_call_arg (bi_call, 0); > - gen_conditions_for_domain (arg, domain, conds, nconds); > + gen_conditions_for_domain (arg, domain, conds, nconds, bi_call); > } > > return; > @@ -1290,6 +1295,8 @@ pass_call_cdce::execute (function *fun) > return 0; > > shrink_wrap_conditional_dead_built_in_calls (cond_dead_built_in_calls); > + if (get_range_query (fun) != get_global_range_query ()) > + disable_ranger (fun); > free_dominance_info (CDI_POST_DOMINATORS); > /* As we introduced new control-flow we need to insert PHI-nodes > for the call-clobbers of the remaining call. */ > --- gcc/testsuite/gcc.target/i386/pr103559-1.c.jj 2023-03-30 16:28:30.040980530 +0200 > +++ gcc/testsuite/gcc.target/i386/pr103559-1.c 2023-03-30 16:29:33.866047038 +0200 > @@ -0,0 +1,13 @@ > +/* PR tree-optimization/103559 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > +/* { dg-final { scan-tree-dump " = .SQRT \\\(" "optimized" } } */ > +/* { dg-final { scan-tree-dump " = sqrtf \\\(" "optimized" } } */ > + > +float sqrtf (float); > + > +float > +foo (float x) > +{ > + return sqrtf (x); > +} > --- gcc/testsuite/gcc.target/i386/pr103559-2.c.jj 2023-03-30 16:28:33.211934155 +0200 > +++ gcc/testsuite/gcc.target/i386/pr103559-2.c 2023-03-30 16:29:44.855886307 +0200 > @@ -0,0 +1,15 @@ > +/* PR tree-optimization/103559 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > +/* { dg-final { scan-tree-dump " = .SQRT \\\(" "optimized" } } */ > +/* { dg-final { scan-tree-dump-not " = sqrtf \\\(" "optimized" } } */ > + > +float sqrtf (float); > + > +float > +foo (float x) > +{ > + if (__builtin_isless (x, 0)) > + __builtin_unreachable (); > + return sqrtf (x); > +} > --- gcc/testsuite/gcc.target/i386/pr103559-3.c.jj 2023-03-30 16:28:36.318888712 +0200 > +++ gcc/testsuite/gcc.target/i386/pr103559-3.c 2023-03-30 16:29:51.977782145 +0200 > @@ -0,0 +1,15 @@ > +/* PR tree-optimization/103559 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > +/* { dg-final { scan-tree-dump-not " = .SQRT \\\(" "optimized" } } */ > +/* { dg-final { scan-tree-dump " = sqrtf \\\(" "optimized" } } */ > + > +float sqrtf (float); > + > +float > +foo (float x) > +{ > + if (!__builtin_isless (x, 0)) > + __builtin_unreachable (); > + return sqrtf (x); > +} > --- gcc/testsuite/gcc.target/i386/pr103559-4.c.jj 2023-03-30 16:28:39.710839098 +0200 > +++ gcc/testsuite/gcc.target/i386/pr103559-4.c 2023-03-30 16:29:57.564700431 +0200 > @@ -0,0 +1,13 @@ > +/* PR tree-optimization/103559 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > +/* { dg-final { scan-tree-dump-not " = sqrtf \\\(" "optimized" } } */ > + > +float sqrtf (float); > + > +float > +foo (float x) > +{ > + x = x * x; > + return sqrtf (x); > +} > > Jakub >