Re: [PATCH] handle vla plus offset in strlen (PR 90662)

2019-06-11 Thread Jeff Law
On 6/7/19 1:12 PM, Martin Sebor wrote:
> On 6/6/19 3:53 PM, Jeff Law wrote:
>> On 6/5/19 4:51 PM, Martin Sebor wrote:
>>> One of my new tests for the strlen/sprintf integration tripped
>>> over an incomplete handling of VLAs by the strlen pass.  Where
>>> it can determine the length of a substring at some offset with
>>> other kinds of arrays, the pass fails with VLAs because they
>>> are represented as pointers to arrays.
>>>
>>> The attached patch adds the missing handling so that code like
>>> the following can be fully folded even for VLAs.
>>>
>>>    int f (int n)
>>>    {
>>>  char a[n];
>>>  strcpy (a, "12345");
>>>  if (strlen (&a[2]) != 3)
>>>    abort ();
>>>    }
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> gcc-90662.diff
>>>
>>> PR tree-optimization/90662 - strlen of a string in a vla plus offset not 
>>> folded
>>>
>>>
>>> gcc/ChangeLog:
>>>
>>> PR tree-optimization/90662
>>> * tree-ssa-strlen.c (get_stridx): Handle simple VLAs and pointers
>>> to arrays.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR tree-optimization/90662
>>> * gcc.dg/strlenopt-62.c: New test.
>>> * gcc.dg/strlenopt-63.c: New test.
>> We're relying on the fact that in a MEM_REF the offset is always a byte
>> offset, so no scaling for wchars needed, right?
> 
> I was assuming that the strlen pass only deals with narrow chars.
> But it can be called on a wide character array (or an array of
> any other type) even if it makes little sense, and the code isn't
> prepared to handle that.  Thanks for the hint!  The attached
> revision adds the scaling along with tests for non-char VLAs
> and pointer to arrays.
> 
> Martin
> 
>>
>> OK for the trunk.
>> THanks,
>> Jeff
>>
> 
> 
> gcc-90662.diff
> 
> PR tree-optimization/90662 - strlen of a string in a vla plus offset not 
> folded
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/90662
>   * tree-ssa-strlen.c (get_stridx): Handle simple VLAs and pointers
>   to arrays.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/90662
>   * gcc.dg/strlenopt-62.c: New test.
>   * gcc.dg/strlenopt-63.c: New test.
>   * gcc.dg/strlenopt-64.c: New test.
OK
jeff


Re: [PATCH] handle vla plus offset in strlen (PR 90662)

2019-06-07 Thread Martin Sebor

On 6/6/19 3:53 PM, Jeff Law wrote:

On 6/5/19 4:51 PM, Martin Sebor wrote:

One of my new tests for the strlen/sprintf integration tripped
over an incomplete handling of VLAs by the strlen pass.  Where
it can determine the length of a substring at some offset with
other kinds of arrays, the pass fails with VLAs because they
are represented as pointers to arrays.

The attached patch adds the missing handling so that code like
the following can be fully folded even for VLAs.

   int f (int n)
   {
 char a[n];
 strcpy (a, "12345");
 if (strlen (&a[2]) != 3)
   abort ();
   }

Tested on x86_64-linux.

Martin

gcc-90662.diff

PR tree-optimization/90662 - strlen of a string in a vla plus offset not folded

gcc/ChangeLog:

PR tree-optimization/90662
* tree-ssa-strlen.c (get_stridx): Handle simple VLAs and pointers
to arrays.

gcc/testsuite/ChangeLog:

PR tree-optimization/90662
* gcc.dg/strlenopt-62.c: New test.
* gcc.dg/strlenopt-63.c: New test.

We're relying on the fact that in a MEM_REF the offset is always a byte
offset, so no scaling for wchars needed, right?


I was assuming that the strlen pass only deals with narrow chars.
But it can be called on a wide character array (or an array of
any other type) even if it makes little sense, and the code isn't
prepared to handle that.  Thanks for the hint!  The attached
revision adds the scaling along with tests for non-char VLAs
and pointer to arrays.

Martin



OK for the trunk.
THanks,
Jeff



PR tree-optimization/90662 - strlen of a string in a vla plus offset not folded

gcc/ChangeLog:

	PR tree-optimization/90662
	* tree-ssa-strlen.c (get_stridx): Handle simple VLAs and pointers
	to arrays.

gcc/testsuite/ChangeLog:

	PR tree-optimization/90662
	* gcc.dg/strlenopt-62.c: New test.
	* gcc.dg/strlenopt-63.c: New test.
	* gcc.dg/strlenopt-64.c: New test.

diff --git a/gcc/testsuite/gcc.dg/strlenopt-62.c b/gcc/testsuite/gcc.dg/strlenopt-62.c
new file mode 100644
index 000..0e09a7ab0e1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-62.c
@@ -0,0 +1,189 @@
+/* PR tree-optimization/90662 - strlen of a string in a vla plus offset
+   not folded
+   { dg-do compile }
+   { dg-options "-O2 -Wall -fdump-tree-gimple -fdump-tree-optimized" } */
+
+#include "strlenopt.h"
+
+typedef __INT16_TYPE__   int16_t;
+typedef __INT32_TYPE__   int32_t;
+
+#define CONCAT(x, y) x ## y
+#define CAT(x, y) CONCAT (x, y)
+#define FAILNAME(name, counter) \
+  CAT (CAT (CAT (call_ ## name ##_on_line_, __LINE__), _), counter)
+
+#define FAIL(name, counter) do {			\
+extern void FAILNAME (name, counter) (void);	\
+FAILNAME (name, counter)();\
+  } while (0)
+
+/* Macro to emit a call to funcation named
+ call_in_true_branch_not_eliminated_on_line_NNN()
+   for each call that's expected to be eliminated.  The dg-final
+   scan-tree-dump-time directive at the bottom of the test verifies
+   that no such call appears in output.  */
+#define ELIM(expr) \
+  if (!(expr)) FAIL (in_true_branch_not_eliminated, __COUNTER__); else (void)0
+
+#define ARGS(...) __VA_ARGS__
+
+void sink (void*, ...);
+
+
+#define T(Type, expect, init, p)			\
+  do {			\
+Type vla[n];	\
+char *ptr = strcpy ((char*)vla, init);		\
+ELIM (expect == strlen (p));			\
+sink (ptr);		\
+  } while (0)
+
+void test_char_vla_local (int n)
+{
+  T (char, 0, "", vla);
+  T (char, 0, "\0", vla);
+  T (char, 1, "1", vla);
+  T (char, 2, "12", vla);
+  T (char, 3, "123", vla);
+
+  T (char, 2, "123", vla + 1);
+  T (char, 1, "123", &vla[2]);
+  T (char, 0, "123", &vla[1] + 2);
+
+  T (char, 2, "123", &vla[2] - 1);
+  T (char, 3, "123", &vla[1] - 1);
+
+  T (char, 0, "", ptr);
+  T (char, 0, "\0", ptr);
+  T (char, 1, "1", ptr);
+  T (char, 2, "12", ptr);
+  T (char, 3, "123", ptr);
+
+  T (char, 2, "123", ptr + 1);
+  T (char, 1, "123", &ptr[2]);
+  T (char, 0, "123", &ptr[1] + 2);
+}
+
+void test_int16_vla_local (int n)
+{
+  T (int16_t, 0, "", (char*)vla);
+  T (int16_t, 0, "\0", (char*)vla);
+  T (int16_t, 1, "1", (char*)vla);
+  T (int16_t, 2, "12", (char*)vla);
+  T (int16_t, 3, "123", (char*)vla);
+
+  T (int16_t, 2, "1234", (char*)(vla + 1));
+  T (int16_t, 2, "123456", (char*)&vla[2]);
+  T (int16_t, 1, "123456", (char*)&vla[2] + 1);
+  T (int16_t, 0, "123456", (char*)&vla[2] + 2);
+  T (int16_t, 0, "123456", (char*)(&vla[1] + 2));
+
+  T (int16_t, 3, "123456", (char*)&vla[2] - 1);
+  T (int16_t, 4, "123456", (char*)&vla[2] - 2);
+
+  T (int16_t, 0, "", ptr);
+  T (int16_t, 0, "\0", ptr);
+  T (int16_t, 1, "1", ptr);
+  T (int16_t, 2, "12", ptr);
+  T (int16_t, 3, "123", ptr);
+
+  T (int16_t, 2, "123", ptr + 1);
+  T (int16_t, 1, "123", &ptr[2]);
+  T (int16_t, 0, "123", &ptr[1] + 2);
+}
+
+void test_int_vla_local (int n)
+{
+  T (int16_t, 0, "", (char*)vla);
+  T (int16_t, 0, "\0", (char*)vla);
+  T (int16_t, 1, "1", (char*)vla);
+  T (int16_t, 2, "12", (char*)vla);
+  T (int16_t, 3, "123", (char*)vla);
+
+  T (i

Re: [PATCH] handle vla plus offset in strlen (PR 90662)

2019-06-06 Thread Jeff Law
On 6/5/19 4:51 PM, Martin Sebor wrote:
> One of my new tests for the strlen/sprintf integration tripped
> over an incomplete handling of VLAs by the strlen pass.  Where
> it can determine the length of a substring at some offset with
> other kinds of arrays, the pass fails with VLAs because they
> are represented as pointers to arrays.
> 
> The attached patch adds the missing handling so that code like
> the following can be fully folded even for VLAs.
> 
>   int f (int n)
>   {
> char a[n];
> strcpy (a, "12345");
> if (strlen (&a[2]) != 3)
>   abort ();
>   }
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> gcc-90662.diff
> 
> PR tree-optimization/90662 - strlen of a string in a vla plus offset not 
> folded
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/90662
>   * tree-ssa-strlen.c (get_stridx): Handle simple VLAs and pointers
>   to arrays.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/90662
>   * gcc.dg/strlenopt-62.c: New test.
>   * gcc.dg/strlenopt-63.c: New test.
We're relying on the fact that in a MEM_REF the offset is always a byte
offset, so no scaling for wchars needed, right?

OK for the trunk.
THanks,
Jeff