public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Aldy Hernandez <aldyh@redhat.com>, Andrew MacLeod <amacleod@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [RFC PATCH] Use ranger in the cdce pass [PR91645]
Date: Fri, 31 Mar 2023 10:16:59 +0200	[thread overview]
Message-ID: <ZCaW+lxMXIASrQJz@tucnak> (raw)

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<tree_node*, va_gc, vl_embed>::operator[](unsigned int)
        ../../gcc/vec.h:890
0x94ba2a vec<equiv_chain*, va_heap, vl_embed>::operator[](unsigned int)
        ../../gcc/value-relation.cc:705
0x94ba2a vec<equiv_chain*, va_heap, vl_ptr>::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?

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  <jakub@redhat.com>

	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"
 \f
 
 /* 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<gimple *> conds,
-                   unsigned *nconds)
+gen_one_condition (tree arg, int lbub, enum tree_code tcode,
+		   const char *temp_name1, const char *temp_name2,
+		   vec<gimple *> 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<gimple *> conds,
-                           unsigned *nconds)
+gen_conditions_for_domain (tree arg, inp_domain domain, vec<gimple *> 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<gimple *> conds,
-                                 unsigned *nconds)
+gen_conditions_for_pow_cst_base (tree base, tree expn, vec<gimple *> 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<gimple *> conds,
-                                 unsigned *nconds)
+gen_conditions_for_pow_int_base (tree base, tree expn, vec<gimple *> 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


             reply	other threads:[~2023-03-31 10:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-31  8:16 Jakub Jelinek [this message]
2023-03-31 11:30 ` Aldy Hernandez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZCaW+lxMXIASrQJz@tucnak \
    --to=jakub@redhat.com \
    --cc=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).