On Thu, Mar 8, 2012 at 8:13 AM, Jiangning Liu <jiangning....@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Richard Guenther [mailto:richard.guent...@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....@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?
Ok.
Thanks,
Richard.
> Thanks,
> -Jiangning
>
> ChangeLog:
>
> 2012-03-08 Jiangning Liu <jiangning....@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....@arm.com>
>
> * gcc.dg/tree-ssa/scev-3.c: New.
> * gcc.dg/tree-ssa/scev-4.c: New.
>
> 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;
>
> 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)
>
>
>
