public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH PR80153]Part1, Get base pointer from the first element of pointer type aff_tree, Simplify add_elt_to_tree
@ 2017-04-10 14:35 Bin Cheng
  2017-04-10 15:37 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Bin Cheng @ 2017-04-10 14:35 UTC (permalink / raw)
  To: gcc-patches; +Cc: nd

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

Hi,
This is another try fixing PR80153.  It's based on richi's idea which is easier to understand than my
previous patch.  The patch gets base pointer from the first element of pointer aff_tree, builds result
expression in aff_tree's type unconditionally.  With this patch, A) we can do unconditional type
conversion on element in add_elt_to_tree, which simplify the function a lot; B) Except for below
mentioned fallout, customer of aff_tree doesn't need to do explicit conversion for result expression
of aff_combination_to_tree now.

Though this patch can handle normal constant base pointer_plus_expr, e.g, ((char *)1024 + (size_t)x),
we still can't handle extreme case in which x is of pointer type and its coef is 1.  I believe this is
a latent problem in tree-affine.c all the time.  It's not exposed because the biggest customer (ivopt)
always generates code in unsigned, rather than pointer type.  Let's see if this patch will uncover the issue.

I also need to do more invariant check in rewrite_use_nonlinear_expr for test gcc.dg/tree-ssa/reassoc-19.c.
 It relies on wrong behavior of tree-affine.c addressed in this patch.

Bootstrap and test on X86_64 and AArch64.  Is it OK if no failures?  The next patch addresses a
breakage of tree-affine.c usage.

Thanks,
bin

2017-04-07  Richard Biener  <rguenther@suse.de>
	    Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/80153
	* tree-affine.c (aff_combination_to_tree): Get base pointer from
	the first element of pointer type aff_tree.  Build result expr in
	aff_tree's type.
	(add_elt_to_tree): Convert to type unconditionally.  Remove other
	fold_convert calls.
	* tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types.
	(rewrite_use_nonlinear_expr): Check invariant using iv information.

gcc/testsuite/ChangeLog
2017-04-07  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/80153
	* gcc.c-torture/execute/pr80153.c: New.

[-- Attachment #2: pr80153-part1-20170407.txt --]
[-- Type: text/plain, Size: 6617 bytes --]

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr80153.c b/gcc/testsuite/gcc.c-torture/execute/pr80153.c
new file mode 100644
index 0000000..3eed578
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr80153.c
@@ -0,0 +1,48 @@
+/* PR tree-optimization/80153 */
+
+void check (int, int, int) __attribute__((noinline));
+void check (int c, int c2, int val)
+{
+  if (!val) {
+    __builtin_abort();
+  }
+}
+
+static const char *buf;
+static int l, i;
+
+void _fputs(const char *str)  __attribute__((noinline));
+void _fputs(const char *str)
+{
+  buf = str;
+  i = 0;
+  l = __builtin_strlen(buf);
+}
+
+char _fgetc() __attribute__((noinline));
+char _fgetc()
+{
+  char val = buf[i];
+  i++;
+  if (i > l)
+    return -1;
+  else
+    return val;
+}
+
+static const char *string = "oops!\n";
+
+int main(void)
+{
+  int i;
+  int c;
+
+  _fputs(string);
+
+  for (i = 0; i < __builtin_strlen(string); i++) {
+    c = _fgetc();
+    check(c, string[i], c == string[i]);
+  }
+
+  return 0;
+}
diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
index 30fff67..13c477d 100644
--- a/gcc/tree-affine.c
+++ b/gcc/tree-affine.c
@@ -377,58 +377,28 @@ static tree
 add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in)
 {
   enum tree_code code;
-  tree type1 = type;
-  if (POINTER_TYPE_P (type))
-    type1 = sizetype;
 
   widest_int scale = wide_int_ext_for_comb (scale_in, type);
 
-  if (scale == -1
-      && POINTER_TYPE_P (TREE_TYPE (elt)))
-    {
-      elt = convert_to_ptrofftype (elt);
-      elt = fold_build1 (NEGATE_EXPR, TREE_TYPE (elt), elt);
-      scale = 1;
-    }
-
+  elt = fold_convert (type, elt);
   if (scale == 1)
     {
       if (!expr)
-	{
-	  if (POINTER_TYPE_P (TREE_TYPE (elt)))
-	    return elt;
-	  else
-	    return fold_convert (type1, elt);
-	}
+	return elt;
 
-      if (POINTER_TYPE_P (TREE_TYPE (expr)))
-	return fold_build_pointer_plus (expr, elt);
-      if (POINTER_TYPE_P (TREE_TYPE (elt)))
-	return fold_build_pointer_plus (elt, expr);
-      return fold_build2 (PLUS_EXPR, type1,
-			  expr, fold_convert (type1, elt));
+      return fold_build2 (PLUS_EXPR, type, expr, elt);
     }
 
   if (scale == -1)
     {
       if (!expr)
-	return fold_build1 (NEGATE_EXPR, type1,
-			    fold_convert (type1, elt));
+	return fold_build1 (NEGATE_EXPR, type, elt);
 
-      if (POINTER_TYPE_P (TREE_TYPE (expr)))
-	{
-	  elt = convert_to_ptrofftype (elt);
-	  elt = fold_build1 (NEGATE_EXPR, TREE_TYPE (elt), elt);
-	  return fold_build_pointer_plus (expr, elt);
-	}
-      return fold_build2 (MINUS_EXPR, type1,
-			  expr, fold_convert (type1, elt));
+      return fold_build2 (MINUS_EXPR, type, expr, elt);
     }
 
-  elt = fold_convert (type1, elt);
   if (!expr)
-    return fold_build2 (MULT_EXPR, type1, elt,
-			wide_int_to_tree (type1, scale));
+    return fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale));
 
   if (wi::neg_p (scale))
     {
@@ -438,15 +408,8 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in)
   else
     code = PLUS_EXPR;
 
-  elt = fold_build2 (MULT_EXPR, type1, elt,
-		     wide_int_to_tree (type1, scale));
-  if (POINTER_TYPE_P (TREE_TYPE (expr)))
-    {
-      if (code == MINUS_EXPR)
-        elt = fold_build1 (NEGATE_EXPR, type1, elt);
-      return fold_build_pointer_plus (expr, elt);
-    }
-  return fold_build2 (code, type1, expr, elt);
+  elt = fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale));
+  return fold_build2 (code, type, expr, elt);
 }
 
 /* Makes tree from the affine combination COMB.  */
@@ -454,17 +417,25 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in)
 tree
 aff_combination_to_tree (aff_tree *comb)
 {
-  tree type = comb->type;
-  tree expr = NULL_TREE;
+  tree type = comb->type, base = NULL_TREE, expr = NULL_TREE;
   unsigned i;
   widest_int off, sgn;
-  tree type1 = type;
-  if (POINTER_TYPE_P (type))
-    type1 = sizetype;
 
   gcc_assert (comb->n == MAX_AFF_ELTS || comb->rest == NULL_TREE);
 
-  for (i = 0; i < comb->n; i++)
+  i = 0;
+  if (POINTER_TYPE_P (type))
+    {
+      type = sizetype;
+      if (comb->n > 0 && comb->elts[0].coef == 1
+	  && POINTER_TYPE_P (TREE_TYPE (comb->elts[0].val)))
+	{
+	  base = comb->elts[0].val;
+	  ++i;
+	}
+    }
+
+  for (; i < comb->n; i++)
     expr = add_elt_to_tree (expr, type, comb->elts[i].val, comb->elts[i].coef);
 
   if (comb->rest)
@@ -482,7 +453,12 @@ aff_combination_to_tree (aff_tree *comb)
       off = comb->offset;
       sgn = 1;
     }
-  return add_elt_to_tree (expr, type, wide_int_to_tree (type1, off), sgn);
+  expr = add_elt_to_tree (expr, type, wide_int_to_tree (type, off), sgn);
+
+  if (base)
+    return fold_build_pointer_plus (base, expr);
+  else
+    return fold_convert (comb->type, expr);
 }
 
 /* Copies the tree elements of COMB to ensure that they are not shared.  */
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index d5bd036..036e041 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -1171,7 +1171,7 @@ alloc_iv (struct ivopts_data *data, tree base, tree step,
       || contain_complex_addr_expr (expr))
     {
       aff_tree comb;
-      tree_to_aff_combination (expr, TREE_TYPE (base), &comb);
+      tree_to_aff_combination (expr, TREE_TYPE (expr), &comb);
       base = fold_convert (TREE_TYPE (base), aff_combination_to_tree (&comb));
     }
 
@@ -7183,7 +7183,7 @@ rewrite_use_nonlinear_expr (struct ivopts_data *data,
 			    struct iv_use *use, struct iv_cand *cand)
 {
   tree comp;
-  tree op, tgt;
+  tree tgt;
   gassign *ass;
   gimple_stmt_iterator bsi;
 
@@ -7194,6 +7194,7 @@ rewrite_use_nonlinear_expr (struct ivopts_data *data,
   if (cand->pos == IP_ORIGINAL
       && cand->incremented_at == use->stmt)
     {
+      tree op = NULL_TREE;
       enum tree_code stmt_code;
 
       gcc_assert (is_gimple_assign (use->stmt));
@@ -7213,14 +7214,19 @@ rewrite_use_nonlinear_expr (struct ivopts_data *data,
 	    op = gimple_assign_rhs2 (use->stmt);
 	  else if (gimple_assign_rhs2 (use->stmt) == cand->var_before)
 	    op = gimple_assign_rhs1 (use->stmt);
-	  else
-	    op = NULL_TREE;
 	}
-      else
-	op = NULL_TREE;
 
-      if (op && expr_invariant_in_loop_p (data->current_loop, op))
-	return;
+      if (op != NULL_TREE)
+	{
+	  if (expr_invariant_in_loop_p (data->current_loop, op))
+	    return;
+	  if (TREE_CODE (op) == SSA_NAME)
+	    {
+	      struct iv *iv = get_iv (data, op);
+	      if (iv != NULL && integer_zerop (iv->step))
+		return;
+	    }
+	}
     }
 
   comp = get_computation (data->current_loop, use, cand);

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

* Re: [PATCH PR80153]Part1, Get base pointer from the first element of pointer type aff_tree, Simplify add_elt_to_tree
  2017-04-10 14:35 [PATCH PR80153]Part1, Get base pointer from the first element of pointer type aff_tree, Simplify add_elt_to_tree Bin Cheng
@ 2017-04-10 15:37 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-04-10 15:37 UTC (permalink / raw)
  To: Bin Cheng; +Cc: gcc-patches, nd

On Mon, Apr 10, 2017 at 4:35 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> This is another try fixing PR80153.  It's based on richi's idea which is easier to understand than my
> previous patch.  The patch gets base pointer from the first element of pointer aff_tree, builds result
> expression in aff_tree's type unconditionally.  With this patch, A) we can do unconditional type
> conversion on element in add_elt_to_tree, which simplify the function a lot; B) Except for below
> mentioned fallout, customer of aff_tree doesn't need to do explicit conversion for result expression
> of aff_combination_to_tree now.
>
> Though this patch can handle normal constant base pointer_plus_expr, e.g, ((char *)1024 + (size_t)x),
> we still can't handle extreme case in which x is of pointer type and its coef is 1.  I believe this is
> a latent problem in tree-affine.c all the time.  It's not exposed because the biggest customer (ivopt)
> always generates code in unsigned, rather than pointer type.  Let's see if this patch will uncover the issue.
>
> I also need to do more invariant check in rewrite_use_nonlinear_expr for test gcc.dg/tree-ssa/reassoc-19.c.
>  It relies on wrong behavior of tree-affine.c addressed in this patch.
>
> Bootstrap and test on X86_64 and AArch64.  Is it OK if no failures?  The next patch addresses a
> breakage of tree-affine.c usage.

Ok.

Thanks,
Richard.

> Thanks,
> bin
>
> 2017-04-07  Richard Biener  <rguenther@suse.de>
>             Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/80153
>         * tree-affine.c (aff_combination_to_tree): Get base pointer from
>         the first element of pointer type aff_tree.  Build result expr in
>         aff_tree's type.
>         (add_elt_to_tree): Convert to type unconditionally.  Remove other
>         fold_convert calls.
>         * tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types.
>         (rewrite_use_nonlinear_expr): Check invariant using iv information.
>
> gcc/testsuite/ChangeLog
> 2017-04-07  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/80153
>         * gcc.c-torture/execute/pr80153.c: New.

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

end of thread, other threads:[~2017-04-10 15:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-10 14:35 [PATCH PR80153]Part1, Get base pointer from the first element of pointer type aff_tree, Simplify add_elt_to_tree Bin Cheng
2017-04-10 15:37 ` Richard Biener

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