Fix an omission in the recent strlen optimization (PR tree-optimization/92056)
diff mbox series

Message ID 20191017083536.GZ2116@tucnak
State New
Headers show
Series
  • Fix an omission in the recent strlen optimization (PR tree-optimization/92056)
Related show

Commit Message

Jakub Jelinek Oct. 17, 2019, 8:35 a.m. UTC
Hi!

objsz computation has two modes.  One is a cheap one that doesn't handle
SSA_NAMEs and is used in say random builtin folding.  The other is
where compute_builtin_object_size is called in between init_object_sizes ()
and fini_object_sizes () calls, where those set up data structures and the
call then handles SSA_NAMEs and caches results for them.  This second mode
is what the objsz pass uses, and in some cases the strlen pass too, but in
other cases it doesn't.  While fini_object_sizes (); is called
unconditionally at the end of strlen pass, init_object_sizes () is only
called when the strlen pass calls handle_printf_call which calls
get_destination_size; after that, any strcmp etc. takes advantage of that,
but if no *printf is encountered, it will not.  Note, init_object_sizes ()
can be called multiple times and does nothing the second and following time,
unless fini_object_sizes () has been called.  And fini_object_sizes () can
be called multiple times and doesn't do anything if since the last
fini_object_sizes () no init_object_sizes () has been called.

So, on the following testcase without the patch, we set the value range
of the first strcmp call to ~[0, 0], because we determine the buffer holding
the first operand is at most 7 bytes long, but the second operand is a
string literal with 7 characters + terminating NUL, but on the second call
we don't, because no sprintf has been called in the function (and processed
before the call).

Fixed thusly, ok for trunk if it passes bootstrap/regtest?

2019-10-17  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/92056
	* tree-ssa-strlen.c (determine_min_objsize): Call init_object_sizes
	before calling compute_builtin_object_size.

	* gcc.dg/tree-ssa/pr92056.c: New test.


	Jakub

Comments

Richard Biener Oct. 17, 2019, 9:14 a.m. UTC | #1
On Thu, 17 Oct 2019, Jakub Jelinek wrote:

> Hi!
> 
> objsz computation has two modes.  One is a cheap one that doesn't handle
> SSA_NAMEs and is used in say random builtin folding.  The other is
> where compute_builtin_object_size is called in between init_object_sizes ()
> and fini_object_sizes () calls, where those set up data structures and the
> call then handles SSA_NAMEs and caches results for them.  This second mode
> is what the objsz pass uses, and in some cases the strlen pass too, but in
> other cases it doesn't.  While fini_object_sizes (); is called
> unconditionally at the end of strlen pass, init_object_sizes () is only
> called when the strlen pass calls handle_printf_call which calls
> get_destination_size; after that, any strcmp etc. takes advantage of that,
> but if no *printf is encountered, it will not.  Note, init_object_sizes ()
> can be called multiple times and does nothing the second and following time,
> unless fini_object_sizes () has been called.  And fini_object_sizes () can
> be called multiple times and doesn't do anything if since the last
> fini_object_sizes () no init_object_sizes () has been called.
> 
> So, on the following testcase without the patch, we set the value range
> of the first strcmp call to ~[0, 0], because we determine the buffer holding
> the first operand is at most 7 bytes long, but the second operand is a
> string literal with 7 characters + terminating NUL, but on the second call
> we don't, because no sprintf has been called in the function (and processed
> before the call).
> 
> Fixed thusly, ok for trunk if it passes bootstrap/regtest?

OK.

Richard.

> 2019-10-17  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/92056
> 	* tree-ssa-strlen.c (determine_min_objsize): Call init_object_sizes
> 	before calling compute_builtin_object_size.
> 
> 	* gcc.dg/tree-ssa/pr92056.c: New test.
> 
> --- gcc/tree-ssa-strlen.c.jj	2019-10-17 00:18:09.851648007 +0200
> +++ gcc/tree-ssa-strlen.c	2019-10-17 10:19:19.546086865 +0200
> @@ -3462,6 +3462,8 @@ determine_min_objsize (tree dest)
>  {
>    unsigned HOST_WIDE_INT size = 0;
>  
> +  init_object_sizes ();
> +
>    if (compute_builtin_object_size (dest, 2, &size))
>      return size;
>  
> --- gcc/testsuite/gcc.dg/tree-ssa/pr92056.c.jj	2019-10-17 10:18:25.819907087 +0200
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr92056.c	2019-10-17 10:17:56.201359262 +0200
> @@ -0,0 +1,36 @@
> +/* PR tree-optimization/92056 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "return 1;" 2 "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "strcmp \\(" "optimized" } } */
> +
> +void bar (int, char *);
> +
> +int
> +foo (int x, char *y, const char *z)
> +{
> +  char *a;
> +  __builtin_sprintf (y, z);
> +  if (x == 3)
> +    a = __builtin_malloc (5);
> +  else if (x == 7)
> +    a = __builtin_malloc (6);
> +  else
> +    a = __builtin_malloc (7);
> +  bar (x, a);
> +  return __builtin_strcmp (a, "abcdefg") != 0;
> +}
> +
> +int
> +baz (int x)
> +{
> +  char *a;
> +  if (x == 3)
> +    a = __builtin_malloc (5);
> +  else if (x == 7)
> +    a = __builtin_malloc (6);
> +  else
> +    a = __builtin_malloc (7);
> +  bar (x, a);
> +  return __builtin_strcmp (a, "abcdefg") != 0;
> +}
> 
> 	Jakub
>

Patch
diff mbox series

--- gcc/tree-ssa-strlen.c.jj	2019-10-17 00:18:09.851648007 +0200
+++ gcc/tree-ssa-strlen.c	2019-10-17 10:19:19.546086865 +0200
@@ -3462,6 +3462,8 @@  determine_min_objsize (tree dest)
 {
   unsigned HOST_WIDE_INT size = 0;
 
+  init_object_sizes ();
+
   if (compute_builtin_object_size (dest, 2, &size))
     return size;
 
--- gcc/testsuite/gcc.dg/tree-ssa/pr92056.c.jj	2019-10-17 10:18:25.819907087 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/pr92056.c	2019-10-17 10:17:56.201359262 +0200
@@ -0,0 +1,36 @@ 
+/* PR tree-optimization/92056 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "return 1;" 2 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "strcmp \\(" "optimized" } } */
+
+void bar (int, char *);
+
+int
+foo (int x, char *y, const char *z)
+{
+  char *a;
+  __builtin_sprintf (y, z);
+  if (x == 3)
+    a = __builtin_malloc (5);
+  else if (x == 7)
+    a = __builtin_malloc (6);
+  else
+    a = __builtin_malloc (7);
+  bar (x, a);
+  return __builtin_strcmp (a, "abcdefg") != 0;
+}
+
+int
+baz (int x)
+{
+  char *a;
+  if (x == 3)
+    a = __builtin_malloc (5);
+  else if (x == 7)
+    a = __builtin_malloc (6);
+  else
+    a = __builtin_malloc (7);
+  bar (x, a);
+  return __builtin_strcmp (a, "abcdefg") != 0;
+}