[PATCH] Don't assume const/pure calls are total (fix PR tree-optimization/19828)

[Zdenek Dvorak]

Hello,

> As per discussion in PR tree-optimization/19828, we should not assume
> const/pure calls are total and therefore can't hoist them out of loops
> unless we know they will be executed (at least once).
> 
> Perhaps LIM could see if they are guarded just by some loop invariant
> and in that case hoist it before the loop guarded with that invariant.

I have reached basically the same conclusion and prepared an equivalent
patch a few minutes ago; using get_call_expr_in as you do is a bit
cleaner, but I would very much like to see the comments from my patch
included in the final solution.

Zdenek

Index: tree-ssa-loop-im.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-loop-im.c,v
retrieving revision 2.27
diff -c -3 -p -r2.27 tree-ssa-loop-im.c
*** tree-ssa-loop-im.c	20 Jan 2005 12:45:13 -0000	2.27
--- tree-ssa-loop-im.c	18 Feb 2005 16:38:55 -0000
*************** Software Foundation, 59 Temple Place - S
*** 38,43 ****
--- 38,65 ----
  #include "tree-pass.h"
  #include "flags.h"
  
+ /* TODO:  Support for predicated code motion.  I.e.
+ 
+    while (1)
+      {
+        if (cond)
+ 	 {
+ 	   a = inv;
+ 	   something;
+ 	 }
+      }
+ 
+    Where COND and INV are is invariants, but evaluating inv may trap or be
+    invalid from some other reason if !COND.  This may be transformed to
+ 
+    if (cond)
+      a = inv;
+    while (1)
+      {
+        if (cond)
+ 	 something;
+      }  */
+ 
  /* A type for the list of statements that have to be moved in order to be able
     to hoist an invariant computation.  */
  
*************** movement_possibility (tree stmt)
*** 227,232 ****
--- 249,277 ----
        || tree_could_trap_p (rhs))
      return MOVE_PRESERVE_EXECUTION;
  
+   if (TREE_CODE (rhs) == CALL_EXPR)
+     {
+       /* While pure or const call is guaranteed to have no side effects, we
+ 	 cannot move it arbitrarily.  Consider code like
+ 
+ 	 char *s = something ();
+ 
+ 	 while (1)
+ 	   {
+ 	     if (s)
+ 	       t = strlen (s);
+ 	     else
+ 	       t = 0;
+ 	   }
+ 
+ 	 Here the strlen call cannot be moved out of the loop, even though
+ 	 s is invariant.  In addition to possibly creating a call with
+ 	 invalid arguments, moving out a function call that is not executed
+ 	 may cause performance regressions in case the call is costly and
+ 	 not executed at all.  */
+       return MOVE_PRESERVE_EXECUTION;
+     }
+ 
    return MOVE_POSSIBLE;
  }
  
Index: testsuite/gcc.dg/tree-ssa/loop-7.c
===================================================================
RCS file: testsuite/gcc.dg/tree-ssa/loop-7.c
diff -N testsuite/gcc.dg/tree-ssa/loop-7.c
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- testsuite/gcc.dg/tree-ssa/loop-7.c	18 Feb 2005 16:38:56 -0000
***************
*** 0 ****
--- 1,33 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O1 -fdump-tree-lim-details" } */
+ 
+ int cst_fun1(int) __attribute__((__const__));
+ int cst_fun2(int) __attribute__((__const__));
+ int pure_fun1(int) __attribute__((__pure__));
+ int pure_fun2(int) __attribute__((__pure__));
+ int foo (void);
+ 
+ int xxx(void)
+ {
+   int i, k = foo (), x = 0;
+ 
+   for (i = 0; i < 100; i++)
+     {
+       x += cst_fun1 (k);
+       x += pure_fun1 (k);
+ 
+       if (k)
+ 	{
+ 	  x += cst_fun2 (k);
+ 	  x += pure_fun2 (k);
+ 	}
+     }
+ 
+   return x;
+ }
+ 
+ /* Calls to cst_fun1 and pure_fun1 may be moved out of the loop.
+    Calls to cst_fun2 and pure_fun2 should not be, since calling
+    with k = 0 may be invalid.  */
+ 
+ /* { dg-final { scan-tree-dump-times "Moving statement" 2 "lim" } } */

> 
> Ok to commit?
> 
> 2005-02-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/19828
> 	* tree-ssa-loop-im.c (movement_possibility): Return
> 	MOVE_PRESERVE_EXECUTION for calls without side-effects.
> 
> 	* gcc.c-torture/execute/20050218-1.c: New test.
> 
> --- gcc/tree-ssa-loop-im.c.jj	2005-01-22 08:49:01.000000000 +0100
> +++ gcc/tree-ssa-loop-im.c	2005-02-18 17:21:28.794793302 +0100
> @@ -224,7 +224,8 @@ movement_possibility (tree stmt)
>      return MOVE_IMPOSSIBLE;
>  
>    if (TREE_CODE (lhs) != SSA_NAME
> -      || tree_could_trap_p (rhs))
> +      || tree_could_trap_p (rhs)
> +      || get_call_expr_in (stmt))
>      return MOVE_PRESERVE_EXECUTION;
>  
>    return MOVE_POSSIBLE;
> --- gcc/testsuite/gcc.c-torture/execute/20050218-1.c.jj	2005-01-27 14:27:08.338732320 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/20050218-1.c	2005-02-18 17:24:35.316666039 +0100
> @@ -0,0 +1,30 @@
> +/* PR tree-optimization/19828 */
> +typedef __SIZE_TYPE__ size_t;
> +extern size_t strlen (const char *s);
> +extern int strncmp (const char *s1, const char *s2, size_t n);
> +extern void abort (void);
> +
> +const char *a[16] = { "a", "bc", "de", "fgh" };
> +
> +int
> +foo (char *x, const char *y, size_t n)
> +{
> +  size_t i, j = 0;
> +  for (i = 0; i < n; i++)
> +    {
> +      if (strncmp (x + j, a[i], strlen (a[i])) != 0)
> +        return 2;
> +      j += strlen (a[i]);
> +      if (y)
> +        j += strlen (y);
> +    }
> +  return 0;
> +}
> +
> +int
> +main (void)
> +{
> +  if (foo ("abcde", (const char *) 0, 3) != 0)
> +    abort ();
> +  return 0;
> +}
> 
> 	Jakub