Fix recursion discovery in ipa-pure-const

Message ID 20211111134037.GF17431@kam.mff.cuni.cz
State Committed
Commit 6e30c48120500ef2e8643a7574636ed02567dbb6
Headers
Series Fix recursion discovery in ipa-pure-const |

Commit Message

Jan Hubicka Nov. 11, 2021, 1:40 p.m. UTC
  Hi,
We make self recursive functions as looping of fear of endless recursion.
This is done correctly for local pure/const and for non-trivial SCCs in
callgraph, but for trivial SCCs we miss the flag.

I think it is bad decision since infinite recursion will run out of stack,
but changing it upsets some testcases and should be done independently.
So this patch is fixing current behaviour to be consistent.

Bootstrapped/regtested x86_64-linux, comitted.

gcc/ChangeLog:

2021-11-11  Jan Hubicka  <hubicka@ucw.cz>

	* ipa-pure-const.c (propagate_pure_const): Self recursion is
	a side effects.
  

Comments

Richard Biener Nov. 11, 2021, 2:30 p.m. UTC | #1
On Thu, Nov 11, 2021 at 2:41 PM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
> We make self recursive functions as looping of fear of endless recursion.
> This is done correctly for local pure/const and for non-trivial SCCs in
> callgraph, but for trivial SCCs we miss the flag.
>
> I think it is bad decision since infinite recursion will run out of stack,

Note it might not always in case we can eliminate the tail-recursion or avoid
stack use by the recursion by other means.  So I think it is conservatively
correct.

Richard.

> but changing it upsets some testcases and should be done independently.
> So this patch is fixing current behaviour to be consistent.
>
> Bootstrapped/regtested x86_64-linux, comitted.
>
> gcc/ChangeLog:
>
> 2021-11-11  Jan Hubicka  <hubicka@ucw.cz>
>
>         * ipa-pure-const.c (propagate_pure_const): Self recursion is
>         a side effects.
>
> diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
> index 505ed4f8a3b..64777cd2d91 100644
> --- a/gcc/ipa-pure-const.c
> +++ b/gcc/ipa-pure-const.c
> @@ -1513,6 +1611,9 @@ propagate_pure_const (void)
>               enum pure_const_state_e edge_state = IPA_CONST;
>               bool edge_looping = false;
>
> +             if (e->recursive_p ())
> +               looping = true;
> +
>               if (dump_file && (dump_flags & TDF_DETAILS))
>                 {
>                   fprintf (dump_file, "    Call to %s",
  
Jan Hubicka Nov. 11, 2021, 2:53 p.m. UTC | #2
> On Thu, Nov 11, 2021 at 2:41 PM Jan Hubicka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi,
> > We make self recursive functions as looping of fear of endless recursion.
> > This is done correctly for local pure/const and for non-trivial SCCs in
> > callgraph, but for trivial SCCs we miss the flag.
> >
> > I think it is bad decision since infinite recursion will run out of stack,
> 
> Note it might not always in case we can eliminate the tail-recursion or avoid
> stack use by the recursion by other means.  So I think it is conservatively
> correct.

I don't know.  If function is pure and has infinite recursion in it it
means that it can only run forever without side effects if it gets lucky
and we tail-recurse it.  There are no other means avoid the stack use from
growing.

First i think code relying on tail-recurse optimization to not run out
of stack is not strictly valid in C/C++ other languages we care.
Also in C++ there is the forced progression which makes even the tail
optiimzed code invalid.

I think in high level code such recursive accessors used for no good
reason are not that infrequent.  Also we had this bug in tree probably
forever since LOOPING_PURE_CONST was added and no one complained ;)

Relaxing this rule breaks some testcases, but odd ones - they are
infinitely self-recursive builtin implementations where we then both
prove function as noreturn & later optimize builtin to constant
so the assembly matching does not see expected thing.

Honza
  

Patch

diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index 505ed4f8a3b..64777cd2d91 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1513,6 +1611,9 @@  propagate_pure_const (void)
 	      enum pure_const_state_e edge_state = IPA_CONST;
 	      bool edge_looping = false;
 
+	      if (e->recursive_p ())
+		looping = true;
+
 	      if (dump_file && (dump_flags & TDF_DETAILS))
 		{
 		  fprintf (dump_file, "    Call to %s",