[1/6] tree-ssa-dom.c: Normalize exprs, starting with ARRAY_REF to MEM_REF

On 30/10/15 05:35, Jeff Law wrote:
> On 10/29/2015 01:18 PM, Alan Lawrence wrote:
>> This patch just teaches DOM that ARRAY_REFs can be equivalent to MEM_REFs (with
>> pointer type to the array element type).
>>
>> gcc/ChangeLog:
>>
>>     * tree-ssa-dom.c (dom_normalize_single_rhs): New.
>>     (dom_normalize_gimple_stmt): New.
>>     (lookup_avail_expr): Call dom_normalize_gimple_stmt.
> Has this been tested?  Do you have tests where it can be shown to make a
> difference independent of the changes to tree-sra.c?
>
> The implementation looks fine, I just want to have some basic tests in the
> testsuite that show the benefit of this normalization.

I'll look at the implementation and Richard's comments soon, but as to tests -
well I had tried before and not had much luck but OK you make me try harder ;).

I'll put these in with the appropriate patches, but ssa-dom-cse-{5,6}.c test
the ARRAY_REF -> MEM_REF part (one of these has to disable SRA from optimizing
the whole test away, the other...ends up waiting until dom2 for the whole loop
to have been unrolled, so I'd not be surprised if this proved a bit fragile,
as in not spotting when some other phase starts doing the optimization instead).
ssa-dom-cse-7.c tests the normalization-of-MEM_REFs part; but AFAICT, the
*only* place making exprs like the problematic MEM[(int[8] *)&a ...], is SRA.
That is, ssa-dom-cse-7.c passes (and the patch series solves PR/63679) if
instead of my patch 2 (normalization of MEM_REFs) we have this:

On 3 November 2015 at 10:27, Alan Lawrence <alan.lawrence@arm.com> wrote:
> That is, ssa-dom-cse-7.c passes (and the patch series solves PR/63679) if
> instead of my patch 2 (normalization of MEM_REFs) we have this:
>
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index 4327990..2889a96 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -1697,7 +1697,7 @@ build_ref_for_offset (location_t loc, tree base, HOST_WIDE_INT offset,
>      }
>    else
>      {
> -      off = build_int_cst (reference_alias_ptr_type (base),
> +      off = build_int_cst (build_pointer_type (exp_type),
>                            base_offset + offset / BITS_PER_UNIT);
>        base = build_fold_addr_expr (unshare_expr (base));
>      }
>
> ...I'll test that fully but I have to wonder what the right path is here!

So with also changing the other reference_alias_ptr_type in the first
case of build_ref_for_offset, it breaks Ada ACATS (on x86):

c52101a "CHECK THAT ARRAY SUBTYPE CONVERSION IS APPLIED AFTER AN ARRAY
VALUE IS DETERMINED"
cc70003
cxac004 (stream access, stream functions)

....I'll not dig any further unless you think that change to SRA is
the right avenue to investigate!

Cheers, Alan

On Tue, Nov 3, 2015 at 12:13 PM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On 3 November 2015 at 10:27, Alan Lawrence <alan.lawrence@arm.com> wrote:
>> That is, ssa-dom-cse-7.c passes (and the patch series solves PR/63679) if
>> instead of my patch 2 (normalization of MEM_REFs) we have this:
>>
>> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>> index 4327990..2889a96 100644
>> --- a/gcc/tree-sra.c
>> +++ b/gcc/tree-sra.c
>> @@ -1697,7 +1697,7 @@ build_ref_for_offset (location_t loc, tree base, HOST_WIDE_INT offset,
>>      }
>>    else
>>      {
>> -      off = build_int_cst (reference_alias_ptr_type (base),
>> +      off = build_int_cst (build_pointer_type (exp_type),
>>                            base_offset + offset / BITS_PER_UNIT);
>>        base = build_fold_addr_expr (unshare_expr (base));
>>      }
>>
>> ...I'll test that fully but I have to wonder what the right path is here!
>
> So with also changing the other reference_alias_ptr_type in the first
> case of build_ref_for_offset, it breaks Ada ACATS (on x86):
>
> c52101a "CHECK THAT ARRAY SUBTYPE CONVERSION IS APPLIED AFTER AN ARRAY
> VALUE IS DETERMINED"
> cc70003
> cxac004 (stream access, stream functions)
>
> ....I'll not dig any further unless you think that change to SRA is
> the right avenue to investigate!

Nope, that change looks wrong to me.

Richard.

> Cheers, Alan