Created attachment 53646 [details] original test case by supercat The attached test case is from user supercat on Stack Overflow (original source: https://stackoverflow.com/questions/42178179/will-casting-around-sockaddr-storage-and-sockaddr-in-break-strict-aliasing/42178347?noredirect=1#comment130509588_42178347, https://godbolt.org/z/83v4ssrn4) and demonstrates wrong TBAA apparently assuming an object of type long long was not modified after the code path modifying it was collapsed with a different code path performing the modification via an lvalue of type long. On 64-bit targets, the test program outputs 1/2 with optimization levels that enable -fstrict-aliasing. The expected output is 2/2. Using -fno-strict-aliasing fixes it. I have not checked this myself, but according to others who have looked at the test case, the regression came between GCC 4.7 and 4.8.
There's also a potentially related test case at https://godbolt.org/z/jfv1Ge6v4 - I'm not yet clear on whether it's likely to have the same root cause.
Confirmed. I don't think it is exactly TBAA which is causing the issue but rather the way PRE does aliasing walks. Take: ``` static inline void set_longish(int is_long, void *p, long x) { if (is_long) *(long*)p = x; else *(long long*)p = x; } static long test(long long *p, int index, int mode) { *p = 1; set_longish(mode, p+index, 2); return *p; } long (*volatile vtest)(long long*, int, int) = test; #include <stdio.h> int main(void) { long long x; long result = vtest(&x, 0, 0); printf("%lu/%llu\n", result, x); } ``` Which is only difference by which order the if statement (and mode which is swapped). This case works.
Here is a testcase which has failed since before 4.1.2: ``` static inline void set_longish(int is_long_long, void *p, long x) { if (is_long_long) *(long long*)p = x; else *(long*)p = x; } static long test(long long *p, int index, int mode) { *p = 1; set_longish(mode, p+index, 2); return *p; } long (*volatile vtest)(long long*, int, int) = test; #include <stdio.h> int main(void) { long long x; long result = vtest(&x, 0, 1); printf("%lu/%llu\n", result, x); } ``` The only difference from the original testcase is marking set_longish as static inline. I suspect what changed between 4.7 and 4.8 for the original testcase is inlining.
Note this has been to be a regression since tree level PRE is what is causing the issue and that was added in GCC 4 or 4.1.
(In reply to Rich Felker from comment #1) > There's also a potentially related test case at > https://godbolt.org/z/jfv1Ge6v4 - I'm not yet clear on whether it's likely > to have the same root cause. This might be a different issue I think.
(In reply to Andrew Pinski from comment #5) > (In reply to Rich Felker from comment #1) > > There's also a potentially related test case at > > https://godbolt.org/z/jfv1Ge6v4 - I'm not yet clear on whether it's likely > > to have the same root cause. > > This might be a different issue I think. Yeah, that's sched2 reordering the accesses (probably cselib is confused). Needs a separate report.
Second one filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107115
I will have a look.
The issue is how we value-number for tail-merging. We do Processing block 1: BB4 Value numbering stmt = MEM[(long int *)_3] = 2; RHS 2 simplified to 2 No store match Value numbering store MEM[(long int *)_3] to 2 Setting value number of .MEM_12 to .MEM_12 (changed) marking outgoing edge 4 -> 5 executable Processing block 2: BB3 Value numbering stmt = *_3 = 2; RHS 2 simplified to 2 Setting value number of .MEM_11 to .MEM_12 (changed) marking outgoing edge 3 -> 5 executable Processing block 3: BB5 Value numbering stmt = .MEM_10 = PHI <.MEM_11(3), .MEM_12(4)> Setting value number of .MEM_10 to .MEM_12 (changed) Value numbering stmt = _9 = *p_5(D); Setting value number of _9 to 1 (changed) oops. So we figure that the two stores from '2' are "redundant" which causes us to only consider one (the wrong one) when later looking up *p_5(D). That's if (!resultsame) { /* Only perform the following when being called from PRE which embeds tail merging. */ if (default_vn_walk_kind == VN_WALK) { assign = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, op); vn_reference_lookup (assign, vuse, VN_NOWALK, &vnresult, false); if (vnresult) { VN_INFO (vdef)->visited = true; return set_ssa_val_to (vdef, vnresult->result_vdef); } that's a case I never understood fully (and that seems wrong). That visited flag set is also odd. I'm testing a patch.
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:85333b9265720fc4e49397301cb16324d2b89aa7 commit r13-3126-g85333b9265720fc4e49397301cb16324d2b89aa7 Author: Richard Biener <rguenther@suse.de> Date: Thu Oct 6 11:20:16 2022 +0200 tree-optimization/107107 - tail-merging VN wrong-code The following fixes an unintended(?) side-effect of the special MODIFY_EXPR expression entries we add for tail-merging during VN. We shouldn't value-number the virtual operand differently here. PR tree-optimization/107107 * tree-ssa-sccvn.cc (visit_reference_op_store): Do not affect value-numbering when doing the tail merging MODIFY_EXPR lookup. * gcc.dg/pr107107.c: New testcase.
Fixed on trunk sofar.
The releases/gcc-12 branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:ff0a274e5c3026b105c7f51126fa51f8178fa42c commit r12-8838-gff0a274e5c3026b105c7f51126fa51f8178fa42c Author: Richard Biener <rguenther@suse.de> Date: Thu Oct 6 11:20:16 2022 +0200 tree-optimization/107107 - tail-merging VN wrong-code The following fixes an unintended(?) side-effect of the special MODIFY_EXPR expression entries we add for tail-merging during VN. We shouldn't value-number the virtual operand differently here. PR tree-optimization/107107 * tree-ssa-sccvn.cc (visit_reference_op_store): Do not affect value-numbering when doing the tail merging MODIFY_EXPR lookup. * gcc.dg/pr107107.c: New testcase. (cherry picked from commit 85333b9265720fc4e49397301cb16324d2b89aa7)
The releases/gcc-11 branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:fa11fc62ddee81a8bc3e69d5e3180695a6dbb666 commit r11-10417-gfa11fc62ddee81a8bc3e69d5e3180695a6dbb666 Author: Richard Biener <rguenther@suse.de> Date: Thu Oct 6 11:20:16 2022 +0200 tree-optimization/107107 - tail-merging VN wrong-code The following fixes an unintended(?) side-effect of the special MODIFY_EXPR expression entries we add for tail-merging during VN. We shouldn't value-number the virtual operand differently here. PR tree-optimization/107107 * tree-ssa-sccvn.c (visit_reference_op_store): Do not affect value-numbering when doing the tail merging MODIFY_EXPR lookup. * gcc.dg/pr107107.c: New testcase. (cherry picked from commit 85333b9265720fc4e49397301cb16324d2b89aa7)
The releases/gcc-10 branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:58e39fcaaf298ff54b6f1a45fa9d15390e8113fb commit r10-11177-g58e39fcaaf298ff54b6f1a45fa9d15390e8113fb Author: Richard Biener <rguenther@suse.de> Date: Thu Oct 6 11:20:16 2022 +0200 tree-optimization/107107 - tail-merging VN wrong-code The following fixes an unintended(?) side-effect of the special MODIFY_EXPR expression entries we add for tail-merging during VN. We shouldn't value-number the virtual operand differently here. PR tree-optimization/107107 * tree-ssa-sccvn.c (visit_reference_op_store): Do not affect value-numbering when doing the tail merging MODIFY_EXPR lookup. * gcc.dg/pr107107.c: New testcase. (cherry picked from commit 85333b9265720fc4e49397301cb16324d2b89aa7)
Fixed.