On 2/23/23 03:36, Tamar Christina wrote:
Hi Andrew,

Oh yeah, and in case you haven't figured it out on your own, you'll
have to remove WIDEN_MULT_EXPR from the range-ops init table.   This
non-standard mechanism only gets checked if there is no standard
range-op table entry for the tree code :-P

Hmm it looks like it'll work, but it keeps segfaulting in:

bool
range_op_handler::fold_range (vrange &r, tree type,
                              const vrange &lh,
                              const vrange &rh,
                              relation_trio rel) const
{
    gcc_checking_assert (m_valid);
    if (m_int)
      return m_int->fold_range (as_a <irange> (r), type,
                           as_a <irange> (lh),
                           as_a <irange> (rh), rel);

while trying to call fold_range.

But m_int is set to the right instance. Probably something I'm
missing, I'll double check it all.

Hmm.  whats your class operator_widen_mult* look like? what are you
inheriting from?   Send me your patch and I'll have a look if you want. this is
somewhat  new territory :-)
I've attached the patch, and my testcase is:

int decMultiplyOp_zacc, decMultiplyOp_iacc;
int *decMultiplyOp_lp;
void decMultiplyOp() {
   decMultiplyOp_lp = &decMultiplyOp_zacc;
   for (; decMultiplyOp_lp < &decMultiplyOp_zacc + decMultiplyOp_iacc;
        decMultiplyOp_lp++)
     *decMultiplyOp_lp = 0;
}

And compiling with aarch64-none-elf-gcc -O2 zero.c -S -o - 
-Werror=stringop-overflow

Also to explain a bit on why we're only seeing this now:

The original sequence for most of the pipeline is based on a cast and 
multiplication

   # RANGE [irange] long unsigned int [0, 2147483647][18446744071562067968, 
+INF]
   _14 = (long unsigned intD.11) decMultiplyOp_iacc.2_13;
   # RANGE [irange] long unsigned int [0, 8589934588][18446744065119617024, 
18446744073709551612] NONZERO 0xfffffffffffffffc
   _15 = _14 * 4;

But things like widening multiply are quite common, so some ISAs have it on 
scalars as well, not just vectors.
So there's a pass widening_mul that runs late for these targets.  This replaces 
the above with

   # RANGE [irange] long unsigned int [0, 8589934588][18446744065119617024, 
18446744073709551612] NONZERO 0xfffffffffffffffc
   _15 = decMultiplyOp_iacc.2_13 w* 4;

And copies over the final range from the original expression.

After that there are passes like the warning passes that try to requery ranged 
to see if any optimization  has changed them.
Before my attempt to support *w this would just return VARYING and it would 
only use the old range.

Now however, without taking care to sign extend when appropriate the MIN range 
changes from a negative value to a large
positive one when we increase the precision.  So passes that re-query late get 
the wrong range.  That's why for instance in this case
we get an incorrect warning generated.

Thanks for the help!

Tamar

I cant imagine it being a linkage thing between the 2 files since the operator 
is
defined in another file and the address taken in this one?
that should work, but strange that cant make the call...

Andrew

It is some sort of linkage/vtable thing.  The fix.diff patch applied on top of what you have will fix the fold issue. This'll do for now until I formalize how this is going to work goign forward.

Inheriting from operator_mult is also going to be hazardous because it also has an op1_range and op2_range...  you should at least define those and return VARYING to avoid other issues.  Same thing applies to widen_plus I think, and it has relation processing and other things as well.  Your widen operands are not what those classes expect, so I think you probably just want a fresh range operator.

It also looks like the mult operation is sign/zero extending both upper bounds, and neither lower bound..   I think that should be the LH upper and lower bound?

I've attached a second patch  (newversion.patch) which incorporates my fix, the fix to the sign of only op1's bounds,  as well as a simplification of the classes to not inherit from operator_mult/plus..   I think this still does what you want?  and it wont get you into unexpected trouble later :-)

let me know if this is still doing what you are expecting...

Andrew

diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
index 81089876d30..824e0338f34 100644
--- a/gcc/gimple-range-op.cc
+++ b/gcc/gimple-range-op.cc
@@ -777,8 +777,6 @@ gimple_range_op_handler::maybe_non_standard ()
       {
 	case WIDEN_MULT_EXPR:
 	{
-	  extern class range_operator &op_widen_mult_signed;
-	  extern class range_operator &op_widen_mult_unsigned;
 	  m_valid = true;
 	  m_op1 = gimple_assign_rhs1 (m_stmt);
 	  m_op2 = gimple_assign_rhs2 (m_stmt);
@@ -788,9 +786,9 @@ gimple_range_op_handler::maybe_non_standard ()
 	    std::swap (m_op1, m_op2);
 
 	  if (signed1 || signed2)
-	    m_int = &op_widen_mult_signed;
+	    m_int = ptr_op_widen_mult_signed;
 	  else
-	    m_int = &op_widen_mult_unsigned;
+	    m_int = ptr_op_widen_mult_unsigned;
 	  break;
 	}
 	default:
diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index c15bd83b077..bace915b256 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -2072,6 +2072,7 @@ public:
 				const wide_int &w0, const wide_int &w1)
     const;
 } op_widen_mult_signed;
+range_operator *ptr_op_widen_mult_signed = &op_widen_mult_signed;
 
 void
 operator_widen_mult_signed::wi_fold (irange &r, tree type,
@@ -2119,6 +2120,7 @@ public:
 				const wide_int &w0, const wide_int &w1)
     const;
 } op_widen_mult_unsigned;
+range_operator *ptr_op_widen_mult_unsigned = &op_widen_mult_unsigned;
 
 void
 operator_widen_mult_unsigned::wi_fold (irange &r, tree type,
diff --git a/gcc/range-op.h b/gcc/range-op.h
index f00b747f08a..5fe463234ae 100644
--- a/gcc/range-op.h
+++ b/gcc/range-op.h
@@ -311,4 +311,6 @@ private:
 // This holds the range op table for floating point operations.
 extern floating_op_table *floating_tree_table;
 
+extern range_operator *ptr_op_widen_mult_signed;
+extern range_operator *ptr_op_widen_mult_unsigned;
 #endif // GCC_RANGE_OP_H
diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
index d9dfdc56939..824e0338f34 100644
--- a/gcc/gimple-range-op.cc
+++ b/gcc/gimple-range-op.cc
@@ -179,6 +179,8 @@ gimple_range_op_handler::gimple_range_op_handler (gimple *s)
   // statements.
   if (is_a <gcall *> (m_stmt))
     maybe_builtin_call ();
+  else
+    maybe_non_standard ();
 }
 
 // Calculate what we can determine of the range of this unary
@@ -764,6 +766,36 @@ public:
   }
 } op_cfn_parity;
 
+// Set up a gimple_range_op_handler for any nonstandard function which can be
+// supported via range-ops.
+
+void
+gimple_range_op_handler::maybe_non_standard ()
+{
+  if (gimple_code (m_stmt) == GIMPLE_ASSIGN)
+    switch (gimple_assign_rhs_code (m_stmt))
+      {
+	case WIDEN_MULT_EXPR:
+	{
+	  m_valid = true;
+	  m_op1 = gimple_assign_rhs1 (m_stmt);
+	  m_op2 = gimple_assign_rhs2 (m_stmt);
+	  bool signed1 = TYPE_SIGN (TREE_TYPE (m_op1)) == SIGNED;
+	  bool signed2 = TYPE_SIGN (TREE_TYPE (m_op2)) == SIGNED;
+	  if (signed2 && !signed1)
+	    std::swap (m_op1, m_op2);
+
+	  if (signed1 || signed2)
+	    m_int = ptr_op_widen_mult_signed;
+	  else
+	    m_int = ptr_op_widen_mult_unsigned;
+	  break;
+	}
+	default:
+	  break;
+      }
+}
+
 // Set up a gimple_range_op_handler for any built in function which can be
 // supported via range-ops.
 
diff --git a/gcc/gimple-range-op.h b/gcc/gimple-range-op.h
index 743b858126e..1bf63c5ce6f 100644
--- a/gcc/gimple-range-op.h
+++ b/gcc/gimple-range-op.h
@@ -41,6 +41,7 @@ public:
 		 relation_trio = TRIO_VARYING);
 private:
   void maybe_builtin_call ();
+  void maybe_non_standard ();
   gimple *m_stmt;
   tree m_op1, m_op2;
 };
diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index 5c67bce6d3a..7cd19a92d00 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -1556,6 +1556,34 @@ operator_plus::op2_range (irange &r, tree type,
   return op1_range (r, type, lhs, op1, rel.swap_op1_op2 ());
 }
 
+class operator_widen_plus : public range_operator
+{
+public:
+  virtual void wi_fold (irange &r, tree type,
+			const wide_int &lh_lb,
+			const wide_int &lh_ub,
+			const wide_int &rh_lb,
+			const wide_int &rh_ub) const;
+} op_widen_plus;
+
+void
+operator_widen_plus::wi_fold (irange &r, tree type,
+			const wide_int &lh_lb, const wide_int &lh_ub,
+			const wide_int &rh_lb, const wide_int &rh_ub) const
+{
+   wi::overflow_type ov_lb, ov_ub;
+   signop s = TYPE_SIGN (type);
+
+   wide_int lh_wlb = wide_int::from (lh_lb, wi::get_precision (lh_lb) * 2, s);
+   wide_int rh_wlb = wide_int::from (rh_lb, wi::get_precision (rh_lb) * 2, s);
+   wide_int lh_wub = wide_int::from (lh_ub, wi::get_precision (lh_ub) * 2, s);
+   wide_int rh_wub = wide_int::from (rh_ub, wi::get_precision (rh_ub) * 2, s);
+
+   wide_int new_lb = wi::add (lh_wlb, rh_wlb, s, &ov_lb);
+   wide_int new_ub = wi::add (lh_wub, rh_wub, s, &ov_ub);
+
+   r = int_range<2> (type, new_lb, new_ub);
+}
 
 class operator_minus : public range_operator
 {
@@ -2031,6 +2059,70 @@ operator_mult::wi_fold (irange &r, tree type,
     }
 }
 
+class operator_widen_mult_signed : public range_operator
+{
+public:
+  virtual void wi_fold (irange &r, tree type,
+			const wide_int &lh_lb,
+			const wide_int &lh_ub,
+			const wide_int &rh_lb,
+			const wide_int &rh_ub)
+    const;
+} op_widen_mult_signed;
+range_operator *ptr_op_widen_mult_signed = &op_widen_mult_signed;
+
+void
+operator_widen_mult_signed::wi_fold (irange &r, tree type,
+				     const wide_int &lh_lb,
+				     const wide_int &lh_ub,
+				     const wide_int &rh_lb,
+				     const wide_int &rh_ub) const
+{
+  signop s = TYPE_SIGN (type);
+
+  wide_int lh_wlb = wide_int::from (lh_lb, wi::get_precision (lh_lb) * 2, SIGNED);
+  wide_int lh_wub = wide_int::from (lh_ub, wi::get_precision (lh_ub) * 2, SIGNED);
+  wide_int rh_wlb = wide_int::from (rh_lb, wi::get_precision (rh_lb) * 2, s);
+  wide_int rh_wub = wide_int::from (rh_ub, wi::get_precision (rh_ub) * 2, s);
+
+  /* We don't expect a widening multiplication to be able to overflow but range
+     calculations for multiplications are complicated.  After widening the
+     operands lets call the base class.  */
+  return op_mult.wi_fold (r, type, lh_wlb, lh_wub, rh_wlb, rh_wub);
+}
+
+
+class operator_widen_mult_unsigned : public range_operator
+{
+public:
+  virtual void wi_fold (irange &r, tree type,
+			const wide_int &lh_lb,
+			const wide_int &lh_ub,
+			const wide_int &rh_lb,
+			const wide_int &rh_ub)
+    const;
+} op_widen_mult_unsigned;
+range_operator *ptr_op_widen_mult_unsigned = &op_widen_mult_unsigned;
+
+void
+operator_widen_mult_unsigned::wi_fold (irange &r, tree type,
+				       const wide_int &lh_lb,
+				       const wide_int &lh_ub,
+				       const wide_int &rh_lb,
+				       const wide_int &rh_ub) const
+{
+  signop s = TYPE_SIGN (type);
+
+  wide_int lh_wlb = wide_int::from (lh_lb, wi::get_precision (lh_lb) * 2, UNSIGNED);
+  wide_int lh_wub = wide_int::from (lh_ub, wi::get_precision (lh_ub) * 2, UNSIGNED);
+  wide_int rh_wlb = wide_int::from (rh_lb, wi::get_precision (rh_lb) * 2, s);
+  wide_int rh_wub = wide_int::from (rh_ub, wi::get_precision (rh_ub) * 2, s);
+
+  /* We don't expect a widening multiplication to be able to overflow but range
+     calculations for multiplications are complicated.  After widening the
+     operands lets call the base class.  */
+  return op_mult.wi_fold (r, type, lh_wlb, lh_wub, rh_wlb, rh_wub);
+}
 
 class operator_div : public cross_product_operator
 {
@@ -4473,6 +4565,7 @@ integral_table::integral_table ()
   set (GT_EXPR, op_gt);
   set (GE_EXPR, op_ge);
   set (PLUS_EXPR, op_plus);
+  set (WIDEN_PLUS_EXPR, op_widen_plus);
   set (MINUS_EXPR, op_minus);
   set (MIN_EXPR, op_min);
   set (MAX_EXPR, op_max);
diff --git a/gcc/range-op.h b/gcc/range-op.h
index f00b747f08a..5fe463234ae 100644
--- a/gcc/range-op.h
+++ b/gcc/range-op.h
@@ -311,4 +311,6 @@ private:
 // This holds the range op table for floating point operations.
 extern floating_op_table *floating_tree_table;
 
+extern range_operator *ptr_op_widen_mult_signed;
+extern range_operator *ptr_op_widen_mult_unsigned;
 #endif // GCC_RANGE_OP_H

Reply via email to