Bug 92133 - Support multi versioning on self recursive function
Summary: Support multi versioning on self recursive function
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ipa (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: spec
  Show dependency treegraph
 
Reported: 2019-10-17 08:02 UTC by Feng Xue
Modified: 2019-12-03 06:15 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-10-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Feng Xue 2019-10-17 08:02:31 UTC
For recursive function, IPA does not allow constant propagation on parameter that is used to control recursion. It is common that the parameter is inc/dec (arithmetic op) a constant before entering next recursion. The following example gives a general source code pattern.

int recur_fn (int i)
{
  if (i == 6)
    {
      do_work ();
      return 0;
    }

  do_prepare ();

  recur_fn (i + 1);

  do_post ();

  return 0;
}

int foo()
{
  ...
  recur_fn (1);
  ...
}

A straight forward optimization is to duplicate recur_fn () 6 times, which constitute a calling chain on the specialized copies as:

  recur_fn<i=1>() -> recur_fn<i=2>() -> ... -> recur_fn<i=6>()
Comment 1 Martin Liška 2019-10-22 09:18:54 UTC
Let me take a look.
Comment 2 Feng Xue 2019-10-22 10:01:49 UTC
(In reply to Martin Liška from comment #1)
> Let me take a look.

I've created a patch (https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01260.html), could you take a time to review it?
Comment 3 Martin Liška 2019-10-22 10:37:17 UTC
(In reply to Feng Xue from comment #2)
> (In reply to Martin Liška from comment #1)
> > Let me take a look.
> 
> I've created a patch
> (https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01260.html), could you take a
> time to review it?

It's domain of Martin Jambor and Honza.
They will provide a review, don't worry.
Comment 4 Thomas Koenig 2019-11-03 22:34:25 UTC
Author: tkoenig
Date: Sun Nov  3 22:33:53 2019
New Revision: 277760

URL: https://gcc.gnu.org/viewcvs?rev=277760&root=gcc&view=rev
Log:
2019-11-03  Thomas Koenig  <tkoenig@gcc.gnu.org>

    PR fortran/92133
    * trans-decl.c (gfc_get_symbol_decl): If __def_init actually
    contains a value, put it into  the read-only section.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-decl.c
Comment 5 Feng Xue 2019-11-04 01:31:19 UTC
(In reply to Thomas Koenig from comment #4)
> Author: tkoenig
> Date: Sun Nov  3 22:33:53 2019
> New Revision: 277760
> 
> URL: https://gcc.gnu.org/viewcvs?rev=277760&root=gcc&view=rev
> Log:
> 2019-11-03  Thomas Koenig  <tkoenig@gcc.gnu.org>
> 
>     PR fortran/92133
>     * trans-decl.c (gfc_get_symbol_decl): If __def_init actually
>     contains a value, put it into  the read-only section.
> 
> 
> Modified:
>     trunk/gcc/fortran/ChangeLog
>     trunk/gcc/fortran/trans-decl.c

Your patch's tracker number should be 92113, not 92133.
Comment 6 Thomas Koenig 2019-11-04 07:40:58 UTC
(In reply to Feng Xue from comment #5)
> (In reply to Thomas Koenig from comment #4)
> > Author: tkoenig
> > Date: Sun Nov  3 22:33:53 2019
> > New Revision: 277760
> > 
> > URL: https://gcc.gnu.org/viewcvs?rev=277760&root=gcc&view=rev
> > Log:
> > 2019-11-03  Thomas Koenig  <tkoenig@gcc.gnu.org>
> > 
> >     PR fortran/92133
> >     * trans-decl.c (gfc_get_symbol_decl): If __def_init actually
> >     contains a value, put it into  the read-only section.
> > 
> > 
> > Modified:
> >     trunk/gcc/fortran/ChangeLog
> >     trunk/gcc/fortran/trans-decl.c
> 
> Your patch's tracker number should be 92113, not 92133.

Unfortunately, this is one of the things that can't be undone...

Fixed in the ChangeLog, and made a remark in PR 92113.
Comment 7 fxue 2019-12-02 06:38:01 UTC
Author: fxue
Date: Mon Dec  2 06:37:30 2019
New Revision: 278893

URL: https://gcc.gnu.org/viewcvs?rev=278893&root=gcc&view=rev
Log:
Enable recursive function versioning

2019-12-02  Feng Xue <fxue@os.amperecomputing.com>

        PR ipa/92133
        * doc/invoke.texi (ipa-cp-max-recursive-depth): Document new option.
        (ipa-cp-min-recursive-probability): Likewise.
        * params.opt (ipa-cp-max-recursive-depth): New.
        (ipa-cp-min-recursive-probability): Likewise.
        * ipa-cp.c (ipcp_lattice<valtype>::add_value): Add two new parameters
        val_p and unlimited.
        (self_recursively_generated_p): New function.
        (get_val_across_arith_op): Likewise.
        (propagate_vals_across_arith_jfunc): Add constant propagation for
        self-recursive function.
        (incorporate_penalties): Do not penalize pure self-recursive function.
        (good_cloning_opportunity_p): Dump node_is_self_scc flag.
        (propagate_constants_topo): Set node_is_self_scc flag for cgraph node.
        (get_info_about_necessary_edges): Relax hotness check for edge to
        self-recursive function.
        * ipa-prop.h (ipa_node_params): Add new field node_is_self_scc.

2019-12-02  Feng Xue  <fxue@os.amperecomputing.com>

        PR ipa/92133
        * gcc.dg/ipa/ipa-clone-2.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/ipa/ipa-clone-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/doc/invoke.texi
    trunk/gcc/ipa-cp.c
    trunk/gcc/ipa-prop.h
    trunk/gcc/params.opt
    trunk/gcc/testsuite/ChangeLog
Comment 8 Martin Jambor 2019-12-02 10:27:44 UTC
I believe this is now implemented, so marking as fixed.
Comment 9 Feng Xue 2019-12-03 01:49:44 UTC
Ok. For any followups on this, I'll create new tracker.
Comment 10 Xiong Hu XS Luo 2019-12-03 05:07:22 UTC
(In reply to Feng Xue from comment #9)
> Ok. For any followups on this, I'll create new tracker.

Seems "--param ipa-cp-eval-threshold=0  --param large-unit-insns=20000 -fno-inline" are required to do the recursive clone for digits_2?
Comment 11 Feng Xue 2019-12-03 06:15:44 UTC
--param ipa-cp-eval-threshold=1 -param ipcp-unit-growth=80 is enough.