[PATCH GCC]Pick up more address lowering cases for ivopt and tree-affine.c

[Bin.Cheng]

On Tue, May 6, 2014 at 6:44 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, May 6, 2014 at 10:39 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Fri, Dec 6, 2013 at 6:19 PM, Richard Biener
>> <richard.guenther@gmail.com> wrote:

>> Hi,
>> I split the patch into two and updated the test case.
>> The patches pass bootstrap/tests on x86/x86_64, also pass test on arm cortex-m3.
>> Is it OK?
>>
>> Thanks,
>> bin
>>
>> PATCH 1:
>>
>> 2014-05-06  Bin Cheng  <bin.cheng@arm.com>
>>
>>     * gcc/tree-affine.c (tree_to_aff_combination): Handle MEM_REF for
>>     core part of address expressions.
>
> No gcc/ in the changelog
>
> Simplify that by using aff_combination_add_cst:
>
> +      if (TREE_CODE (core) == MEM_REF)
> +       {
> +         aff_combination_add_cst (comb, mem_ref_offset (core));
> +         core = TREE_OPERAND (core, 0);
>
> patch 1 is ok with that change.

Installed with below change because of wide-int merge:
-      core = build_fold_addr_expr (core);
+      if (TREE_CODE (core) == MEM_REF)
+    {
+      aff_combination_add_cst (comb, wi::to_widest (TREE_OPERAND (core, 1)));
+      core = TREE_OPERAND (core, 0);

>
>> PATCH 2:
>>
>> 2014-05-06  Bin Cheng  <bin.cheng@arm.com>
>>
>>     * gcc/tree-ssa-loop-ivopts.c (contain_complex_addr_expr): New.
>>     (alloc_iv): Lower base expressions containing ADDR_EXPR.
>
> So this "lowers" addresses(?) which are based on &<not-decl>,
> like &a[0] + 4 but not &a + 4.  I question this odd choice.  ISTR
&a+4 is already in its lowered form, what we want to handle is &EXPR +
4, in which EXPR is MEM_REF/ARRAY_REF, etc..

> when originally introducing address lowering (in rev. 204497) I was
> concerned about the cost?
Yes, you did.  I still think the iv number is relative small for each
loop, thus the change won't cause compilation time issue considering
the existing tree<->affine transformation in ivopt.
I would like to collect more accurate time information for ivopt in
gcc bootstrap.  Should I use "-ftime-report" then add all time slices
in TV_TREE_LOOP_IVOPTS category?  Is there any better solutions?
Thanks.

>
> Now I wonder why we bother to convert the lowered form back
> to trees to store it in ->base and not simply keep (and always
> compute) the affine expansion only.  Thus, change struct iv
> to have aff_tree base instead of tree base?
>
> Can you see what it takes to do such change?  At the point
> we replace uses we go into affine representation again anyway.
Good idea, I may have a look.

Thanks,
bin

-- 
Best Regards.