diff mbox

Improve SCEV for array element

Message ID 001201ccfcfa$ec0e3780$c42aa680$@liu@arm.com
State New
Headers show

Commit Message

Jiangning Liu March 8, 2012, 7:13 a.m. UTC 
> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: Tuesday, March 06, 2012 9:12 PM
> To: Jiangning Liu
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Improve SCEV for array element
> 
> On Fri, Jan 20, 2012 at 10:06 AM, Jiangning Liu <jiangning.liu@arm.com>
> wrote:
> >> It's definitely not ok at this stage but at most for next stage1.
> >
> > OK. I may wait until next stage1.
> >
> >> This is a very narrow pattern-match.  It doesn't allow for &a[i].x
> for
> >> example,
> >> even if a[i] is a one-element structure.  I think the canonical way
> of
> >> handling
> >> ADDR_EXPR is to use sth like
> >>
> >> base = get_inner_reference (TREE_OPERAND (rhs1, 0), ...,
> &offset,  ...);
> >> base = build1 (ADDR_EXPR, TREE_TYPE (rhs1), base);
> >>         chrec1 = analyze_scalar_evolution (loop, base);
> >>         chrec2 = analyze_scalar_evolution (loop, offset);
> >>         chrec1 = chrec_convert (type, chrec1, at_stmt);
> >>         chrec2 = chrec_convert (TREE_TYPE (offset), chrec2, at_stmt);
> >>         res = chrec_fold_plus (type, chrec1, chrec2);
> >>
> >> where you probably need to handle scev_not_known when analyzing
> offset
> >> (which might be NULL).  You also need to add bitpos to the base
> address
> >> (in bytes, of course).  Note that the &MEM_REF case would naturally
> >> work
> >> with this as well.
> >
> > OK. New patch is like below, and bootstrapped on x86-32.
> 
> You want instead of
> 
> +      if (TREE_CODE (TREE_OPERAND (rhs1, 0)) == ARRAY_REF
> +          || TREE_CODE (TREE_OPERAND (rhs1, 0)) == MEM_REF
> +          || TREE_CODE (TREE_OPERAND (rhs1, 0)) == COMPONENT_REF)
> +        {
> 
> if (TREE_CODE (TREE_OPERAND (rhs1, 0)) == MEM_REF
>     || handled_component_p (TREE_OPERAND (rhs1, 0)))
>   {
> 
> +             base = build1 (ADDR_EXPR, TREE_TYPE (rhs1), base);
> +             chrec1 = analyze_scalar_evolution (loop, base);
> 
> can you please add a wrapper
> 
> tree
> analyze_scalar_evolution_for_address_of (struct loop *loop, tree var)
> {
>   return analyze_scalar_evolution (loop, build_fold_addr_expr (var));
> }
> 
> and call that instead of building the ADDR_EXPR there?  We want
> to avoid building that tree node, but even such a simple wrapper would
> be prefered.
> 
> +         if (bitpos)
> 
> if (bitpos != 0)
> 
> +             chrec3 = build_int_cst (integer_type_node,
> +                                     bitpos / BITS_PER_UNIT);
> 
> please use size_int (bitpos / BITS_PER_UNIT) instead.  Using
> integer_type_node is definitely wrong.
> 
> Ok with that changes.
> 

Richard,

Modified as all you suggested, and new code is like below. Bootstrapped on
x86-32. OK for trunk now?

Thanks,
-Jiangning

ChangeLog:

2012-03-08  Jiangning Liu  <jiangning.liu@arm.com>

	* tree-scalar-evolution (interpret_rhs_expr): generate chrec for
	array reference and component reference.
	(analyze_scalar_evolution_for_address_of): New.

ChangeLog for testsuite:

2012-03-08  Jiangning Liu  <jiangning.liu@arm.com>

	* gcc.dg/tree-ssa/scev-3.c: New.
	* gcc.dg/tree-ssa/scev-4.c: New.

     case POINTER_PLUS_EXPR:
       chrec1 = analyze_scalar_evolution (loop, rhs1);
@@ -1961,6 +2006,14 @@ analyze_scalar_evolution (struct loop *loop, tree
var)
   return res;
 }
 
+/* Analyzes and returns the scalar evolution of VAR address in LOOP.  */
+
+static tree
+analyze_scalar_evolution_for_address_of (struct loop *loop, tree var)
+{
+  return analyze_scalar_evolution (loop, build_fold_addr_expr (var));
+}
+
 /* Analyze scalar evolution of use of VERSION in USE_LOOP with respect to
    WRTO_LOOP (which should be a superloop of USE_LOOP)
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
new file mode 100644
index 0000000..28d5c93
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int *a_p;
+int a[1000];
+
+f(int k)
+{
+	int i;
+
+	for (i=k; i<1000; i+=k) {
+		a_p = &a[i];
+		*a_p = 100;
+        }
+}
+
+/* { dg-final { scan-tree-dump-times "&a" 1 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
b/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
new file mode 100644
index 0000000..6c1e530
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
@@ -0,0 +1,23 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+typedef struct {
+	int x;
+	int y;
+} S;
+
+int *a_p;
+S a[1000];
+
+f(int k)
+{
+	int i;
+
+	for (i=k; i<1000; i+=k) {
+		a_p = &a[i].y;
+		*a_p = 100;
+        }
+}
+
+/* { dg-final { scan-tree-dump-times "&a" 1 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
index 2077c8d..c719984
--- a/gcc/tree-scalar-evolution.c
+++ b/gcc/tree-scalar-evolution.c
@@ -266,6 +266,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "params.h"
 
 static tree analyze_scalar_evolution_1 (struct loop *, tree, tree);
+static tree analyze_scalar_evolution_for_address_of (struct loop *loop,
+						     tree var);
 
 /* The cached information about an SSA name VAR, claiming that below
    basic block INSTANTIATED_BELOW, the value of VAR can be expressed
@@ -1712,16 +1714,59 @@  interpret_rhs_expr (struct loop *loop, gimple
at_stmt,
   switch (code)
     {
     case ADDR_EXPR:
-      /* Handle &MEM[ptr + CST] which is equivalent to POINTER_PLUS_EXPR.
*/
-      if (TREE_CODE (TREE_OPERAND (rhs1, 0)) != MEM_REF)
-	{
-	  res = chrec_dont_know;
-	  break;
-	}
+      if (TREE_CODE (TREE_OPERAND (rhs1, 0)) == MEM_REF
+	  || handled_component_p (TREE_OPERAND (rhs1, 0)))
+        {
+	  enum machine_mode mode;
+	  HOST_WIDE_INT bitsize, bitpos;
+	  int unsignedp;
+	  int volatilep = 0;
+	  tree base, offset;
+	  tree chrec3;
+	  tree unitpos;
+
+	  base = get_inner_reference (TREE_OPERAND (rhs1, 0),
+				      &bitsize, &bitpos, &offset,
+				      &mode, &unsignedp, &volatilep, false);
+
+	  if (TREE_CODE (base) == MEM_REF)
+	    {
+	      rhs2 = TREE_OPERAND (base, 1);
+	      rhs1 = TREE_OPERAND (base, 0);
+
+	      chrec1 = analyze_scalar_evolution (loop, rhs1);
+	      chrec2 = analyze_scalar_evolution (loop, rhs2);
+	      chrec1 = chrec_convert (type, chrec1, at_stmt);
+	      chrec2 = chrec_convert (TREE_TYPE (rhs2), chrec2, at_stmt);
+	      res = chrec_fold_plus (type, chrec1, chrec2);
+	    }
+	  else
+	    {
+	      chrec1 = analyze_scalar_evolution_for_address_of (loop, base);
+	      chrec1 = chrec_convert (type, chrec1, at_stmt);
+	      res = chrec1;
+	    }
+
+	  if (offset != NULL_TREE)
+	    {
+	      chrec2 = analyze_scalar_evolution (loop, offset);
+	      chrec2 = chrec_convert (TREE_TYPE (offset), chrec2, at_stmt);
+	      res = chrec_fold_plus (type, res, chrec2);
+	    }
+
+	  if (bitpos != 0)
+	    {
+	      gcc_assert ((bitpos % BITS_PER_UNIT) == 0);
 
-      rhs2 = TREE_OPERAND (TREE_OPERAND (rhs1, 0), 1);
-      rhs1 = TREE_OPERAND (TREE_OPERAND (rhs1, 0), 0);
-      /* Fall through.  */
+	      unitpos = size_int_kind (bitpos / BITS_PER_UNIT, SIZETYPE);
+	      chrec3 = analyze_scalar_evolution (loop, unitpos);
+	      chrec3 = chrec_convert (TREE_TYPE (unitpos), chrec3, at_stmt);
+	      res = chrec_fold_plus (type, res, chrec3);
+	    }
+        }
+      else
+	res = chrec_dont_know;
+      break;