This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH PR64705]Don't aggressively expand induction variable's base


On Wed, Feb 11, 2015 at 7:24 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Feb 11, 2015 at 9:23 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Tue, Feb 10, 2015 at 12:24 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>
>> Previously, the computation of _1174 can be replaced by _629 in bb8 in
>> DOM2 pass, while it can't after patching.  This is the only possible
>> regression that I can see on TREE level.  There is another difference
>> but not regression on TREE.  Seems real change happens on RTL pre with
>> different register uses in the input.  I failed to go further or
>> extract a test case, it's just very volatile.
>
> Well, I can see what is happening and indeed we shouldn't blame the
> patch for this.
>
>> With all of this, I guess this patch shouldn't be blamed for this.  I
>> also wonder if the PR should be fixed in this way since the patch
>> definitely is a corner case.
>
> It might not fix the real issue (whatever that is), but not making
> IVOPTs (or tree-affines) life harder is very good (I believe I have
> seen this kind of issue as well).
I guess IV's base is expanded because we want to explore more CSE opportunities?
I suspect this doesn't work very well because of two reasons: 1)
overload the tree-affine facility; 2) weak IV rewriting capacity in
GCC (for example, mess up loop variant/invariant part expressions).  I
will do experiments on this.

As for the patch itself, I collected instrumental data from GCC
bootstrap and Spec2k6 compilation.  Can confirm in most cases
(bootstrap 99.9%, spec2k6 99.1%), there is only one ssa name in IV's
step.

>
> So I do think that the patch is fine.  Just seen the known-to-work GCC 3.4
> version so it's even a regression ....

Here is the refined patch according to your comments.  It passes
bootstrap and test on x86_64.

Thanks,
bin

2015-02-12  Bin Cheng  <bin.cheng@arm.com>

    PR tree-optimization/64705
    * tree-ssa-loop-niter.h (expand_simple_operations): New parameter.
    * tree-ssa-loop-niter.c (expand_simple_operations): New parameter.
    * tree-ssa-loop-ivopts.c (extract_single_var_from_expr): New.
    (find_bivs, find_givs_in_stmt_scev): Pass new argument to
    expand_simple_operations.

gcc/testsuite/ChangeLog
2015-02-12  Bin Cheng  <bin.cheng@arm.com>

    PR tree-optimization/64705
    * gcc.dg/tree-ssa/pr64705.c: New test.

Attachment: pr64705-20150212.txt
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]