diff mbox series

[1/2] Use value_relation class instead of direct calls to intersect/union.

Message ID 2b7a5b75-aa71-b399-ff07-1f62dfac6cdc@redhat.com
State Accepted
Headers
Series [1/2] Use value_relation class instead of direct calls to intersect/union. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew MacLeod Jan. 23, 2023, 5:44 p.m. UTC 
  In order to fix 108447, this patch changes the calls to relation_union 
and relation_intersection to instead utilize the value_relation class, 
and make the calls through that instead.

value_relation manages the operands of a relation, and thus understands 
whether this is a floating point relation and can make any adjustments 
required to get the proper results.  THis patch doesn't do anything 
additional, just that isolation so we can track if it introduces 
anything speerate from the next change which adds another relation.

Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK?

Andrew

Comments

Richard Biener Jan. 24, 2023, 8:52 a.m. UTC | #1 
On Mon, Jan 23, 2023 at 6:44 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
> In order to fix 108447, this patch changes the calls to relation_union
> and relation_intersection to instead utilize the value_relation class,
> and make the calls through that instead.
>
> value_relation manages the operands of a relation, and thus understands
> whether this is a floating point relation and can make any adjustments
> required to get the proper results.  THis patch doesn't do anything
> additional, just that isolation so we can track if it introduces
> anything speerate from the next change which adds another relation.
>
> Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK?

LGTM

> Andrew
Jakub Jelinek Jan. 24, 2023, 9:36 a.m. UTC | #2 
On Mon, Jan 23, 2023 at 12:44:42PM -0500, Andrew MacLeod wrote:
> In order to fix 108447, this patch changes the calls to relation_union and
> relation_intersection to instead utilize the value_relation class, and make
> the calls through that instead.
> 
> value_relation manages the operands of a relation, and thus understands
> whether this is a floating point relation and can make any adjustments
> required to get the proper results.  THis patch doesn't do anything
> additional, just that isolation so we can track if it introduces anything
> speerate from the next change which adds another relation.
> 
> Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK?

LGTM.

	Jakub
diff mbox series

Patch

From 566421cb3b91272b56b01ff9ad5a243cada38ff7 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Fri, 20 Jan 2023 17:13:08 -0500
Subject: [PATCH 1/2] Use value_relation class instead of direct calls to
 intersect/union.

There are subtle differences how union and intersection behave depending
on whetehr the operands are float or integral.  Directly calling
relation_union or relation_intersect does not expose the origin of the
operands, but value_relation does have them.
THis patch makes sure calls are  through the value relation class.

	* gimple-range-fold.cc (fold_using_range::relation_fold_and_or):
	Call union and intersect through a value_relation class.
	* value-relation.cc (value_relation::intersect): Make parameter
	const.
	(value_relation::union_): Likewise.
	(dom_oracle::set_one_relation): Use value-relation class for
	union and intersect.
	(path_oracle::register_relation): Likewise.
	* value-relation.h (value_relation::intersect): Add const to param.
	(value_relation::union_): Likewise.
---
 gcc/gimple-range-fold.cc | 17 +++++------------
 gcc/value-relation.cc    | 17 ++++++++---------
 gcc/value-relation.h     |  4 ++--
 3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 91eb6298254..500c482eeec 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1039,14 +1039,6 @@  fold_using_range::relation_fold_and_or (irange& lhs_range, gimple *s,
   if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
     return;
 
-  // Make sure they are the same dependencies, and detect the order of the
-  // relationship.
-  bool reverse_op2 = true;
-  if (ssa1_dep1 == ssa2_dep1 && ssa1_dep2 == ssa2_dep2)
-    reverse_op2 = false;
-  else if (ssa1_dep1 != ssa2_dep2 || ssa1_dep2 != ssa2_dep1)
-    return;
-
   int_range<2> bool_one (boolean_true_node, boolean_true_node);
 
   relation_kind relation1 = handler1.op1_op2_relation (bool_one);
@@ -1054,15 +1046,16 @@  fold_using_range::relation_fold_and_or (irange& lhs_range, gimple *s,
   if (relation1 == VREL_VARYING || relation2 == VREL_VARYING)
     return;
 
-  if (reverse_op2)
-    relation2 = relation_negate (relation2);
+  value_relation vr1 (relation1, ssa1_dep1, ssa1_dep2);
+  value_relation vr2 (relation2, ssa2_dep1, ssa2_dep2);
 
+  // Only one of the follwoing intersection/unions is performed.
   // x && y is false if the relation intersection of the true cases is NULL.
-  if (is_and && relation_intersect (relation1, relation2) == VREL_UNDEFINED)
+  if (is_and && vr1.intersect (vr2) && vr1.kind () == VREL_UNDEFINED)
     lhs_range = int_range<2> (boolean_false_node, boolean_false_node);
   // x || y is true if the union of the true cases is NO-RELATION..
   // ie, one or the other being true covers the full range of possibilties.
-  else if (!is_and && relation_union (relation1, relation2) == VREL_VARYING)
+  else if (!is_and && vr1.union_ (vr2) && vr1.kind () == VREL_VARYING)
     lhs_range = bool_one;
   else
     return;
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index 6f8a1b7e7d3..432828e2b13 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -782,7 +782,7 @@  value_relation::negate ()
 // Perform an intersection between 2 relations. *this &&= p.
 
 bool
-value_relation::intersect (value_relation &p)
+value_relation::intersect (const value_relation &p)
 {
   // Save previous value
   relation_kind old = related;
@@ -800,7 +800,7 @@  value_relation::intersect (value_relation &p)
 // Perform a union between 2 relations. *this ||= p.
 
 bool
-value_relation::union_ (value_relation &p)
+value_relation::union_ (const value_relation &p)
 {
   // Save previous value
   relation_kind old = related;
@@ -1118,8 +1118,7 @@  dom_oracle::set_one_relation (basic_block bb, relation_kind k, tree op1,
       // will be the aggregate of all the previous ones.
       curr = find_relation_dom (bb, v1, v2);
       if (curr != VREL_VARYING)
-	k = relation_intersect (curr, k);
-
+	vr.intersect (value_relation (curr, op1, op2));
       bitmap_set_bit (bm, v1);
       bitmap_set_bit (bm, v2);
       bitmap_set_bit (m_relation_set, v1);
@@ -1127,7 +1126,7 @@  dom_oracle::set_one_relation (basic_block bb, relation_kind k, tree op1,
 
       ptr = (relation_chain *) obstack_alloc (&m_chain_obstack,
 					      sizeof (relation_chain));
-      ptr->set_relation (k, op1, op2);
+      ptr->set_relation (vr.kind (), op1, op2);
       ptr->m_next = m_relations[bbi].m_head;
       m_relations[bbi].m_head = ptr;
     }
@@ -1528,9 +1527,9 @@  path_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
   if (ssa1 == ssa2)
     return;
 
+  value_relation vr (k, ssa1, ssa2);
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
-      value_relation vr (k, ssa1, ssa2);
       fprintf (dump_file, " Registering value_relation (path_oracle) ");
       vr.dump (dump_file);
       fprintf (dump_file, " (root: bb%d)\n", bb->index);
@@ -1538,9 +1537,9 @@  path_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
 
   relation_kind curr = query_relation (bb, ssa1, ssa2);
   if (curr != VREL_VARYING)
-    k = relation_intersect (curr, k);
+    vr.intersect (value_relation (curr, ssa1, ssa2));
 
-  if (k == VREL_EQ)
+  if (vr.kind () == VREL_EQ)
     {
       register_equiv (bb, ssa1, ssa2);
       return;
@@ -1550,7 +1549,7 @@  path_oracle::register_relation (basic_block bb, relation_kind k, tree ssa1,
   bitmap_set_bit (m_relations.m_names, SSA_NAME_VERSION (ssa2));
   relation_chain *ptr = (relation_chain *) obstack_alloc (&m_chain_obstack,
 						      sizeof (relation_chain));
-  ptr->set_relation (k, ssa1, ssa2);
+  ptr->set_relation (vr.kind (), ssa1, ssa2);
   ptr->m_next = m_relations.m_head;
   m_relations.m_head = ptr;
 }
diff --git a/gcc/value-relation.h b/gcc/value-relation.h
index 664fd71c925..354a0fd4130 100644
--- a/gcc/value-relation.h
+++ b/gcc/value-relation.h
@@ -426,8 +426,8 @@  public:
   inline tree op1 () const { return name1; }
   inline tree op2 () const { return name2; }
 
-  bool union_ (value_relation &p);
-  bool intersect (value_relation &p);
+  bool union_ (const value_relation &p);
+  bool intersect (const value_relation &p);
   void negate ();
   bool apply_transitive (const value_relation &rel);
 
-- 
2.39.0