[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 Martin Sebor changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #8 from Martin Sebor --- Adjusted patch committed in r263018.
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 --- Comment #7 from Martin Sebor --- Author: msebor Date: Thu Jul 26 16:45:43 2018 New Revision: 263018 URL: https://gcc.gnu.org/viewcvs?rev=263018=gcc=rev Log: PR tree-optimization/86043 - strlen after memcpy partially overwriting a string not optimized PR tree-optimization/86042 - missing strlen optimization after second strcpy gcc/ChangeLog: PR tree-optimization/86043 PR tree-optimization/86042 * tree-ssa-strlen.c (handle_builtin_memcpy): Handle strict overlaps. (get_string_cst_length): Rename... (get_min_string_length): ...to this. Add argument. (handle_char_store): Extend to handle multi-character stores by MEM_REF. * tree.c (initializer_zerop): Use new argument. Handle MEM_REF. * tree.h (initializer_zerop): Add argument. gcc/testsuite/ChangeLog: PR tree-optimization/86043 PR tree-optimization/86042 * gcc/testsuite/gcc.dg/attr-nonstring-2.c: Xfail test cases due to pr86688. * gcc.dg/strlenopt-44.c: New test. Added: trunk/gcc/testsuite/gcc.dg/strlenopt-54.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/attr-nonstring-2.c trunk/gcc/tree-ssa-strlen.c trunk/gcc/tree.c trunk/gcc/tree.h
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 Jakub Jelinek changed: What|Removed |Added Target Milestone|8.2 |8.3 --- Comment #6 from Jakub Jelinek --- GCC 8.2 has been released.
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 Martin Sebor changed: What|Removed |Added Keywords||patch --- Comment #5 from Martin Sebor --- Patch: https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00403.html
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042
--- Comment #4 from rguenther at suse dot de ---
On Tue, 5 Jun 2018, msebor at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042
>
> --- Comment #3 from Martin Sebor ---
> The strcpy() calls are first transformed into
>
> MEM[(char * {ref-all})] = MEM[(char * {ref-all})"12"];
>
> In GCC 7, the above is then transformed into
>
> MEM[(char * {ref-all})] = "12";
>
> (I'm not sure what the difference is).
"12" is considered a constant while MEM[(char * {ref-all})"12"]
is considered a read from the constant pool.
It's not simplified to that because GCC 8 uses 'unsigned char[]'
as access type while GCC 7 used char[] and those are not compatible.
I suspect if we change
/* We get an aggregate copy. Use an unsigned char[] type to
perform the copying to preserve padding and to avoid any issues
with TREE_ADDRESSABLE types or float modes behavior on copying.
*/
desttype = build_array_type_nelts (unsigned_char_type_node,
tree_to_uhwi (len));
in gimple_fold_builtin_memory_op to use char_type_node then we'll
get back GCC 7 behavior for this case. (I chose unsigned char type
to not change IL based on -f[un]signed-char)
I suspect that with -funsigned-char the testcase already works with GCC8?
" In GCC 7, the second instance of the
> above is then removed in fre1.
By means of redundant store removal which ATM only handles stores from
constants and registers but not aggregate copies.
> In GCC 8, the second instance makes it all the way to the strlen pass where
> handle_char_store() isn't prepared to deal with it if a length record exists
> for the destination. I think the strlen() limitation can be handled by the
> same solution as bug 86043: i.e., have handle_char_store() handle cases where
> substrings of any length is overwritten without changing their length, not
> just
> those of length one by plain character assignment.
Yes, that's a good enhancement independent of this bug.
> I don't know why the duplicate MEM assignment above isn't eliminated earlier
> (that may be a separate bug).
See above - redundant store removal doesn't handle aggregate copies.
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 Martin Sebor changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot gnu.org
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042
--- Comment #3 from Martin Sebor ---
The strcpy() calls are first transformed into
MEM[(char * {ref-all})] = MEM[(char * {ref-all})"12"];
In GCC 7, the above is then transformed into
MEM[(char * {ref-all})] = "12";
(I'm not sure what the difference is). In GCC 7, the second instance of the
above is then removed in fre1.
In GCC 8, the second instance makes it all the way to the strlen pass where
handle_char_store() isn't prepared to deal with it if a length record exists
for the destination. I think the strlen() limitation can be handled by the
same solution as bug 86043: i.e., have handle_char_store() handle cases where
substrings of any length is overwritten without changing their length, not just
those of length one by plain character assignment.
I don't know why the duplicate MEM assignment above isn't eliminated earlier
(that may be a separate bug).
[Bug tree-optimization/86042] [8/9 Regression] missing strlen optimization after second strcpy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 Richard Biener changed: What|Removed |Added Priority|P3 |P2 Status|UNCONFIRMED |NEW Last reconfirmed||2018-06-05 CC||jakub at gcc dot gnu.org, ||rguenth at gcc dot gnu.org Target Milestone|--- |8.2 Ever confirmed|0 |1 --- Comment #2 from Richard Biener --- Confirmed. Note this was a correctness fix so we properly copy padding given aggregate assignment in GIMPLE is not a block copy if done for a structure type. For this case in question it shouldn't have made a difference though. We seem to invalidate the string info for some reason after visiting the second store.
