Re: [PATCH] Improve SCEV for array element

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Feb 13, 2012 at 10:54 AM, Jiangning Liu <jiangning.liu@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Jiangning Liu
>> Sent: Friday, January 20, 2012 5:07 PM
>> To: 'Richard Guenther'
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: RE: [PATCH] Improve SCEV for array element
>>
>> > 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.
>>
>> ChangeLog:
>>
>> 2012-01-20 ?Jiangning Liu ?<jiangning.liu@arm.com>
>>
>> ? ? ? ? * tree-scalar-evolution (interpret_rhs_expr): generate chrec
>> for
>> ? ? ? ? array reference and component reference.
>>
>>
>> ChangeLog for testsuite:
>>
>> 2012-01-20 ?Jiangning Liu ?<jiangning.liu@arm.com>
>>
>> ? ? ? ? * gcc.dg/tree-ssa/scev-3.c: New.
>> ? ? ? ? * gcc.dg/tree-ssa/scev-4.c: New.
>>
>
> Richard,
>
> PING... Is this patch OK after branch 4.7 is created and trunk is open
> again?

It's on my (rather large) list of things to review for 4.8.  Be patient ...

Richard.

> Thanks,
> -Jiangning
>
>> 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..4e06b75
>> --- a/gcc/tree-scalar-evolution.c
>> +++ b/gcc/tree-scalar-evolution.c
>> @@ -1712,16 +1712,61 @@ 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)) == ARRAY_REF
>> + ? ? ? ? ?|| TREE_CODE (TREE_OPERAND (rhs1, 0)) == MEM_REF
>> + ? ? ? ? ?|| TREE_CODE (TREE_OPERAND (rhs1, 0)) == COMPONENT_REF)
>> + ? ? ? ?{
>> + ? ? ? enum machine_mode mode;
>> + ? ? ? HOST_WIDE_INT bitsize, bitpos;
>> + ? ? ? int unsignedp;
>> + ? ? ? int volatilep = 0;
>> + ? ? ? tree base, offset;
>> + ? ? ? tree chrec3;
>> +
>> + ? ? ? 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
>> + ? ? ? ? {
>> + ? ? ? ? ? base = build1 (ADDR_EXPR, TREE_TYPE (rhs1), base);
>> + ? ? ? ? ? chrec1 = analyze_scalar_evolution (loop, base);
>> + ? ? ? ? ? chrec1 = chrec_convert (type, chrec1, at_stmt);
>> + ? ? ? ? ? res = chrec1;
>> + ? ? ? ? }
>>
>> - ? ? ?rhs2 = TREE_OPERAND (TREE_OPERAND (rhs1, 0), 1);
>> - ? ? ?rhs1 = TREE_OPERAND (TREE_OPERAND (rhs1, 0), 0);
>> - ? ? ?/* Fall through. ?*/
>> + ? ? ? 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)
>> + ? ? ? ? {
>> + ? ? ? ? ? gcc_assert ((bitpos % BITS_PER_UNIT) == 0);
>> +
>> + ? ? ? ? ? chrec3 = build_int_cst (integer_type_node,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bitpos / BITS_PER_UNIT);
>> + ? ? ? ? ? chrec3 = analyze_scalar_evolution (loop, chrec3);
>> + ? ? ? ? ? chrec3 = chrec_convert (integer_type_node, 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);
>>
>> Thanks,
>> -Jiangning
>
>
>