public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] Use a boolean type when folding conditionals in simplify_using_ranges.
@ 2024-05-16 10:00 Aldy Hernandez
  2024-05-16 10:01 ` [COMMITTED] Cleanup prange sanity checks Aldy Hernandez
  2024-05-16 10:01 ` [COMMITTED] Revert "Revert: "Enable prange support."" Aldy Hernandez
  0 siblings, 2 replies; 7+ messages in thread
From: Aldy Hernandez @ 2024-05-16 10:00 UTC (permalink / raw)
  To: GCC patches; +Cc: Martin Jambor, Andrew MacLeod, Aldy Hernandez

In adding some traps for PR114985 I noticed that the conditional
folding code in simplify_using_ranges was using the wrong type.  This
cleans up the oversight.

gcc/ChangeLog:

	PR tree-optimization/114985
	* vr-values.cc (simplify_using_ranges::fold_cond_with_ops): Use
	boolean type when folding conditionals.
---
 gcc/vr-values.cc | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
index 0572bf6c8c7..e6ea9592574 100644
--- a/gcc/vr-values.cc
+++ b/gcc/vr-values.cc
@@ -316,10 +316,9 @@ simplify_using_ranges::fold_cond_with_ops (enum tree_code code,
       || !query->range_of_expr (r1, op1, s))
     return NULL_TREE;
 
-  tree type = TREE_TYPE (op0);
   int_range<1> res;
   range_op_handler handler (code);
-  if (handler && handler.fold_range (res, type, r0, r1))
+  if (handler && handler.fold_range (res, boolean_type_node, r0, r1))
     {
       if (res == range_true ())
 	return boolean_true_node;
-- 
2.45.0


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

* [COMMITTED] Cleanup prange sanity checks.
  2024-05-16 10:00 [COMMITTED] Use a boolean type when folding conditionals in simplify_using_ranges Aldy Hernandez
@ 2024-05-16 10:01 ` Aldy Hernandez
  2024-05-16 10:01 ` [COMMITTED] Revert "Revert: "Enable prange support."" Aldy Hernandez
  1 sibling, 0 replies; 7+ messages in thread
From: Aldy Hernandez @ 2024-05-16 10:01 UTC (permalink / raw)
  To: GCC patches; +Cc: Martin Jambor, Andrew MacLeod, Aldy Hernandez

The pointers_handled_p() code was a temporary sanity check, and not
even a good one, since we have a cleaner way of checking type
mismatches with operand_check_p.  This patch removes all the code, and
adds an explicit type check for relational operators, which are the
main problem in PR114985.

Adding this check makes it clear where the type mismatch is happening
in IPA, even without prange.  I've added code to skip the range
folding if the types don't match what the operator expects.  In order
to reproduce the latent bug, just remove the operand_check_p calls.

Tested on x86-64 and ppc64le with and without prange support.

gcc/ChangeLog:

	PR tree-optimization/114985
	* gimple-range-op.cc: Remove pointers_handled_p.
	* ipa-cp.cc (ipa_value_range_from_jfunc): Skip range folding if
	operands don't match.
	(propagate_vr_across_jump_function): Same.
	* range-op-mixed.h: Remove pointers_handled_p and tweak
	operand_check_p.
	* range-op-ptr.cc (range_operator::pointers_handled_p): Remove.
	(pointer_plus_operator::pointers_handled_p): Remove.
	(class operator_pointer_diff): Remove pointers_handled_p.
	(operator_pointer_diff::pointers_handled_p): Remove.
	(operator_identity::pointers_handled_p): Remove.
	(operator_cst::pointers_handled_p): Remove.
	(operator_cast::pointers_handled_p): Remove.
	(operator_min::pointers_handled_p): Remove.
	(operator_max::pointers_handled_p): Remove.
	(operator_addr_expr::pointers_handled_p): Remove.
	(operator_bitwise_and::pointers_handled_p): Remove.
	(operator_bitwise_or::pointers_handled_p): Remove.
	(operator_equal::pointers_handled_p): Remove.
	(operator_not_equal::pointers_handled_p): Remove.
	(operator_lt::pointers_handled_p): Remove.
	(operator_le::pointers_handled_p): Remove.
	(operator_gt::pointers_handled_p): Remove.
	(operator_ge::pointers_handled_p): Remove.
	* range-op.cc (TRAP_ON_UNHANDLED_POINTER_OPERATORS): Remove.
	(range_op_handler::lhs_op1_relation): Remove pointers_handled_p checks.
	(range_op_handler::lhs_op2_relation): Same.
	(range_op_handler::op1_op2_relation): Same.
	* range-op.h: Remove RO_* declarations.
---
 gcc/gimple-range-op.cc |  24 ----
 gcc/ipa-cp.cc          |  12 ++
 gcc/range-op-mixed.h   |  38 ++----
 gcc/range-op-ptr.cc    | 259 -----------------------------------------
 gcc/range-op.cc        |  43 +------
 gcc/range-op.h         |  17 ---
 6 files changed, 25 insertions(+), 368 deletions(-)

diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
index 55dfbb23ce2..7321342b00d 100644
--- a/gcc/gimple-range-op.cc
+++ b/gcc/gimple-range-op.cc
@@ -329,19 +329,6 @@ public:
     r = lhs;
     return true;
   }
-  virtual bool pointers_handled_p (range_op_dispatch_type type,
-				   unsigned dispatch) const
-  {
-    switch (type)
-      {
-      case DISPATCH_FOLD_RANGE:
-	return dispatch == RO_PPP;
-      case DISPATCH_OP1_RANGE:
-	return dispatch == RO_PPP;
-      default:
-	return true;
-      }
-  }
 } op_cfn_pass_through_arg1;
 
 // Implement range operator for CFN_BUILT_IN_SIGNBIT.
@@ -1132,17 +1119,6 @@ public:
     r.set (type, wi::zero (TYPE_PRECISION (type)), max - 2);
     return true;
   }
-  virtual bool pointers_handled_p (range_op_dispatch_type type,
-				   unsigned dispatch) const
-  {
-    switch (type)
-      {
-      case DISPATCH_FOLD_RANGE:
-	return dispatch == RO_IPI;
-      default:
-	return true;
-      }
-  }
 } op_cfn_strlen;
 
 
diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index 5781f50c854..09cab761822 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -1740,6 +1740,11 @@ ipa_value_range_from_jfunc (vrange &vr,
 
 	  if (!handler
 	      || !op_res.supports_type_p (vr_type)
+	      /* Sometimes we try to fold comparison operators using a
+		 pointer type to hold the result instead of a boolean
+		 type.  Avoid trapping in the sanity check in
+		 fold_range until this is fixed.  */
+	      || !handler.operand_check_p (vr_type, srcvr.type (), op_vr.type ())
 	      || !handler.fold_range (op_res, vr_type, srcvr, op_vr))
 	    op_res.set_varying (vr_type);
 
@@ -2547,6 +2552,13 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
 
 	  if (!handler
 	      || !ipa_supports_p (operand_type)
+	      /* Sometimes we try to fold comparison operators using a
+		 pointer type to hold the result instead of a boolean
+		 type.  Avoid trapping in the sanity check in
+		 fold_range until this is fixed.  */
+	      || !handler.operand_check_p (operand_type,
+					   src_lats->m_value_range.m_vr.type (),
+					   op_vr.type ())
 	      || !handler.fold_range (op_res, operand_type,
 				      src_lats->m_value_range.m_vr, op_vr))
 	    op_res.set_varying (param_type);
diff --git a/gcc/range-op-mixed.h b/gcc/range-op-mixed.h
index 44d51d68655..cc1db2f6775 100644
--- a/gcc/range-op-mixed.h
+++ b/gcc/range-op-mixed.h
@@ -151,9 +151,8 @@ public:
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
   // Check op1 and op2 for compatibility.
-  bool operand_check_p (tree, tree t1, tree t2) const final override
-    { return range_compatible_p (t1, t2); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
+  bool operand_check_p (tree t1, tree t2, tree t3) const final override
+  { return range_compatible_p (t2, t3) && INTEGRAL_TYPE_P (t1); }
 };
 
 class operator_not_equal : public range_operator
@@ -203,9 +202,8 @@ public:
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
   // Check op1 and op2 for compatibility.
-  bool operand_check_p (tree, tree t1, tree t2) const final override
-    { return range_compatible_p (t1, t2); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
+  bool operand_check_p (tree t0, tree t1, tree t2) const final override
+  { return range_compatible_p (t1, t2) && INTEGRAL_TYPE_P (t0); }
 };
 
 class operator_lt :  public range_operator
@@ -252,9 +250,8 @@ public:
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
   // Check op1 and op2 for compatibility.
-  bool operand_check_p (tree, tree t1, tree t2) const final override
-    { return range_compatible_p (t1, t2); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
+  bool operand_check_p (tree t1, tree t2, tree t3) const final override
+  { return range_compatible_p (t2, t3) && INTEGRAL_TYPE_P (t1); }
 };
 
 class operator_le :  public range_operator
@@ -304,9 +301,8 @@ public:
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
   // Check op1 and op2 for compatibility.
-  bool operand_check_p (tree, tree t1, tree t2) const final override
-    { return range_compatible_p (t1, t2); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
+  bool operand_check_p (tree t1, tree t2, tree t3) const final override
+  { return range_compatible_p (t2, t3) && INTEGRAL_TYPE_P (t1); }
 };
 
 class operator_gt :  public range_operator
@@ -355,9 +351,8 @@ public:
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
   // Check op1 and op2 for compatibility.
-  bool operand_check_p (tree, tree t1, tree t2) const final override
-    { return range_compatible_p (t1, t2); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
+  bool operand_check_p (tree t1, tree t2, tree t3) const final override
+  { return range_compatible_p (t2, t3) && INTEGRAL_TYPE_P (t1); }
 };
 
 class operator_ge :  public range_operator
@@ -407,9 +402,8 @@ public:
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
   // Check op1 and op2 for compatibility.
-  bool operand_check_p (tree, tree t1, tree t2) const final override
-    { return range_compatible_p (t1, t2); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
+  bool operand_check_p (tree t1, tree t2, tree t3) const final override
+  { return range_compatible_p (t2, t3) && INTEGRAL_TYPE_P (t1); }
 };
 
 class operator_identity : public range_operator
@@ -442,7 +436,6 @@ public:
   relation_kind lhs_op1_relation (const prange &lhs,
 				  const prange &op1, const prange &op2,
 				  relation_kind rel) const final override;
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 };
 
 class operator_cst : public range_operator
@@ -458,7 +451,6 @@ public:
   bool fold_range (frange &r, tree type,
 		   const frange &op1, const frange &op2,
 		   relation_trio = TRIO_VARYING) const final override;
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 };
 
 
@@ -507,7 +499,6 @@ public:
 				  relation_kind) const final override;
   void update_bitmask (irange &r, const irange &lh,
 		       const irange &rh) const final override;
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 private:
   bool truncating_cast_p (const irange &inner, const irange &outer) const;
   bool inside_domain_p (const wide_int &min, const wide_int &max,
@@ -730,7 +721,6 @@ public:
   bool op1_range (prange &r, tree type,
 		  const prange &lhs, const prange &op2,
 		  relation_trio rel = TRIO_VARYING) const final override;
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 };
 
 class operator_bitwise_not : public range_operator
@@ -807,7 +797,6 @@ public:
   // Check compatibility of all operands.
   bool operand_check_p (tree t1, tree t2, tree t3) const final override
     { return range_compatible_p (t1, t2) && range_compatible_p (t1, t3); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 protected:
   void wi_fold (irange &r, tree type, const wide_int &lh_lb,
 		const wide_int &lh_ub, const wide_int &rh_lb,
@@ -834,7 +823,6 @@ public:
   // Check compatibility of all operands.
   bool operand_check_p (tree t1, tree t2, tree t3) const final override
     { return range_compatible_p (t1, t2) && range_compatible_p (t1, t3); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 protected:
   void wi_fold (irange &r, tree type, const wide_int &lh_lb,
 		const wide_int &lh_ub, const wide_int &rh_lb,
@@ -855,7 +843,6 @@ public:
   // Check compatibility of all operands.
   bool operand_check_p (tree t1, tree t2, tree t3) const final override
     { return range_compatible_p (t1, t2) && range_compatible_p (t1, t3); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 protected:
   void wi_fold (irange &r, tree type, const wide_int &lh_lb,
 		const wide_int &lh_ub, const wide_int &rh_lb,
@@ -876,7 +863,6 @@ public:
   // Check compatibility of all operands.
   bool operand_check_p (tree t1, tree t2, tree t3) const final override
     { return range_compatible_p (t1, t2) && range_compatible_p (t1, t3); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 protected:
   void wi_fold (irange &r, tree type, const wide_int &lh_lb,
 		const wide_int &lh_ub, const wide_int &rh_lb,
diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc
index 2f47f3354ed..c24097a1c30 100644
--- a/gcc/range-op-ptr.cc
+++ b/gcc/range-op-ptr.cc
@@ -49,18 +49,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-ccp.h"
 #include "range-op-mixed.h"
 
-// Return TRUE if a range-op folder TYPE either handles or can safely
-// ignore the dispatch pattern in DISPATCH.  Return FALSE for any
-// combination not handled, which will result in a hard fail up the
-// chain.
-
-bool
-range_operator::pointers_handled_p (range_op_dispatch_type ATTRIBUTE_UNUSED,
-				    unsigned dispatch ATTRIBUTE_UNUSED) const
-{
-  return true;
-}
-
 bool
 range_operator::fold_range (prange &, tree, const prange &, const prange &,
 			    relation_trio) const
@@ -323,7 +311,6 @@ public:
 			  const irange &lhs,
 			  const irange &op1,
 			  relation_trio = TRIO_VARYING) const;
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
   void update_bitmask (irange &r, const irange &lh, const irange &rh) const
     { update_known_bitmask (r, POINTER_PLUS_EXPR, lh, rh); }
 } op_pointer_plus;
@@ -401,21 +388,6 @@ pointer_plus_operator::op2_range (irange &r, tree type,
   return true;
 }
 
-bool
-pointer_plus_operator::pointers_handled_p (range_op_dispatch_type type,
-					   unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_PPI;
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 void
 pointer_plus_operator::wi_fold (irange &r, tree type,
 				const wide_int &lh_lb,
@@ -612,7 +584,6 @@ class operator_pointer_diff : public range_operator
   void update_bitmask (irange &r,
 		       const prange &lh, const prange &rh) const final override
   { update_known_bitmask (r, POINTER_DIFF_EXPR, lh, rh); }
-  bool pointers_handled_p (range_op_dispatch_type, unsigned) const final override;
 } op_pointer_diff;
 
 bool
@@ -631,13 +602,6 @@ operator_pointer_diff::op1_op2_relation_effect (irange &lhs_range, tree type,
   return minus_op1_op2_relation_effect (lhs_range, type, op1, op2, rel);
 }
 
-bool
-operator_pointer_diff::pointers_handled_p (range_op_dispatch_type,
-					   unsigned) const
-{
-  return true;
-}
-
 bool
 operator_pointer_diff::op1_op2_relation_effect (irange &lhs_range, tree type,
 						const irange &op1_range,
@@ -836,21 +800,6 @@ operator_identity::op1_range (prange &r, tree type ATTRIBUTE_UNUSED,
   return true;
 }
 
-bool
-operator_identity::pointers_handled_p (range_op_dispatch_type type,
-				       unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_LHS_OP1_RELATION:
-      return dispatch == RO_PPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_cst::fold_range (prange &r, tree type ATTRIBUTE_UNUSED,
 			  const prange &lh,
@@ -861,19 +810,6 @@ operator_cst::fold_range (prange &r, tree type ATTRIBUTE_UNUSED,
   return true;
 }
 
-bool
-operator_cst::pointers_handled_p (range_op_dispatch_type type,
-				  unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_PPP;
-    default:
-      return true;
-    }
-}
-
 // Cast between pointers.
 
 bool
@@ -1099,26 +1035,6 @@ operator_cast::lhs_op1_relation (const irange &lhs,
   return bits_to_pe (prec);
 }
 
-bool
-operator_cast::pointers_handled_p (range_op_dispatch_type type,
-				   unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-    case DISPATCH_OP1_RANGE:
-      return (dispatch == RO_PPP
-	      || dispatch == RO_IPI
-	      || dispatch == RO_PIP);
-    case DISPATCH_LHS_OP1_RELATION:
-      return (dispatch == RO_PPP
-	      || dispatch == RO_PII
-	      || dispatch == RO_IPP);
-    default:
-      return true;
-    }
-}
-
 bool
 operator_min::fold_range (prange &r, tree type,
 			  const prange &op1,
@@ -1141,19 +1057,6 @@ operator_min::fold_range (prange &r, tree type,
   return true;
 }
 
-bool
-operator_min::pointers_handled_p (range_op_dispatch_type type,
-				  unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_PPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_max::fold_range (prange &r, tree type,
 			  const prange &op1,
@@ -1176,19 +1079,6 @@ operator_max::fold_range (prange &r, tree type,
   return true;
 }
 
-bool
-operator_max::pointers_handled_p (range_op_dispatch_type type,
-				  unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_PPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_addr_expr::op1_range (prange &r, tree type,
 			       const prange &lhs,
@@ -1210,23 +1100,6 @@ operator_addr_expr::op1_range (prange &r, tree type,
   return true;
 }
 
-bool
-operator_addr_expr::pointers_handled_p (range_op_dispatch_type type,
-					unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      // NOTE: It looks like we never generate this combination.
-      gcc_unreachable ();
-      return false;
-    case DISPATCH_OP1_RANGE:
-      return dispatch == RO_PPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_bitwise_and::fold_range (prange &r, tree type,
 				  const prange &op1,
@@ -1244,30 +1117,6 @@ operator_bitwise_and::fold_range (prange &r, tree type,
   return true;
 }
 
-bool
-operator_bitwise_and::pointers_handled_p (range_op_dispatch_type type,
-					  unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_PPP;
-    default:
-      return true;
-    }
-}
-
-bool
-operator_bitwise_or::pointers_handled_p (range_op_dispatch_type,
-					 unsigned) const
-{
-  // NOTE: It looks like we never generate bitwise OR with pointers.
-  // If this is indeed the case, we can move operator_bitwise_or from
-  // range-op-mixed.h to range-op.h.
-  gcc_unreachable ();
-  return false;
-}
-
 bool
 operator_equal::fold_range (irange &r, tree type,
 			    const prange &op1,
@@ -1367,24 +1216,6 @@ operator_equal::op1_op2_relation (const irange &lhs, const prange &,
   return VREL_VARYING;
 }
 
-bool
-operator_equal::pointers_handled_p (range_op_dispatch_type type,
-				    unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_IPP;
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_PIP;
-    case DISPATCH_OP1_OP2_RELATION:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_not_equal::fold_range (irange &r, tree type,
 				const prange &op1,
@@ -1485,24 +1316,6 @@ operator_not_equal::op1_op2_relation (const irange &lhs, const prange &,
   return VREL_VARYING;
 }
 
-bool
-operator_not_equal::pointers_handled_p (range_op_dispatch_type type,
-					unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_IPP;
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_PIP;
-    case DISPATCH_OP1_OP2_RELATION:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_lt::fold_range (irange &r, tree type,
 			 const prange &op1,
@@ -1596,24 +1409,6 @@ operator_lt::op1_op2_relation (const irange &lhs, const prange &,
   return VREL_VARYING;
 }
 
-bool
-operator_lt::pointers_handled_p (range_op_dispatch_type type,
-				 unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_IPP;
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_PIP;
-    case DISPATCH_OP1_OP2_RELATION:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_le::fold_range (irange &r, tree type,
 			 const prange &op1,
@@ -1704,24 +1499,6 @@ operator_le::op1_op2_relation (const irange &lhs, const prange &,
   return VREL_VARYING;
 }
 
-bool
-operator_le::pointers_handled_p (range_op_dispatch_type type,
-				 unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_IPP;
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_PIP;
-    case DISPATCH_OP1_OP2_RELATION:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_gt::fold_range (irange &r, tree type,
 			 const prange &op1, const prange &op2,
@@ -1810,24 +1587,6 @@ operator_gt::op1_op2_relation (const irange &lhs, const prange &,
   return VREL_VARYING;
 }
 
-bool
-operator_gt::pointers_handled_p (range_op_dispatch_type type,
-				 unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_IPP;
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_PIP;
-    case DISPATCH_OP1_OP2_RELATION:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 bool
 operator_ge::fold_range (irange &r, tree type,
 			 const prange &op1,
@@ -1918,24 +1677,6 @@ operator_ge::op1_op2_relation (const irange &lhs, const prange &,
   return VREL_VARYING;
 }
 
-bool
-operator_ge::pointers_handled_p (range_op_dispatch_type type,
-				 unsigned dispatch) const
-{
-  switch (type)
-    {
-    case DISPATCH_FOLD_RANGE:
-      return dispatch == RO_IPP;
-    case DISPATCH_OP1_RANGE:
-    case DISPATCH_OP2_RANGE:
-      return dispatch == RO_PIP;
-    case DISPATCH_OP1_OP2_RELATION:
-      return dispatch == RO_IPP;
-    default:
-      return true;
-    }
-}
-
 // Initialize any pointer operators to the primary table
 
 void
diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index 6a410ff656c..852d59c20e9 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -49,11 +49,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-ccp.h"
 #include "range-op-mixed.h"
 
-// Set to 1 to trap on range-op entries that cannot handle the pointer
-// combination being requested.  This is a temporary sanity check to
-// aid in debugging, and will be removed later in the release cycle.
-#define TRAP_ON_UNHANDLED_POINTER_OPERATORS 0
-
 // Instantiate the operators which apply to multiple types here.
 
 operator_equal op_equal;
@@ -238,11 +233,6 @@ range_op_handler::fold_range (vrange &r, tree type,
 #if CHECKING_P
   if (!lh.undefined_p () && !rh.undefined_p ())
     gcc_assert (m_operator->operand_check_p (type, lh.type (), rh.type ()));
-  if (TRAP_ON_UNHANDLED_POINTER_OPERATORS
-      && has_pointer_operand_p (r, lh, rh)
-      && !m_operator->pointers_handled_p (DISPATCH_FOLD_RANGE,
-					  dispatch_kind (r, lh, rh)))
-    discriminator_fail (r, lh, rh);
 #endif
   switch (dispatch_kind (r, lh, rh))
     {
@@ -305,11 +295,6 @@ range_op_handler::op1_range (vrange &r, tree type,
 #if CHECKING_P
   if (!op2.undefined_p ())
     gcc_assert (m_operator->operand_check_p (lhs.type (), type, op2.type ()));
-  if (TRAP_ON_UNHANDLED_POINTER_OPERATORS
-      && has_pointer_operand_p (r, lhs, op2)
-      && !m_operator->pointers_handled_p (DISPATCH_OP1_RANGE,
-					  dispatch_kind (r, lhs, op2)))
-    discriminator_fail (r, lhs, op2);
 #endif
   switch (dispatch_kind (r, lhs, op2))
     {
@@ -360,11 +345,6 @@ range_op_handler::op2_range (vrange &r, tree type,
 #if CHECKING_P
   if (!op1.undefined_p ())
     gcc_assert (m_operator->operand_check_p (lhs.type (), op1.type (), type));
-  if (TRAP_ON_UNHANDLED_POINTER_OPERATORS
-      && has_pointer_operand_p (r, lhs, op1)
-      && !m_operator->pointers_handled_p (DISPATCH_OP2_RANGE,
-					  dispatch_kind (r, lhs, op1)))
-    discriminator_fail (r, lhs, op1);
 #endif
   switch (dispatch_kind (r, lhs, op1))
     {
@@ -402,14 +382,6 @@ range_op_handler::lhs_op1_relation (const vrange &lhs,
 				    relation_kind rel) const
 {
   gcc_checking_assert (m_operator);
-#if CHECKING_P
-  if (TRAP_ON_UNHANDLED_POINTER_OPERATORS
-      && has_pointer_operand_p (lhs, op1, op2)
-      && !m_operator->pointers_handled_p (DISPATCH_LHS_OP1_RELATION,
-					  dispatch_kind (lhs, op1, op2)))
-    discriminator_fail (lhs, op1, op2);
-#endif
-
   switch (dispatch_kind (lhs, op1, op2))
     {
       case RO_III:
@@ -450,13 +422,6 @@ range_op_handler::lhs_op2_relation (const vrange &lhs,
 				    relation_kind rel) const
 {
   gcc_checking_assert (m_operator);
-#if CHECKING_P
-  if (TRAP_ON_UNHANDLED_POINTER_OPERATORS
-      && has_pointer_operand_p (lhs, op1, op2)
-      && !m_operator->pointers_handled_p (DISPATCH_LHS_OP2_RELATION,
-					  dispatch_kind (lhs, op1, op2)))
-    discriminator_fail (lhs, op1, op2);
-#endif
   switch (dispatch_kind (lhs, op1, op2))
     {
       case RO_III:
@@ -484,13 +449,7 @@ range_op_handler::op1_op2_relation (const vrange &lhs,
 				    const vrange &op2) const
 {
   gcc_checking_assert (m_operator);
-#if CHECKING_P
-  if (TRAP_ON_UNHANDLED_POINTER_OPERATORS
-      && has_pointer_operand_p (lhs, op1, op2)
-      && !m_operator->pointers_handled_p (DISPATCH_OP1_OP2_RELATION,
-					  dispatch_kind (lhs, op1, op2)))
-    discriminator_fail (lhs, op1, op2);
-#endif
+
   switch (dispatch_kind (lhs, op1, op2))
     {
       case RO_III:
diff --git a/gcc/range-op.h b/gcc/range-op.h
index 2bad5a90e11..7fd7f0d5ce6 100644
--- a/gcc/range-op.h
+++ b/gcc/range-op.h
@@ -229,7 +229,6 @@ public:
 
   // Compatability check for operands.
   virtual bool operand_check_p (tree, tree, tree) const;
-  virtual bool pointers_handled_p (enum range_op_dispatch_type, unsigned) const;
 
 protected:
   // Perform an integral operation between 2 sub-ranges and return it.
@@ -411,20 +410,4 @@ protected:
   void initialize_float_ops ();
 };
 
-// Temporary exports so the pointers_handled_p() sanity code can see
-// which pointer combination is being attempted.  This will be deleted
-// once pointers_handled_p is gone.
-extern const unsigned RO_III;
-extern const unsigned RO_IFI;
-extern const unsigned RO_IFF;
-extern const unsigned RO_FFF;
-extern const unsigned RO_FIF;
-extern const unsigned RO_FII;
-extern const unsigned RO_PPP;
-extern const unsigned RO_PPI;
-extern const unsigned RO_IPP;
-extern const unsigned RO_IPI;
-extern const unsigned RO_PIP;
-extern const unsigned RO_PII;
-
 #endif // GCC_RANGE_OP_H
-- 
2.45.0


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

* [COMMITTED] Revert "Revert: "Enable prange support.""
  2024-05-16 10:00 [COMMITTED] Use a boolean type when folding conditionals in simplify_using_ranges Aldy Hernandez
  2024-05-16 10:01 ` [COMMITTED] Cleanup prange sanity checks Aldy Hernandez
@ 2024-05-16 10:01 ` Aldy Hernandez
  2024-05-16 10:08   ` Jakub Jelinek
  1 sibling, 1 reply; 7+ messages in thread
From: Aldy Hernandez @ 2024-05-16 10:01 UTC (permalink / raw)
  To: GCC patches; +Cc: Martin Jambor, Andrew MacLeod, Aldy Hernandez

This reverts commit d7bb8eaade3cd3aa70715c8567b4d7b08098e699 and enables prange
support again.
---
 gcc/gimple-range-cache.cc     |  4 ++--
 gcc/gimple-range-fold.cc      |  4 ++--
 gcc/gimple-range-fold.h       |  2 +-
 gcc/gimple-range-infer.cc     |  2 +-
 gcc/gimple-range-op.cc        |  2 +-
 gcc/gimple-range-path.cc      |  2 +-
 gcc/gimple-ssa-warn-access.cc |  2 +-
 gcc/ipa-cp.h                  |  2 +-
 gcc/range-op-ptr.cc           |  4 ----
 gcc/range-op.cc               | 18 ++++--------------
 gcc/tree-ssa-structalias.cc   |  2 +-
 gcc/value-range.cc            |  1 +
 gcc/value-range.h             |  4 ++--
 13 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 72ac2552311..bdd2832873a 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -274,10 +274,10 @@ sbr_sparse_bitmap::sbr_sparse_bitmap (tree t, vrange_allocator *allocator,
   // Pre-cache zero and non-zero values for pointers.
   if (POINTER_TYPE_P (t))
     {
-      int_range<2> nonzero;
+      prange nonzero;
       nonzero.set_nonzero (t);
       m_range[1] = m_range_allocator->clone (nonzero);
-      int_range<2> zero;
+      prange zero;
       zero.set_zero (t);
       m_range[2] = m_range_allocator->clone (zero);
     }
diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 9c4ad1ee7b9..a9c8c4d03e6 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -597,7 +597,7 @@ fold_using_range::fold_stmt (vrange &r, gimple *s, fur_source &src, tree name)
   // Process addresses.
   if (gimple_code (s) == GIMPLE_ASSIGN
       && gimple_assign_rhs_code (s) == ADDR_EXPR)
-    return range_of_address (as_a <irange> (r), s, src);
+    return range_of_address (as_a <prange> (r), s, src);
 
   gimple_range_op_handler handler (s);
   if (handler)
@@ -757,7 +757,7 @@ fold_using_range::range_of_range_op (vrange &r,
 // If a range cannot be calculated, set it to VARYING and return true.
 
 bool
-fold_using_range::range_of_address (irange &r, gimple *stmt, fur_source &src)
+fold_using_range::range_of_address (prange &r, gimple *stmt, fur_source &src)
 {
   gcc_checking_assert (gimple_code (stmt) == GIMPLE_ASSIGN);
   gcc_checking_assert (gimple_assign_rhs_code (stmt) == ADDR_EXPR);
diff --git a/gcc/gimple-range-fold.h b/gcc/gimple-range-fold.h
index 7cbe15d05e5..c7c599bfc93 100644
--- a/gcc/gimple-range-fold.h
+++ b/gcc/gimple-range-fold.h
@@ -157,7 +157,7 @@ protected:
 			  fur_source &src);
   bool range_of_call (vrange &r, gcall *call, fur_source &src);
   bool range_of_cond_expr (vrange &r, gassign* cond, fur_source &src);
-  bool range_of_address (irange &r, gimple *s, fur_source &src);
+  bool range_of_address (prange &r, gimple *s, fur_source &src);
   bool range_of_phi (vrange &r, gphi *phi, fur_source &src);
   void range_of_ssa_name_with_loop_info (vrange &, tree, class loop *, gphi *,
 					 fur_source &src);
diff --git a/gcc/gimple-range-infer.cc b/gcc/gimple-range-infer.cc
index c8e8b9b60ac..d5e1aa14275 100644
--- a/gcc/gimple-range-infer.cc
+++ b/gcc/gimple-range-infer.cc
@@ -123,7 +123,7 @@ gimple_infer_range::add_nonzero (tree name)
 {
   if (!gimple_range_ssa_p (name))
     return;
-  int_range<2> nz;
+  prange nz;
   nz.set_nonzero (TREE_TYPE (name));
   add_range (name, nz);
 }
diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
index 7321342b00d..aec3f39ec0e 100644
--- a/gcc/gimple-range-op.cc
+++ b/gcc/gimple-range-op.cc
@@ -1107,7 +1107,7 @@ class cfn_strlen : public range_operator
 {
 public:
   using range_operator::fold_range;
-  virtual bool fold_range (irange &r, tree type, const irange &,
+  virtual bool fold_range (irange &r, tree type, const prange &,
 			   const irange &, relation_trio) const
   {
     wide_int max = irange_val_max (ptrdiff_type_node);
diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
index 96c6ac6b6a5..f1a12f76144 100644
--- a/gcc/gimple-range-path.cc
+++ b/gcc/gimple-range-path.cc
@@ -443,7 +443,7 @@ path_range_query::compute_ranges_in_block (basic_block bb)
 void
 path_range_query::adjust_for_non_null_uses (basic_block bb)
 {
-  int_range_max r;
+  prange r;
   bitmap_iterator bi;
   unsigned i;
 
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 2c10d19e7f3..0cd5b6d6ef4 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4213,7 +4213,7 @@ pass_waccess::check_pointer_uses (gimple *stmt, tree ptr,
 		 where the realloc call is known to have failed are valid.
 		 Ignore pointers that nothing is known about.  Those could
 		 have escaped along with their nullness.  */
-	      value_range vr;
+	      prange vr;
 	      if (m_ptr_qry.rvals->range_of_expr (vr, realloc_lhs, use_stmt))
 		{
 		  if (vr.zero_p ())
diff --git a/gcc/ipa-cp.h b/gcc/ipa-cp.h
index abeaaa4053e..e62a09f38af 100644
--- a/gcc/ipa-cp.h
+++ b/gcc/ipa-cp.h
@@ -296,7 +296,7 @@ bool values_equal_for_ipcp_p (tree x, tree y);
 static inline bool
 ipa_supports_p (tree type)
 {
-  return irange::supports_p (type);
+  return irange::supports_p (type) || prange::supports_p (type);
 }
 
 #endif /* IPA_CP_H */
diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc
index c24097a1c30..9421d3cd21d 100644
--- a/gcc/range-op-ptr.cc
+++ b/gcc/range-op-ptr.cc
@@ -1684,8 +1684,4 @@ range_op_table::initialize_pointer_ops ()
 {
   set (POINTER_PLUS_EXPR, op_pointer_plus);
   set (POINTER_DIFF_EXPR, op_pointer_diff);
-  set (BIT_AND_EXPR, op_hybrid_and);
-  set (BIT_IOR_EXPR, op_hybrid_or);
-  set (MIN_EXPR, op_hybrid_min);
-  set (MAX_EXPR, op_hybrid_max);
 }
diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index 852d59c20e9..d188d1a1b63 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -102,23 +102,13 @@ range_op_table::range_op_table ()
   set (MINUS_EXPR, op_minus);
   set (NEGATE_EXPR, op_negate);
   set (MULT_EXPR, op_mult);
-
-  // Occur in both integer and pointer tables, but currently share
-  // integral implementation.
   set (ADDR_EXPR, op_addr);
   set (BIT_NOT_EXPR, op_bitwise_not);
   set (BIT_XOR_EXPR, op_bitwise_xor);
-
-  // These are in both integer and pointer tables, but pointer has a different
-  // implementation.
-  // If commented out, there is a hybrid version in range-op-ptr.cc which
-  // is used until there is a pointer range class.  Then we can simply
-  // uncomment the operator here and use the unified version.
-
-  // set (BIT_AND_EXPR, op_bitwise_and);
-  // set (BIT_IOR_EXPR, op_bitwise_or);
-  // set (MIN_EXPR, op_min);
-  // set (MAX_EXPR, op_max);
+  set (BIT_AND_EXPR, op_bitwise_and);
+  set (BIT_IOR_EXPR, op_bitwise_or);
+  set (MIN_EXPR, op_min);
+  set (MAX_EXPR, op_max);
 }
 
 // Instantiate a default range operator for opcodes with no entry.
diff --git a/gcc/tree-ssa-structalias.cc b/gcc/tree-ssa-structalias.cc
index 9c63305063c..bb59c6a7c02 100644
--- a/gcc/tree-ssa-structalias.cc
+++ b/gcc/tree-ssa-structalias.cc
@@ -6833,7 +6833,7 @@ find_what_p_points_to (tree fndecl, tree p)
   struct ptr_info_def *pi;
   tree lookup_p = p;
   varinfo_t vi;
-  value_range vr;
+  prange vr;
   get_range_query (DECL_STRUCT_FUNCTION (fndecl))->range_of_expr (vr, p);
   bool nonnull = vr.nonzero_p ();
 
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 5bcb2c3f650..334ffb70fbc 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -1518,6 +1518,7 @@ irange::verify_range ()
       gcc_checking_assert (m_num_ranges == 0);
       return;
     }
+  gcc_checking_assert (supports_p (type ()));
   gcc_checking_assert (m_num_ranges <= m_max_ranges);
 
   // Legacy allowed these to represent VARYING for unknown types.
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 37ce91dc52d..44cdbd717f4 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -994,7 +994,7 @@ irange::varying_compatible_p () const
   const wide_int &u = m_base[1];
   tree t = m_type;
 
-  if (m_kind == VR_VARYING && t == error_mark_node)
+  if (m_kind == VR_VARYING)
     return true;
 
   unsigned prec = TYPE_PRECISION (t);
@@ -1039,7 +1039,7 @@ irange::nonzero_p () const
 inline bool
 irange::supports_p (const_tree type)
 {
-  return INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type);
+  return INTEGRAL_TYPE_P (type);
 }
 
 inline bool
-- 
2.45.0


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

* Re: [COMMITTED] Revert "Revert: "Enable prange support.""
  2024-05-16 10:01 ` [COMMITTED] Revert "Revert: "Enable prange support."" Aldy Hernandez
@ 2024-05-16 10:08   ` Jakub Jelinek
  2024-05-16 10:14     ` Aldy Hernandez
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2024-05-16 10:08 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC patches, Martin Jambor, Andrew MacLeod

On Thu, May 16, 2024 at 12:01:01PM +0200, Aldy Hernandez wrote:
> This reverts commit d7bb8eaade3cd3aa70715c8567b4d7b08098e699 and enables prange
> support again.

Please don't do this.
This breaks ChangeLog generation, will need to handle it tomorrow by hand again.
Both the ammendments to the git (cherry-pick -x or revert) added message
lines
This reverts commit COMMITHASH.
and
(cherry picked from commit COMMITHASH)
and revert of revert.

	Jakub


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

* Re: [COMMITTED] Revert "Revert: "Enable prange support.""
  2024-05-16 10:08   ` Jakub Jelinek
@ 2024-05-16 10:14     ` Aldy Hernandez
  2024-05-16 10:27       ` Xi Ruoyao
  2024-05-16 10:28       ` Jakub Jelinek
  0 siblings, 2 replies; 7+ messages in thread
From: Aldy Hernandez @ 2024-05-16 10:14 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC patches, Martin Jambor, Andrew MacLeod

Wait, what's the preferred way of reverting a patch?  I followed what I saw in:

commit 04ee1f788ceaa4c7f777ff3b9441ae076191439c
Author: Jeff Law <jlaw@ventanamicro.com>
Date:   Mon May 13 21:42:38 2024 -0600

    Revert "[PATCH v2 1/3] RISC-V: movmem for RISCV with V extension"

    This reverts commit df15eb15b5f820321c81efc75f0af13ff8c0dd5b.

and here:

commit 0c6dd4b0973738ce43e76b468a002ab5eb58aaf4
Author: YunQiang Su <syq@debian.org>
Date:   Mon May 13 14:15:38 2024 +0800

    Revert "MIPS: Support constraint 'w' for MSA instruction"

    This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8.

and here:

commit f6ce85502eb2e4e7bbd9b3c6c1c065a004f8f531
Author: Hans-Peter Nilsson <hp@bitrange.com>
Date:   Wed May 8 04:11:20 2024 +0200

    Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle
xpass from combine improvement""

    This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7.

etc etc.

Next time, would you like me to add manual changelog entries?

My apologies, I thought what I did was the blessed way of doing things.
Aldy

On Thu, May 16, 2024 at 12:08 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, May 16, 2024 at 12:01:01PM +0200, Aldy Hernandez wrote:
> > This reverts commit d7bb8eaade3cd3aa70715c8567b4d7b08098e699 and enables prange
> > support again.
>
> Please don't do this.
> This breaks ChangeLog generation, will need to handle it tomorrow by hand again.
> Both the ammendments to the git (cherry-pick -x or revert) added message
> lines
> This reverts commit COMMITHASH.
> and
> (cherry picked from commit COMMITHASH)
> and revert of revert.
>
>         Jakub
>


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

* Re: [COMMITTED] Revert "Revert: "Enable prange support.""
  2024-05-16 10:14     ` Aldy Hernandez
@ 2024-05-16 10:27       ` Xi Ruoyao
  2024-05-16 10:28       ` Jakub Jelinek
  1 sibling, 0 replies; 7+ messages in thread
From: Xi Ruoyao @ 2024-05-16 10:27 UTC (permalink / raw)
  To: Aldy Hernandez, Jakub Jelinek; +Cc: GCC patches, Martin Jambor, Andrew MacLeod

On Thu, 2024-05-16 at 12:14 +0200, Aldy Hernandez wrote:
> Wait, what's the preferred way of reverting a patch?  I followed what
> I saw in:
> 
> commit 04ee1f788ceaa4c7f777ff3b9441ae076191439c
> Author: Jeff Law <jlaw@ventanamicro.com>
> Date:   Mon May 13 21:42:38 2024 -0600
> 
>     Revert "[PATCH v2 1/3] RISC-V: movmem for RISCV with V extension"
> 
>     This reverts commit df15eb15b5f820321c81efc75f0af13ff8c0dd5b.

Revert is OK, but revert revert is not.

https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651144.html

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

* Re: [COMMITTED] Revert "Revert: "Enable prange support.""
  2024-05-16 10:14     ` Aldy Hernandez
  2024-05-16 10:27       ` Xi Ruoyao
@ 2024-05-16 10:28       ` Jakub Jelinek
  1 sibling, 0 replies; 7+ messages in thread
From: Jakub Jelinek @ 2024-05-16 10:28 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC patches, Martin Jambor, Andrew MacLeod

On Thu, May 16, 2024 at 12:14:09PM +0200, Aldy Hernandez wrote:
> Wait, what's the preferred way of reverting a patch?  I followed what I saw in:

Reverting a patch (that isn't a reversion) just push git revert.
The important part is not to modify the This reverts commit line from what
git revert created.

> commit 04ee1f788ceaa4c7f777ff3b9441ae076191439c
> Author: Jeff Law <jlaw@ventanamicro.com>
> Date:   Mon May 13 21:42:38 2024 -0600
> 
>     Revert "[PATCH v2 1/3] RISC-V: movmem for RISCV with V extension"
> 
>     This reverts commit df15eb15b5f820321c81efc75f0af13ff8c0dd5b.

So, this is just fine.

> and here:
> 
> commit 0c6dd4b0973738ce43e76b468a002ab5eb58aaf4
> Author: YunQiang Su <syq@debian.org>
> Date:   Mon May 13 14:15:38 2024 +0800
> 
>     Revert "MIPS: Support constraint 'w' for MSA instruction"
> 
>     This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8.

And this too.

What is not fine is hand edit the message:
    This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8 because
    foo and bar.
You can do that separately, so
    This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8.
    The reversion is because of foo and bar.
Or being further creative:
    This reverts commit r13-8390-g9de6ff5ec9a46951d2.
etc.

> commit f6ce85502eb2e4e7bbd9b3c6c1c065a004f8f531
> Author: Hans-Peter Nilsson <hp@bitrange.com>
> Date:   Wed May 8 04:11:20 2024 +0200
> 
>     Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle
> xpass from combine improvement""
> 
>     This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7.

This one is not fine.  Our current infrastructure for ChangeLog
generation can't deal with that and there is no agreement what to
write in the ChangeLog for it anyway, whether 2 reversions turn it into
Reapply commit: or 2 Revert: lines?  What happens on 3rd reversion?
So, one needs to manually remove the
This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7.
line and supply ChangeLog entry.

For cases like this or the ammended lines (or say if This reverts
commit or (cherry-picked from ....) lines refer to invalid commit
the daily DATESTAMP update then fails, I need to add manually
all problematic commits to IGNORED_COMMITS, rerun it by hand and
then write the ChangeLog entries by hand.
See
https://gcc.gnu.org/r13-8764
https://gcc.gnu.org/r15-426
https://gcc.gnu.org/r15-345
https://gcc.gnu.org/r15-344
https://gcc.gnu.org/r15-341
https://gcc.gnu.org/r14-9832
https://gcc.gnu.org/r14-9830
for what I had to do only in April/May for this.

	Jakub


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

end of thread, other threads:[~2024-05-16 10:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-16 10:00 [COMMITTED] Use a boolean type when folding conditionals in simplify_using_ranges Aldy Hernandez
2024-05-16 10:01 ` [COMMITTED] Cleanup prange sanity checks Aldy Hernandez
2024-05-16 10:01 ` [COMMITTED] Revert "Revert: "Enable prange support."" Aldy Hernandez
2024-05-16 10:08   ` Jakub Jelinek
2024-05-16 10:14     ` Aldy Hernandez
2024-05-16 10:27       ` Xi Ruoyao
2024-05-16 10:28       ` Jakub Jelinek

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