On Thu, 23 Nov 2017, Jakub Jelinek wrote: > On Thu, Nov 23, 2017 at 11:30:22AM +0100, Richard Biener wrote: > > FAIL: gcc.c-torture/execute/builtins/memcpy-chk.c execution, -O2 > > ... > > FAIL: gcc.c-torture/execute/builtins/memmove-chk.c execution, -O2 > > ... > > FAIL: gcc.c-torture/execute/builtins/strncat-chk.c execution, -O2 > > ... > > FAIL: gcc.dg/builtin-object-size-4.c execution test > > > > where I looked into the object-size case. For > > > > r = &a.a[4]; > > r = memset (r, 'a', 2); > > if (__builtin_object_size (r, 3) != sizeof (a.a) - 4) > > abort (); > > r = memset (r + 2, 'b', 2) + 2; > > if (__builtin_object_size (r, 3) != sizeof (a.a) - 8) > > abort (); > > > > FRE now propagates &a.a[4] + CST adjustments through the memsets > > and constant folds the last __builtin_object_size check to false. > > That is because it tests a [13] mode which as you know is fragile and > doesn't like any kind of folding into MEM_REFs where the original access > path (&a.a[4] + 4) is lost. So, if it is match.pd that does this, whatever is > doing it there should be also guarded with cfun->after_inlining. > While mode 3 is probably never used in real-world, mode 1 is used a lot and > so we shouldn't regress that.
Hum. But that pessimizes a _lot_ of early folding. Like we'd no longer optimize r = &a.a[4]; r = r + 1; if (r != &a.a[0]) and similar stuff exposed a lot by C++ abstraction. We really only want to avoid elimination in certain places. Even the current stop-gap (that isn't really one) is bad for optimization. > BTW, seems pass_through_call could now use ERF_RETURNS_ARG stuff > except for __builtin_assume_aligned which intentionally doesn't have > ret1 because we really don't want to "optimize" it by propagating the > first argument to its uses too early. I'll handle it. Thanks (double-check all those builtins have ret1). > Though, with your changes perhaps there won't be any pass through calls > anymore except for __builtin_assume_aligned. For the object-size pass before FRE there certainly would be. So I have the following testsuite adjustment for the builtins.exp FAILs. Two of them quite obvious - just hide the conditional equivalence we exposed. The strncat-chk.c is a little less obvious. We have const char *const s1 = "hello world"; char dst[64], *d2; ... /* These __strncat_chk calls should be optimized into __strcat_chk, as strlen (src) <= len. */ strcpy (dst, s1); if (strncat (dst, "foo", 3) != dst || strcmp (dst, "hello worldfoo")) abort (); strcpy (dst, s1); if (strncat (dst, "foo", 100) != dst || strcmp (dst, "hello worldfoo")) abort (); strcpy (dst, s1); if (strncat (dst, s1, 100) != dst || strcmp (dst, "hello worldhello world")) abort (); if (chk_calls != 3) abort (); so it's obvious we can optimize away all checking given the strlen pass ought to be able to track the length of dst. Somehow we currently don't but with the patch we do (but not at -O[1sg]). Looks like a missed optimization to me (currently). I couldn't figure any way to remove that optimization possibility while preserving strlen (src) <= len knowledge. Like hiding 's1' because that makes the strcpy do a chk and thus we get 6 checks (that I could have consistently at all opt levels). Any preference for strncat-chk.c? Thanks, Richard. 2017-11-23 Richard Biener <rguent...@suse.de> * gcc.c-torture/execute/builtins/memcpy-chk.c: Hide established conditional equivalence of buf3 and buf1. * gcc.c-torture/execute/builtins/memmove-chk.c: Likewise. * gcc.c-torture/execute/builtins/strncat-chk.c: Allow for optimizing of chk calls. Index: gcc/testsuite/gcc.c-torture/execute/builtins/memcpy-chk.c =================================================================== --- gcc/testsuite/gcc.c-torture/execute/builtins/memcpy-chk.c (revision 255093) +++ gcc/testsuite/gcc.c-torture/execute/builtins/memcpy-chk.c (working copy) @@ -146,6 +146,7 @@ test2_sub (long *buf3, char *buf4, char call. */ /* buf3 points to an unknown object, so __memcpy_chk should not be done. */ + __asm ("" : "=r" (buf3) : "0" (buf3)); if (memcpy ((char *) buf3 + 4, buf5, n + 6) != (char *) buf1 + 4 || memcmp (buf1, "aBcdRSTUVWklmnopq\0", 19)) abort (); Index: gcc/testsuite/gcc.c-torture/execute/builtins/memmove-chk.c =================================================================== --- gcc/testsuite/gcc.c-torture/execute/builtins/memmove-chk.c (revision 255093) +++ gcc/testsuite/gcc.c-torture/execute/builtins/memmove-chk.c (working copy) @@ -149,6 +149,7 @@ test2_sub (long *buf3, char *buf4, char call. */ /* buf3 points to an unknown object, so __memmove_chk should not be done. */ + __asm ("" : "=r" (buf3) : "0" (buf3)); if (memmove ((char *) buf3 + 4, buf5, n + 6) != (char *) buf1 + 4 || memcmp (buf1, "aBcdRSTUVWklmnopq\0", 19)) abort (); Index: gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c =================================================================== --- gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (revision 255093) +++ gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (working copy) @@ -78,7 +78,9 @@ test1 (void) strcpy (dst, s1); if (strncat (dst, s1, 100) != dst || strcmp (dst, "hello worldhello world")) abort (); - if (chk_calls != 3) + /* With enough optimization we can track the string length in dst, + currently at -O2+ but not -O[1sg]. */ + if (chk_calls != 0 && chk_calls != 3) abort (); chk_calls = 0; Index: gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c =================================================================== --- gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (revision 255093) +++ gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (working copy) @@ -69,16 +69,18 @@ test1 (void) /* These __strncat_chk calls should be optimized into __strcat_chk, as strlen (src) <= len. */ - strcpy (dst, s1); + const char *s1p; + __asm ("" : "=r" (s1p) : "0" (s1)); + strcpy (dst, s1p); if (strncat (dst, "foo", 3) != dst || strcmp (dst, "hello worldfoo")) abort (); - strcpy (dst, s1); + strcpy (dst, s1p); if (strncat (dst, "foo", 100) != dst || strcmp (dst, "hello worldfoo")) abort (); - strcpy (dst, s1); + strcpy (dst, s1p); if (strncat (dst, s1, 100) != dst || strcmp (dst, "hello worldhello world")) abort (); - if (chk_calls != 3) + if (chk_calls != 6) abort (); chk_calls = 0;