This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR81503 (SLSR invalid fold)


Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81503 identifies a problem in
SLSR where an invalid replacement is made because the desired value doesn't
fit in the target type.  This patch addresses that issue.  The signed case
is simple, as we require the value not to change when restricted to the
precision of the target type.  For the unsigned case, a negative value is
expected to be converted to the congruent positive value, so we need to
check for that.  We have several test cases in the test suite that exercise
the negative bump for an unsigned/pointer type.

Per Jakub's suggestion I attempted to do all of this within the wide_int
and widest_int framework.  However, I was unable to find a way to generate
maxval as a widest_int within that framework.  I can use 

   wide_int maxval = wi::max_value (prec, sign);

to get a wide_int, but I can't find a way to convert this to a widest_int
so that I can add it to directly to bump.  I am probably missing something
obvious here, so I welcome any suggestions.  At any rate, using
TYPE_MAX_VALUE works, it's just not as clean as I would like.

BTW, I plan a follow-up patch to rewrite the complex condition for 
executing the main block of code, which is starting to get out of control.  
It will be simpler to read if the conditions are negated and used to do an
immediate return.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.
Is this ok for trunk?

(Just noticed a tab/space problem on the comment lines, which I will fix.)

Thanks,
Bill


[gcc]

2017-08-03  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type.

[gcc/testsuite]

2017-08-03  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.


Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 250791)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -2074,6 +2074,10 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
 {
   tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt));
   enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt);
+  unsigned int prec = TYPE_PRECISION (target_type);
+  tree maxval = (POINTER_TYPE_P (target_type)
+		 ? TYPE_MAX_VALUE (sizetype)
+		 : TYPE_MAX_VALUE (target_type));
 
   /* It is highly unlikely, but possible, that the resulting
      bump doesn't fit in a HWI.  Abandon the replacement
@@ -2082,6 +2086,17 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
      types but allows for safe negation without twisted logic.  */
   if (wi::fits_shwi_p (bump)
       && bump.to_shwi () != HOST_WIDE_INT_MIN
+      /* It is more likely that the bump doesn't fit in the target
+	 type, so check whether constraining it to that type changes
+	 the value.  For a signed type, the value mustn't change.
+         For an unsigned type, the value may only change to a 
+         congruent value (for negative bumps).  */
+      && (TYPE_UNSIGNED (target_type)
+	  || wi::eq_p (bump, wi::ext (bump, prec, SIGNED)))
+      && (!TYPE_UNSIGNED (target_type)
+	  || wi::eq_p (bump, wi::ext (bump, prec, UNSIGNED))
+	  || wi::eq_p (bump + wi::to_widest (maxval) + 1,
+		       wi::ext (bump, prec, UNSIGNED)))
       /* It is not useful to replace casts, copies, negates, or adds of
 	 an SSA name and a constant.  */
       && cand_code != SSA_NAME
Index: gcc/testsuite/gcc.c-torture/execute/pr81503.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr81503.c	(nonexistent)
+++ gcc/testsuite/gcc.c-torture/execute/pr81503.c	(working copy)
@@ -0,0 +1,15 @@
+unsigned short a = 41461;
+unsigned short b = 3419;
+int c = 0;
+
+void foo() {
+  if (a + b * ~(0 != 5))
+    c = -~(b * ~(0 != 5)) + 2147483647;
+}
+
+int main() {
+  foo();
+  if (c != 2147476810)
+    return -1;
+  return 0;
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]