public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] Remove shadowed oracle field.
@ 2021-10-01 12:43 Aldy Hernandez
  2021-10-01 12:43 ` [PATCH] Pass relations down to range_operator::op[12]_range Aldy Hernandez
  2021-10-01 12:43 ` [PATCH] Handle EQ_EXPR relation for operator_lshift Aldy Hernandez
  0 siblings, 2 replies; 7+ messages in thread
From: Aldy Hernandez @ 2021-10-01 12:43 UTC (permalink / raw)
  To: GCC patches

The m_oracle field in the path solver was shadowing the base class.
This was causing subtle problems while calculating outgoing edges
between blocks, because the query object being passed did not have an
oracle set.

This should further improve our solving ability.

Tested on x86-64 Linux.

gcc/ChangeLog:

	* gimple-range-path.cc (path_range_query::compute_ranges): Use
	get_path_oracle.
	* gimple-range-path.h (class path_range_query): Remove shadowed
	m_oracle field.
	(path_range_query::get_path_oracle): New.
---
 gcc/gimple-range-path.cc | 2 +-
 gcc/gimple-range-path.h  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
index a29d5318ca9..422abfddb8f 100644
--- a/gcc/gimple-range-path.cc
+++ b/gcc/gimple-range-path.cc
@@ -480,7 +480,7 @@ path_range_query::compute_ranges (const vec<basic_block> &path,
   if (m_resolve)
     {
       add_copies_to_imports ();
-      m_oracle->reset_path ();
+      get_path_oracle ()->reset_path ();
       compute_relations (path);
     }
 
diff --git a/gcc/gimple-range-path.h b/gcc/gimple-range-path.h
index cf49c6dc086..5f4e73a5949 100644
--- a/gcc/gimple-range-path.h
+++ b/gcc/gimple-range-path.h
@@ -38,7 +38,6 @@ public:
   bool range_of_expr (irange &r, tree name, gimple * = NULL) override;
   bool range_of_stmt (irange &r, gimple *, tree name = NULL) override;
   bool unreachable_path_p ();
-  path_oracle *oracle () { return m_oracle; }
   void dump (FILE *) override;
   void debug ();
 
@@ -46,6 +45,7 @@ private:
   bool internal_range_of_expr (irange &r, tree name, gimple *);
   bool defined_outside_path (tree name);
   void range_on_path_entry (irange &r, tree name);
+  path_oracle *get_path_oracle () { return (path_oracle *)m_oracle; }
 
   // Cache manipulation.
   void set_cache (const irange &r, tree name);
@@ -85,7 +85,6 @@ private:
   auto_bitmap m_imports;
   gimple_ranger &m_ranger;
   non_null_ref m_non_null;
-  path_oracle *m_oracle;
 
   // Current path position.
   unsigned m_pos;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] Pass relations down to range_operator::op[12]_range.
  2021-10-01 12:43 [COMMITTED] Remove shadowed oracle field Aldy Hernandez
@ 2021-10-01 12:43 ` Aldy Hernandez
  2021-10-01 13:38   ` Andrew MacLeod
  2021-10-01 12:43 ` [PATCH] Handle EQ_EXPR relation for operator_lshift Aldy Hernandez
  1 sibling, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-10-01 12:43 UTC (permalink / raw)
  To: GCC patches

It looks like we don't pass relations down to the op[12]_range
operators.  This is causing problems when implementing some relational
magic for the shift operators.

Andrew, this looks like an oversight.  If so, how does this look?

Tested on x86-64 Linux.

gcc/ChangeLog:

	* gimple-range-gori.cc (gimple_range_calc_op1): Add relation argument.
	(gimple_range_calc_op2): Same.
	(gori_compute::compute_operand1_range): Pass relation to
	gimple_range_calc_op*.
	(gori_compute::compute_operand2_range): Same.
---
 gcc/gimple-range-gori.cc | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index 4a1ade7f921..c7cfb71d849 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -59,7 +59,8 @@ gimple_range_calc_op1 (irange &r, const gimple *stmt, const irange &lhs_range)
 
 bool
 gimple_range_calc_op1 (irange &r, const gimple *stmt,
-		       const irange &lhs_range, const irange &op2_range)
+		       const irange &lhs_range, const irange &op2_range,
+		       relation_kind rel)
 {
   // Unary operation are allowed to pass a range in for second operand
   // as there are often additional restrictions beyond the type which
@@ -72,7 +73,7 @@ gimple_range_calc_op1 (irange &r, const gimple *stmt,
       return true;
     }
   return gimple_range_handler (stmt)->op1_range (r, type, lhs_range,
-						 op2_range);
+						 op2_range, rel);
 }
 
 // Calculate what we can determine of the range of this statement's
@@ -82,7 +83,8 @@ gimple_range_calc_op1 (irange &r, const gimple *stmt,
 
 bool
 gimple_range_calc_op2 (irange &r, const gimple *stmt,
-		       const irange &lhs_range, const irange &op1_range)
+		       const irange &lhs_range, const irange &op1_range,
+		       relation_kind rel)
 {
   tree type = TREE_TYPE (gimple_range_operand2 (stmt));
   // An empty range is viral.
@@ -92,7 +94,7 @@ gimple_range_calc_op2 (irange &r, const gimple *stmt,
       return true;
     }
   return gimple_range_handler (stmt)->op2_range (r, type, lhs_range,
-						 op1_range);
+						 op1_range, rel);
 }
 
 // Return TRUE if GS is a logical && or || expression.
@@ -1000,6 +1002,12 @@ gori_compute::compute_operand1_range (irange &r, gimple *stmt,
   int_range_max op1_range, op2_range;
   tree op1 = gimple_range_operand1 (stmt);
   tree op2 = gimple_range_operand2 (stmt);
+  relation_kind rel;
+
+  if (op1 && op2)
+    rel = src.query_relation (op1, op2);
+  else
+    rel = VREL_NONE;
 
   // Fetch the known range for op1 in this block.
   src.get_operand (op1_range, op1);
@@ -1008,7 +1016,7 @@ gori_compute::compute_operand1_range (irange &r, gimple *stmt,
   if (op2)
     {
       src.get_operand (op2_range, op2);
-      if (!gimple_range_calc_op1 (r, stmt, lhs, op2_range))
+      if (!gimple_range_calc_op1 (r, stmt, lhs, op2_range, rel))
 	return false;
     }
   else
@@ -1016,7 +1024,7 @@ gori_compute::compute_operand1_range (irange &r, gimple *stmt,
       // We pass op1_range to the unary operation.  Nomally it's a
       // hidden range_for_type parameter, but sometimes having the
       // actual range can result in better information.
-      if (!gimple_range_calc_op1 (r, stmt, lhs, op1_range))
+      if (!gimple_range_calc_op1 (r, stmt, lhs, op1_range, rel))
 	return false;
     }
 
@@ -1077,12 +1085,18 @@ gori_compute::compute_operand2_range (irange &r, gimple *stmt,
   int_range_max op1_range, op2_range;
   tree op1 = gimple_range_operand1 (stmt);
   tree op2 = gimple_range_operand2 (stmt);
+  relation_kind rel;
+
+  if (op1 && op2)
+    rel = src.query_relation (op1, op2);
+  else
+    rel = VREL_NONE;
 
   src.get_operand (op1_range, op1);
   src.get_operand (op2_range, op2);
 
   // Intersect with range for op2 based on lhs and op1.
-  if (!gimple_range_calc_op2 (r, stmt, lhs, op1_range))
+  if (!gimple_range_calc_op2 (r, stmt, lhs, op1_range, rel))
     return false;
 
   unsigned idx;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] Handle EQ_EXPR relation for operator_lshift.
  2021-10-01 12:43 [COMMITTED] Remove shadowed oracle field Aldy Hernandez
  2021-10-01 12:43 ` [PATCH] Pass relations down to range_operator::op[12]_range Aldy Hernandez
@ 2021-10-01 12:43 ` Aldy Hernandez
  2021-10-01 14:52   ` Aldy Hernandez
  1 sibling, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-10-01 12:43 UTC (permalink / raw)
  To: GCC patches

Knowing that X << X is non-zero means X is also non-zero.  This patch
teaches this this to range-ops.

As usual, the big twiddling experts could come up with all sorts of
fancy enhancements in this area, and we welcome all patches :).

I will push this pending tests.

gcc/ChangeLog:

	PR tree-optimization/102546
	* range-op.cc (operator_lshift::op1_range): Handle EQ_EXPR
	relation.
---
 gcc/range-op.cc                          | 19 ++++++++++++++++---
 gcc/testsuite/gcc.dg/tree-ssa/pr102546.c | 23 +++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102546.c

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index 5e37133026d..53f3be4266e 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -2075,9 +2075,14 @@ operator_lshift::op1_range (irange &r,
 			    tree type,
 			    const irange &lhs,
 			    const irange &op2,
-			    relation_kind rel ATTRIBUTE_UNUSED) const
+			    relation_kind rel) const
 {
   tree shift_amount;
+  int_range<2> adjust (type);
+
+  if (rel == EQ_EXPR && !lhs.contains_p (build_zero_cst (type)))
+    adjust.set_nonzero (type);
+
   if (op2.singleton_p (&shift_amount))
     {
       wide_int shift = wi::to_wide (shift_amount);
@@ -2086,10 +2091,11 @@ operator_lshift::op1_range (irange &r,
       if (wi::ge_p (shift, wi::uhwi (TYPE_PRECISION (type),
 				     TYPE_PRECISION (op2.type ())),
 		    UNSIGNED))
-	return false;
+	goto done;
       if (shift == 0)
 	{
 	  r = lhs;
+	  r.intersect (adjust);
 	  return true;
 	}
 
@@ -2126,9 +2132,16 @@ operator_lshift::op1_range (irange &r,
 
       if (utype != type)
 	range_cast (r, type);
+      r.intersect (adjust);
       return true;
     }
-  return false;
+
+ done:
+  if (adjust.varying_p ())
+    return false;
+
+  r = adjust;
+  return true;
 }
 
 bool
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
new file mode 100644
index 00000000000..4bd98747732
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-O3 -fdump-tree-optimized" }
+
+static int a;
+static char b, c, d;
+void bar(void);
+void foo(void);
+
+int main() {
+    int f = 0;
+    for (; f <= 5; f++) {
+        bar();
+        b = b && f;
+        d = f << f;
+        if (!(a >= d || f))
+            foo();
+        c = 1;
+        for (; c; c = 0)
+            ;
+    }
+}
+
+// { dg-final { scan-tree-dump-not "foo" "optimized" } }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Pass relations down to range_operator::op[12]_range.
  2021-10-01 12:43 ` [PATCH] Pass relations down to range_operator::op[12]_range Aldy Hernandez
@ 2021-10-01 13:38   ` Andrew MacLeod
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew MacLeod @ 2021-10-01 13:38 UTC (permalink / raw)
  To: Aldy Hernandez, GCC patches

On 10/1/21 8:43 AM, Aldy Hernandez wrote:
> It looks like we don't pass relations down to the op[12]_range
> operators.  This is causing problems when implementing some relational
> magic for the shift operators.
>
> Andrew, this looks like an oversight.  If so, how does this look?


Hrm.  It's at least partial.  It will utilize relations as they exist at 
the query point, but they would not include any relations introduced by 
the unwind sequence.

at the If, there is no relation c_1 < a_2.  Its true we are checking the 
true edge, and in theory the relation should be registered on that true 
edge..  Perhaps I need to register the relation on the edge from the 
stmt before resolving the stmt rather than after like we currently do.

Let me think about that for a bit.

Andrew


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Handle EQ_EXPR relation for operator_lshift.
  2021-10-01 12:43 ` [PATCH] Handle EQ_EXPR relation for operator_lshift Aldy Hernandez
@ 2021-10-01 14:52   ` Aldy Hernandez
  2021-10-02 19:50     ` Aldy Hernandez
  0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-10-01 14:52 UTC (permalink / raw)
  To: GCC patches

[-- Attachment #1: Type: text/plain, Size: 3297 bytes --]

Well, after talking with Andrew it seems that X << Y being non-zero
also implies X is non-zero.  So we don't even need relationals here.

So, I leave gori relationals in his capable hands, while I test this
much simpler patch which fixes the PR with no additional
infrastructure ;-).

Will push pending tests.
Aldy

On Fri, Oct 1, 2021 at 2:43 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Knowing that X << X is non-zero means X is also non-zero.  This patch
> teaches this this to range-ops.
>
> As usual, the big twiddling experts could come up with all sorts of
> fancy enhancements in this area, and we welcome all patches :).
>
> I will push this pending tests.
>
> gcc/ChangeLog:
>
>         PR tree-optimization/102546
>         * range-op.cc (operator_lshift::op1_range): Handle EQ_EXPR
>         relation.
> ---
>  gcc/range-op.cc                          | 19 ++++++++++++++++---
>  gcc/testsuite/gcc.dg/tree-ssa/pr102546.c | 23 +++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
>
> diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> index 5e37133026d..53f3be4266e 100644
> --- a/gcc/range-op.cc
> +++ b/gcc/range-op.cc
> @@ -2075,9 +2075,14 @@ operator_lshift::op1_range (irange &r,
>                             tree type,
>                             const irange &lhs,
>                             const irange &op2,
> -                           relation_kind rel ATTRIBUTE_UNUSED) const
> +                           relation_kind rel) const
>  {
>    tree shift_amount;
> +  int_range<2> adjust (type);
> +
> +  if (rel == EQ_EXPR && !lhs.contains_p (build_zero_cst (type)))
> +    adjust.set_nonzero (type);
> +
>    if (op2.singleton_p (&shift_amount))
>      {
>        wide_int shift = wi::to_wide (shift_amount);
> @@ -2086,10 +2091,11 @@ operator_lshift::op1_range (irange &r,
>        if (wi::ge_p (shift, wi::uhwi (TYPE_PRECISION (type),
>                                      TYPE_PRECISION (op2.type ())),
>                     UNSIGNED))
> -       return false;
> +       goto done;
>        if (shift == 0)
>         {
>           r = lhs;
> +         r.intersect (adjust);
>           return true;
>         }
>
> @@ -2126,9 +2132,16 @@ operator_lshift::op1_range (irange &r,
>
>        if (utype != type)
>         range_cast (r, type);
> +      r.intersect (adjust);
>        return true;
>      }
> -  return false;
> +
> + done:
> +  if (adjust.varying_p ())
> +    return false;
> +
> +  r = adjust;
> +  return true;
>  }
>
>  bool
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
> new file mode 100644
> index 00000000000..4bd98747732
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
> @@ -0,0 +1,23 @@
> +// { dg-do compile }
> +// { dg-options "-O3 -fdump-tree-optimized" }
> +
> +static int a;
> +static char b, c, d;
> +void bar(void);
> +void foo(void);
> +
> +int main() {
> +    int f = 0;
> +    for (; f <= 5; f++) {
> +        bar();
> +        b = b && f;
> +        d = f << f;
> +        if (!(a >= d || f))
> +            foo();
> +        c = 1;
> +        for (; c; c = 0)
> +            ;
> +    }
> +}
> +
> +// { dg-final { scan-tree-dump-not "foo" "optimized" } }
> --
> 2.31.1
>

[-- Attachment #2: 0001-PR102546-X-Y-being-non-zero-implies-X-is-also-non-ze.patch --]
[-- Type: text/x-patch, Size: 2807 bytes --]

From fa11285b9ff1d75c877369c1df7760c3f76a4fe5 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Fri, 1 Oct 2021 13:05:36 +0200
Subject: [PATCH] [PR102546] X << Y being non-zero implies X is also non-zero.

This patch teaches this to range-ops.

Tested on x86-64 Linux.

gcc/ChangeLog:

	PR tree-optimization/102546
	* range-op.cc (operator_lshift::op1_range): Teach range-ops that
	X << Y is non-zero implies X is also non-zero.
---
 gcc/range-op.cc                          | 18 ++++++++++++++----
 gcc/testsuite/gcc.dg/tree-ssa/pr102546.c | 23 +++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102546.c

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index 5e37133026d..2baca4a197f 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -2078,6 +2078,12 @@ operator_lshift::op1_range (irange &r,
 			    relation_kind rel ATTRIBUTE_UNUSED) const
 {
   tree shift_amount;
+
+  if (!lhs.contains_p (build_zero_cst (type)))
+    r.set_nonzero (type);
+  else
+    r.set_varying (type);
+
   if (op2.singleton_p (&shift_amount))
     {
       wide_int shift = wi::to_wide (shift_amount);
@@ -2089,21 +2095,24 @@ operator_lshift::op1_range (irange &r,
 	return false;
       if (shift == 0)
 	{
-	  r = lhs;
+	  r.intersect (lhs);
 	  return true;
 	}
 
       // Work completely in unsigned mode to start.
       tree utype = type;
+      int_range_max tmp_range;
       if (TYPE_SIGN (type) == SIGNED)
 	{
 	  int_range_max tmp = lhs;
 	  utype = unsigned_type_for (type);
 	  range_cast (tmp, utype);
-	  op_rshift.fold_range (r, utype, tmp, op2);
+	  op_rshift.fold_range (tmp_range, utype, tmp, op2);
 	}
       else
-	op_rshift.fold_range (r, utype, lhs, op2);
+	op_rshift.fold_range (tmp_range, utype, lhs, op2);
+
+      r.intersect (tmp_range);
 
       // Start with ranges which can produce the LHS by right shifting the
       // result by the shift amount.
@@ -2128,7 +2137,8 @@ operator_lshift::op1_range (irange &r,
 	range_cast (r, type);
       return true;
     }
-  return false;
+
+  return !r.varying_p ();
 }
 
 bool
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
new file mode 100644
index 00000000000..4bd98747732
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-O3 -fdump-tree-optimized" }
+
+static int a;
+static char b, c, d;
+void bar(void);
+void foo(void);
+
+int main() {
+    int f = 0;
+    for (; f <= 5; f++) {
+        bar();
+        b = b && f;
+        d = f << f;
+        if (!(a >= d || f))
+            foo();
+        c = 1;
+        for (; c; c = 0)
+            ;
+    }
+}
+
+// { dg-final { scan-tree-dump-not "foo" "optimized" } }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Handle EQ_EXPR relation for operator_lshift.
  2021-10-01 14:52   ` Aldy Hernandez
@ 2021-10-02 19:50     ` Aldy Hernandez
  2021-10-02 20:53       ` Jeff Law
  0 siblings, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2021-10-02 19:50 UTC (permalink / raw)
  To: GCC patches

[-- Attachment #1: Type: text/plain, Size: 3701 bytes --]

Bah.  The range was being clobbered half way through the calculation.

Tested on x86-64 Linux.

Pushed.

On Fri, Oct 1, 2021 at 4:52 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Well, after talking with Andrew it seems that X << Y being non-zero
> also implies X is non-zero.  So we don't even need relationals here.
>
> So, I leave gori relationals in his capable hands, while I test this
> much simpler patch which fixes the PR with no additional
> infrastructure ;-).
>
> Will push pending tests.
> Aldy
>
> On Fri, Oct 1, 2021 at 2:43 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > Knowing that X << X is non-zero means X is also non-zero.  This patch
> > teaches this this to range-ops.
> >
> > As usual, the big twiddling experts could come up with all sorts of
> > fancy enhancements in this area, and we welcome all patches :).
> >
> > I will push this pending tests.
> >
> > gcc/ChangeLog:
> >
> >         PR tree-optimization/102546
> >         * range-op.cc (operator_lshift::op1_range): Handle EQ_EXPR
> >         relation.
> > ---
> >  gcc/range-op.cc                          | 19 ++++++++++++++++---
> >  gcc/testsuite/gcc.dg/tree-ssa/pr102546.c | 23 +++++++++++++++++++++++
> >  2 files changed, 39 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
> >
> > diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> > index 5e37133026d..53f3be4266e 100644
> > --- a/gcc/range-op.cc
> > +++ b/gcc/range-op.cc
> > @@ -2075,9 +2075,14 @@ operator_lshift::op1_range (irange &r,
> >                             tree type,
> >                             const irange &lhs,
> >                             const irange &op2,
> > -                           relation_kind rel ATTRIBUTE_UNUSED) const
> > +                           relation_kind rel) const
> >  {
> >    tree shift_amount;
> > +  int_range<2> adjust (type);
> > +
> > +  if (rel == EQ_EXPR && !lhs.contains_p (build_zero_cst (type)))
> > +    adjust.set_nonzero (type);
> > +
> >    if (op2.singleton_p (&shift_amount))
> >      {
> >        wide_int shift = wi::to_wide (shift_amount);
> > @@ -2086,10 +2091,11 @@ operator_lshift::op1_range (irange &r,
> >        if (wi::ge_p (shift, wi::uhwi (TYPE_PRECISION (type),
> >                                      TYPE_PRECISION (op2.type ())),
> >                     UNSIGNED))
> > -       return false;
> > +       goto done;
> >        if (shift == 0)
> >         {
> >           r = lhs;
> > +         r.intersect (adjust);
> >           return true;
> >         }
> >
> > @@ -2126,9 +2132,16 @@ operator_lshift::op1_range (irange &r,
> >
> >        if (utype != type)
> >         range_cast (r, type);
> > +      r.intersect (adjust);
> >        return true;
> >      }
> > -  return false;
> > +
> > + done:
> > +  if (adjust.varying_p ())
> > +    return false;
> > +
> > +  r = adjust;
> > +  return true;
> >  }
> >
> >  bool
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
> > new file mode 100644
> > index 00000000000..4bd98747732
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102546.c
> > @@ -0,0 +1,23 @@
> > +// { dg-do compile }
> > +// { dg-options "-O3 -fdump-tree-optimized" }
> > +
> > +static int a;
> > +static char b, c, d;
> > +void bar(void);
> > +void foo(void);
> > +
> > +int main() {
> > +    int f = 0;
> > +    for (; f <= 5; f++) {
> > +        bar();
> > +        b = b && f;
> > +        d = f << f;
> > +        if (!(a >= d || f))
> > +            foo();
> > +        c = 1;
> > +        for (; c; c = 0)
> > +            ;
> > +    }
> > +}
> > +
> > +// { dg-final { scan-tree-dump-not "foo" "optimized" } }
> > --
> > 2.31.1
> >

[-- Attachment #2: 0001-PR102563-Do-not-clobber-range-in-operator_lshift-op1.patch --]
[-- Type: text/x-patch, Size: 2537 bytes --]

From 72739a6fde0020b98cdabad7218c6d4f5ce36bce Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Sat, 2 Oct 2021 16:59:26 +0200
Subject: [PATCH] [PR102563] Do not clobber range in
 operator_lshift::op1_range.

We're clobbering the final range before we're done calculating it.

Tested on x86-64 Linux.

gcc/ChangeLog:

	* range-op.cc (operator_lshift::op1_range): Do not clobber
	range.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/pr102563.c: New test.
---
 gcc/range-op.cc                          | 12 ++++++------
 gcc/testsuite/gcc.dg/tree-ssa/pr102563.c | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102563.c

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index 2baca4a197f..bbf2924f815 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -2112,8 +2112,6 @@ operator_lshift::op1_range (irange &r,
       else
 	op_rshift.fold_range (tmp_range, utype, lhs, op2);
 
-      r.intersect (tmp_range);
-
       // Start with ranges which can produce the LHS by right shifting the
       // result by the shift amount.
       // ie   [0x08, 0xF0] = op1 << 2 will start with
@@ -2128,13 +2126,15 @@ operator_lshift::op1_range (irange &r,
       unsigned low_bits = TYPE_PRECISION (utype)
 			  - TREE_INT_CST_LOW (shift_amount);
       wide_int up_mask = wi::mask (low_bits, true, TYPE_PRECISION (utype));
-      wide_int new_ub = wi::bit_or (up_mask, r.upper_bound ());
-      wide_int new_lb = wi::set_bit (r.lower_bound (), low_bits);
+      wide_int new_ub = wi::bit_or (up_mask, tmp_range.upper_bound ());
+      wide_int new_lb = wi::set_bit (tmp_range.lower_bound (), low_bits);
       int_range<2> fill_range (utype, new_lb, new_ub);
-      r.union_ (fill_range);
+      tmp_range.union_ (fill_range);
 
       if (utype != type)
-	range_cast (r, type);
+	range_cast (tmp_range, type);
+
+      r.intersect (tmp_range);
       return true;
     }
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102563.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102563.c
new file mode 100644
index 00000000000..8871dffe24a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102563.c
@@ -0,0 +1,16 @@
+// { dg-do compile }
+// { dg-options "-O2 -w" }
+
+int _bdf_parse_glyphs_bp;
+long _bdf_parse_glyphs_nibbles;
+
+void _bdf_parse_glyphs_p() 
+{
+  long p_2;
+
+  _bdf_parse_glyphs_nibbles = p_2 << 1;
+
+  for (; 0 < _bdf_parse_glyphs_nibbles;)
+    if (1 < _bdf_parse_glyphs_nibbles)
+      _bdf_parse_glyphs_bp = _bdf_parse_glyphs_p;
+}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Handle EQ_EXPR relation for operator_lshift.
  2021-10-02 19:50     ` Aldy Hernandez
@ 2021-10-02 20:53       ` Jeff Law
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2021-10-02 20:53 UTC (permalink / raw)
  To: Aldy Hernandez, GCC patches



On 10/2/2021 1:50 PM, Aldy Hernandez via Gcc-patches wrote:
> Bah.  The range was being clobbered half way through the calculation.
>
> Tested on x86-64 Linux.
>
> Pushed.
>
> On Fri, Oct 1, 2021 at 4:52 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>> Well, after talking with Andrew it seems that X << Y being non-zero
>> also implies X is non-zero.  So we don't even need relationals here.
>>
>> So, I leave gori relationals in his capable hands, while I test this
>> much simpler patch which fixes the PR with no additional
>> infrastructure ;-).
>>
>> Will push pending tests.
>> Aldy
>>
>> On Fri, Oct 1, 2021 at 2:43 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>> Knowing that X << X is non-zero means X is also non-zero.  This patch
>>> teaches this this to range-ops.
>>>
>>> As usual, the big twiddling experts could come up with all sorts of
>>> fancy enhancements in this area, and we welcome all patches :).
>>>
>>> I will push this pending tests.
>>>
>>> gcc/ChangeLog:
>>>
>>>          PR tree-optimization/102546
>>>          * range-op.cc (operator_lshift::op1_range): Handle EQ_EXPR
>>>          relation.
I'm going to assume this also fixes a similar ICE building the linux 
kernel.  One less thing to bisect today :-)

jeff


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-02 20:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-01 12:43 [COMMITTED] Remove shadowed oracle field Aldy Hernandez
2021-10-01 12:43 ` [PATCH] Pass relations down to range_operator::op[12]_range Aldy Hernandez
2021-10-01 13:38   ` Andrew MacLeod
2021-10-01 12:43 ` [PATCH] Handle EQ_EXPR relation for operator_lshift Aldy Hernandez
2021-10-01 14:52   ` Aldy Hernandez
2021-10-02 19:50     ` Aldy Hernandez
2021-10-02 20:53       ` Jeff Law

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).