[Bug rtl-optimization/41033] RTL alias-oracle does not honor -fno-strict-aliasing
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41033 --- Comment #6 from GCC Commits --- The master branch has been updated by Sam James : https://gcc.gnu.org/g:a75c6295252d0d998a18927dc7510fac965134c4 commit r15-2349-ga75c6295252d0d998a18927dc7510fac965134c4 Author: Sam James Date: Thu Jul 18 08:27:29 2024 +0100 testsuite: Add dg-do run to even more tests All of these are for wrong-code bugs. Confirmed to be used before but with no execution. Tested on x86_64-pc-linux-gnu and checked test logs before/after. 2024-07-26 Sam James PR target/7559 PR c++/9704 PR c++/16115 PR c++/19317 PR rtl-optimization/11536 PR target/20322 PR tree-optimization/31966 PR rtl-optimization/41033 PR tree-optimization/67947 * g++.dg/cpp1z/byte1.C: Add dg-do run directive. * g++.dg/init/call1.C: Ditto. * g++.dg/init/copy5.C: Ditto. * g++.dg/opt/nrv9.C: Ditto. * gcc.dg/20021006-1.c: Ditto. * gcc.dg/20030721-1.c: Ditto. * gcc.dg/20050307-1.c: Ditto. * gcc.dg/pr41033.c: Ditto. * gcc.dg/torture/pr67947.c: Ditto. * gcc.dg/tree-ssa/pr31966.c: Ditto. * gcc.dg/tree-ssa/tailcall-3.c: Ditto. * gcc.dg/tree-ssa/vrp74.c: Ditto. * gcc.target/nvptx/abort.c: Fix whitespace in dg directive.
[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 --- Comment #5 from Xi Ruoyao --- *** Bug 116066 has been marked as a duplicate of this bug. ***
[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 Jeffrey A. Law changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #4 from Jeffrey A. Law --- Fixed on the trunk.
[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 --- Comment #3 from GCC Commits --- The master branch has been updated by Jeff Law : https://gcc.gnu.org/g:34fb0feca71f763b2fbe832548749666d34a4a76 commit r15-2321-g34fb0feca71f763b2fbe832548749666d34a4a76 Author: Jeff Law Date: Thu Jul 25 12:32:28 2024 -0600 [PR rtl-optimization/116039] Fix life computation for promoted subregs So this turned out to be a neat little test and while the fuzzer found it on RISC-V, I wouldn't be surprised if the underlying issue is also the root cause of the loongarch issue with ext-dce. The key issue is that if we have something like (set (dest) (any_extend (subreg (source If the subreg object is marked with SUBREG_PROMOTED and the sign/unsigned state matches the any_extend opcode, then combine (and I guess anything using simplify-rtx) may simplify that to (set (dest) (source)) That implies that bits outside the mode of the subreg are actually live and valid. This needs to be accounted for during liveness computation. We have to be careful here though. If we're too conservative about setting additional bits live, then we'll inhibit the desired optimization in the coremark examples. To do a good job we need to know the extension opcode. I'm extremely unhappy with how the use handling works in ext-dce. It mixes different conceptual steps and has horribly complex control flow. It only handles a subset of the unary/binary opcodes, etc etc. It's just damn mess. It's going to need some more noodling around. In the mean time this is a bit hacky in that it depends on non-obvious behavior to know it can get the extension opcode, but I don't want to leave the trunk in a broken state while I figure out the refactoring problem. Bootstrapped and regression tested on x86 and tested on the crosses. Pushing to the trunk. PR rtl-optimization/116039 gcc/ * ext-dce.cc (ext_dce_process_uses): Add some comments about concerns with current code. Mark additional bit groups as live when we have an extension of a suitably promoted subreg. gcc/testsuite * gcc.dg/torture/pr116039.c: New test.
[Bug c++/115658] char16_t and char32_t aliasing is conserative
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658 Sam James changed: What|Removed |Added CC||jason at gcc dot gnu.org Ever confirmed|0 |1 Last reconfirmed||2024-07-25 Status|UNCONFIRMED |NEW
[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 --- Comment #2 from Jeffrey A. Law --- Very interesting little testcase. This may be the loongarch bug that was recently reported. It appears the root cause is this insn (from a hacked up version, so the insn #s may not match up perfectly): (insn 82 79 83 4 (set (reg:DI 246 [ _123 ]) (sign_extend:DI (subreg/s/u:QI (reg:DI 260) 0))) "j.c":11:11 128 {*extendqidi2} (expr_list:REG_DEAD (reg:DI 260) (expr_list:REG_EQUAL (sign_extend:DI (mem/c:QI (const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR0") [flags 0x182]) (const_int 57 [0x39]))) [0 d[9]+0 S1 A8])) Note the /s/u flags on the SUBREG. Those tell the optimizers that the value is actually a sign extended object already and the optimizers can drop the extraneous extension. In a subtle way those flags essentially mean that bits outside the subreg's mode are live. So we really should have marked additional groups in (reg 260) as live. That in turn would have kept key bit groups live in a different register and inhibited extension removal. There's a nonzero chance this is also the loongarch bug that just got reported.
[Bug tree-optimization/116034] [12/13 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #13 from GCC Commits --- The releases/gcc-14 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:181f40f5cf8510a16191e4768dadbe2cb7a5c095 commit r14-10507-g181f40f5cf8510a16191e4768dadbe2cb7a5c095 Author: Jakub Jelinek Date: Wed Jul 24 18:00:05 2024 +0200 testsuite: Fix up pr116034.c test for big/pdp endian [PR116061] Didn't notice the memmove is into an int variable, so the test was still failing on big endian. 2024-07-24 Jakub Jelinek PR tree-optimization/116034 PR testsuite/116061 * gcc.dg/pr116034.c (g): Change type from int to unsigned short. (foo): Guard memmove call on __SIZEOF_SHORT__ == 2. (cherry picked from commit 69e69847e21a8d951ab5f09fd3421449564dba31)
[Bug tree-optimization/116034] [12/13 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #12 from GCC Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:69e69847e21a8d951ab5f09fd3421449564dba31 commit r15-2274-g69e69847e21a8d951ab5f09fd3421449564dba31 Author: Jakub Jelinek Date: Wed Jul 24 18:00:05 2024 +0200 testsuite: Fix up pr116034.c test for big/pdp endian [PR116061] Didn't notice the memmove is into an int variable, so the test was still failing on big endian. 2024-07-24 Jakub Jelinek PR tree-optimization/116034 PR testsuite/116061 * gcc.dg/pr116034.c (g): Change type from int to unsigned short. (foo): Guard memmove call on __SIZEOF_SHORT__ == 2.
[Bug tree-optimization/116034] [12/13 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 Jakub Jelinek changed: What|Removed |Added Known to fail|14.1.1, 15.0|14.1.0 Known to work||14.1.1, 15.0 Summary|[12/13/14/15 Regression]|[12/13 Regression] wrong |wrong code with memcpy()|code with memcpy() from |from _Complex unsigned |_Complex unsigned short at |short at|-fno-strict-aliasing -O1 |-fno-strict-aliasing -O1|and above |and above | --- Comment #11 from Jakub Jelinek --- Fixed for 14.2+/15+ for now.
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #10 from GCC Commits --- The releases/gcc-14 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:084768c865cd50a6f7ff177db2dbdbb7aadaeee0 commit r14-10501-g084768c865cd50a6f7ff177db2dbdbb7aadaeee0 Author: Jakub Jelinek Date: Tue Jul 23 10:50:29 2024 +0200 ssa: Fix up maybe_rewrite_mem_ref_base complex type handling [PR116034] The folding into REALPART_EXPR is correct, used only when the mem_offset is zero, but for IMAGPART_EXPR it didn't check the exact offset value (just that it is not 0). The following patch fixes that by using IMAGPART_EXPR only if the offset is right and using BITFIELD_REF or whatever else otherwise. 2024-07-23 Jakub Jelinek Andrew Pinski PR tree-optimization/116034 * tree-ssa.cc (maybe_rewrite_mem_ref_base): Only use IMAGPART_EXPR if MEM_REF offset is equal to element type size. * gcc.dg/pr116034.c: New test. (cherry picked from commit b9cefd67a2a464a3c9413e6b3f28e7dc7a9ef162)
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #9 from GCC Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:b9cefd67a2a464a3c9413e6b3f28e7dc7a9ef162 commit r15-2220-gb9cefd67a2a464a3c9413e6b3f28e7dc7a9ef162 Author: Jakub Jelinek Date: Tue Jul 23 10:50:29 2024 +0200 ssa: Fix up maybe_rewrite_mem_ref_base complex type handling [PR116034] The folding into REALPART_EXPR is correct, used only when the mem_offset is zero, but for IMAGPART_EXPR it didn't check the exact offset value (just that it is not 0). The following patch fixes that by using IMAGPART_EXPR only if the offset is right and using BITFIELD_REF or whatever else otherwise. 2024-07-23 Jakub Jelinek Andrew Pinski PR tree-optimization/116034 * tree-ssa.cc (maybe_rewrite_mem_ref_base): Only use IMAGPART_EXPR if MEM_REF offset is equal to element type size. * gcc.dg/pr116034.c: New test.
[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 Andrew Pinski changed: What|Removed |Added Last reconfirmed||2024-07-23 Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED --- Comment #1 from Andrew Pinski --- .
[Bug rtl-optimization/116039] [15] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 Andrew Pinski changed: What|Removed |Added Component|target |rtl-optimization CC||pinskia at gcc dot gnu.org Target Milestone|--- |15.0
[Bug target/116039] New: [15] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039 Bug ID: 116039 Summary: [15] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5 Product: gcc Version: 15.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: target Assignee: unassigned at gcc dot gnu.org Reporter: patrick at rivosinc dot com Target Milestone: --- Testcase: int c[12]; char d[12]; int *f = c; int *z = (int *)1; long long y; int main() { c[9] = 0xff; for (int i = 0; i < 12; i += 3) d[9] = z ? f[i] : 0; for (long i = 0; i < 12; ++i) y ^= d[i]; __builtin_printf("%llu\n", y); } Commands: > /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/riscv64-unknown-linux-gnu-gcc > -O3 red.c -o user-config.out -fsigned-char -fno-strict-aliasing -fwrapv > QEMU_CPU=rv64,vlen=128,rvv_ta_all_1s=true,rvv_ma_all_1s=true,v=true,vext_spec=v1.0,zve32f=true,zve64f=true > timeout --verbose -k 0.1 4 > /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/qemu-riscv64 > user-config.out 255 > /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/riscv64-unknown-linux-gnu-gcc > -O2 red.c -o user-config.out -fsigned-char -fno-strict-aliasing -fwrapv > QEMU_CPU=rv64,vlen=128,rvv_ta_all_1s=true,rvv_ma_all_1s=true,v=true,vext_spec=v1.0,zve32f=true,zve64f=true > timeout --verbose -k 0.1 4 > /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/qemu-riscv64 > user-config.out 18446744073709551615 First bad commit: r15-1901-g98914f9eba5 Found via fuzzer.
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #8 from Andrew Pinski --- (In reply to Jakub Jelinek from comment #7) > Created attachment 58723 [details] > gcc15-pr116034.patch > > Full untested patch. This looks good to me. This is basically the same as the patch which I was coming up with too.
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #7 from Jakub Jelinek --- Created attachment 58723 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58723=edit gcc15-pr116034.patch Full untested patch.
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #6 from Zdenek Sojka --- (In reply to Sam James from comment #1) > on godbolt, 6.5 looks OK, and 7.1 starts to fail for amd64. My apologies; the results for gcc-6.5.0 and gcc-7.5.0 were not correct due to linking errors. Indeed 6.5.0 works, and 7.5.0 fails.
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #5 from Jakub Jelinek --- Started with r7-2818-gebfa15ab654738fcc926a506b3788a303958fa02
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #4 from Andrew Pinski --- I think the issue is in maybe_rewrite_mem_ref_base : else if (TREE_CODE (TREE_TYPE (sym)) == COMPLEX_TYPE && useless_type_conversion_p (TREE_TYPE (*tp), TREE_TYPE (TREE_TYPE (sym { *tp = build1 (integer_zerop (TREE_OPERAND (*tp, 1)) ? REALPART_EXPR : IMAGPART_EXPR, TREE_TYPE (*tp), sym); } It only checks for 0 and non-zero rather than 0 and size (of type) like it is done in gimple-fold.cc and fold-const.cc. Which means it has been latent since r0-107202-g64a3d6470e5eb7 and exposed by something started to be optimized in GCC 7.
[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 Andrew Pinski changed: What|Removed |Added Ever confirmed|0 |1 Status|UNCONFIRMED |NEW Last reconfirmed||2024-07-22 Target Milestone|--- |12.5 Summary|wrong code with memcpy()|[12/13/14/15 Regression] |from _Complex unsigned |wrong code with memcpy() |short at|from _Complex unsigned |-fno-strict-aliasing -O1|short at |and above |-fno-strict-aliasing -O1 ||and above --- Comment #3 from Andrew Pinski --- Confirmed. Looks like the code that folds MEM[(charD.10 * {ref-all}) + 1B] into REAL/IMAG is going wrong and should not have folded it into that. I have not looked into why though.
[Bug tree-optimization/116034] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 --- Comment #2 from Andrew Pinski --- Folding statement: _1 = + 1; Queued stmt for removal. Folds to: <__complex__ short unsigned int> [(void *) + 1B] Folding statement: _3 = MEM [(char * {ref-all})_1]; Folded into: _3 = MEM [(char * {ref-all}) + 1B]; That is ok, but then we change it into: _3 = IMAGPART_EXPR ;
[Bug tree-optimization/116034] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 Sam James changed: What|Removed |Added CC||sjames at gcc dot gnu.org --- Comment #1 from Sam James --- on godbolt, 6.5 looks OK, and 7.1 starts to fail for amd64.
[Bug tree-optimization/116034] New: wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034 Bug ID: 116034 Summary: wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above Product: gcc Version: 15.0 Status: UNCONFIRMED Keywords: wrong-code Severity: normal Priority: P3 Component: tree-optimization Assignee: unassigned at gcc dot gnu.org Reporter: zsojka at seznam dot cz Target Milestone: --- Host: x86_64-pc-linux-gnu Target: x86_64-pc-linux-gnu Created attachment 58721 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58721=edit reduced testcase Output: $ x86_64-pc-linux-gnu-gcc -O1 -fno-strict-aliasing testcase.c $ ./a.out Aborted $ x86_64-pc-linux-gnu-gcc -v Using built-in specs. COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r15-2206-20240722194717-g6f81b7fa799-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/15.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++ --enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra --disable-bootstrap --with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld --with-as=/usr/bin/x86_64-pc-linux-gnu-as --enable-libsanitizer --disable-libstdcxx-pch --prefix=/repo/gcc-trunk//binary-trunk-r15-2206-20240722194717-g6f81b7fa799-checking-yes-rtl-df-extra-nobootstrap-amd64 Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 15.0.0 20240722 (experimental) (GCC) This is also failing on aarch64-unknown-linux-gnu and powerpc64le-unknown-linux-gnu, but it's OK on mips64el-unknown-linux-gnuabi64 and riscv64-unknown-linux-gnu.
[Bug tree-optimization/113630] [12 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 Richard Biener changed: What|Removed |Added Target Milestone|11.5|12.5 --- Comment #10 from Richard Biener --- GCC 11 branch is being closed.
[Bug lto/112732] during IPA pass: *free_lang_data ICE: 'verify_type' failed: type variant with 'TYPE_ALIAS_SET_KNOWN_P' with -Os -Wstrict-aliasing=2 -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112732 --- Comment #7 from GCC Commits --- The releases/gcc-11 branch has been updated by Richard Biener : https://gcc.gnu.org/g:e7879391bb2b86606d0ce35ed97eccc108970e36 commit r11-11561-ge7879391bb2b86606d0ce35ed97eccc108970e36 Author: Richard Biener Date: Tue Nov 28 12:36:21 2023 +0100 middle-end/112732 - stray TYPE_ALIAS_SET in type variant The following fixes a stray TYPE_ALIAS_SET in a type variant built by build_opaque_vector_type which is diagnosed by type checking enabled with -flto. PR middle-end/112732 * tree.c (build_opaque_vector_type): Reset TYPE_ALIAS_SET of the newly built type. (cherry picked from commit f26d68d5d128c86faaceeb81b1e8f22254ad53df)
[Bug c++/115658] char16_t and char32_t aliasing is conserative
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658 --- Comment #3 from Tom Honermann --- In retrospect, I think I misunderstood Andrew's motivation for filing this issue. There is a difference of behavior between gcc and clang with regard to aliasing of `char16_t` and `char32_t` with respect to other types. This is illustrated by the following example as demonstrated at https://www.godbolt.org/z/PsMsPMa73. Please note that (at least) `-O2` is necessary to reliably demonstrate differences in behavior. Additionally, the use of `-fshort-wchar` to influence the size of `wchar_t` affects behavior. ``` template U f(T *p, U *q) { *p = 1; U u = *q; *p = 2; return u; } template wchar_t f(char16_t*, wchar_t*); template unsigned short f(char16_t*, unsigned short*); template wchar_t f(char32_t*, wchar_t*); template unsigned int f(char32_t*, unsigned int*); ``` The test case exercises dead store elimination in the presence of aliasing types. If `T` may alias `U`, then the write of `1` to `*p` is observable by `*q`, but may otherwise be eliminated due to the later write of `2` to `*p`. For Clang, there is no aliasing between any of these types and the store of `1` to `*p` is always eliminated. For MSVC, it appears that either dead store elimination is not performed at all, or aliasing is permitted across all of these types (even when the size differs). For gcc with `-fshort-wchar`, there appear to be two alias sets: - `wchar_t`, `char16_t`, and `unsigned short`. - `char32_t` and `unsigned int`. For gcc without `-fshort-wchar`, there are also two alias sets, but they are not symmetric in the presence of that option. Note that `char32_t` never aliases with `wchar_t` even when they have the same size. This asymmetry is explainable in consideration of compatibility with MSVC (where `wchar_t` is always 16-bit). - `char16_t` and `unsigned short`. - `char32_t` and `unsigned int`. Adding the following explicit template instantiations demonstrates that all of gcc, clang, and MSVC permit aliasing between the set of `char`, `unsigned char`, and `char8_t` (because `char` and `unsigned char` are permitted to alias all types). https://www.godbolt.org/z/Pjxb661Y7. ``` template char f(char8_t*, char*); template unsigned char f(char8_t*, unsigned char*); ``` To reiterate, I think the current gcc behavior is correct and defensible given two goals: - A desire to match MSVC behavior in the limited context of a 16-bit `wchar_t` type. - A desire to match C behavior with respect to `char16_t` and `char32_t` aliasing the underlying types of `uint_least16_t` and `uint_least32_t` (the former are typedefs in C).
[Bug c++/115658] char16_t and char32_t aliasing is conserative
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658 Tom Honermann changed: What|Removed |Added CC||tom at honermann dot net --- Comment #2 from Tom Honermann --- I think the prior comments regarding the aliasing sets refer to portions of c_common_get_alias_set() in gcc/gcc/c-family/c-common.cc, specifically the lines at https://github.com/gcc-mirror/gcc/blob/629257bcb81434117f1e9c68479032563176dc0c/gcc/c-family/c-common.cc#L3892-L3895. I think the alias sets are right; they are consistent with what the C++ standard states in [basic.lval]p11 (http://eel.is/c++draft/basic.lval#11). > So maybe we really should keep on treating them the same. and maybe change char8_t back to similar as unsigned char ... I don't think that should be necessary. P0482R6 was explicit in its intent that char8_t not be an aliasing type (http://wg21.link/p0482r6#proposal).
[Bug c++/115658] char16_t and char32_t aliasing is conserative
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658 --- Comment #1 from Andrew Pinski --- Though I should note https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2626r0.pdf and https://github.com/sg16-unicode/sg16-meetings/tree/master#may-22nd-2024 So maybe we really should keep on treating them the same. and maybe change char8_t back to similar as unsigned char ...
[Bug c++/115658] New: char16_t and char32_t aliasing is conserative
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658 Bug ID: 115658 Summary: char16_t and char32_t aliasing is conserative Product: gcc Version: 15.0 Status: UNCONFIRMED Keywords: alias, missed-optimization Severity: enhancement Priority: P3 Component: c++ Assignee: unassigned at gcc dot gnu.org Reporter: pinskia at gcc dot gnu.org Target Milestone: --- when char8_t was added, a new aliasing set was done: r9-5405-g2d91f79dc990f8 But when char16_t and char32_t was added (for GCC 4.4/C++11): r0-88474-gc466b2cd136139 That was not done. Maybe it should be done now. Noticed from https://github.com/sg16-unicode/sg16/issues/67 .
[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552 --- Comment #3 from davidhcefx --- Thanks for your explanation, we now understand the problem.
[Bug tree-optimization/107467] [12/13/14/15 Regression] Miscompilation involing -Os , -flto and -fno-strict-aliasing since r12-656-ga564da506f52be66
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107467 Richard Biener changed: What|Removed |Added Target Milestone|12.4|12.5 --- Comment #11 from Richard Biener --- GCC 12.4 is being released, retargeting bugs to GCC 12.5.
[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552 --- Comment #2 from Andrew Pinski --- So basically since main writes via a double type but then spookyhash_short reads via a uint64_t type there is an alias violation. Using an union to change the pointer type does not change there is an alias violation happening. You need to use memcpy for copying from one type to another in this case. Instead of doing: c += u.p64[0]; You could do load_64([0]) and have load_64 defined to be: ``` static inline uint64_t load_64(void *a) { uint64_t t; memcpy(, a, sizeof(t)); return t; } ```
[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552 Andrew Pinski changed: What|Removed |Added Resolution|--- |INVALID Status|UNCONFIRMED |RESOLVED --- Comment #1 from Andrew Pinski --- union { const uint8_t *p8; uint32_t *p32; uint64_t *p64; size_t i; } u; Does not get around aliasing rules of C/C++.
[Bug c/115552] New: wrong code at -O2 and above when strict-aliasing with uint128 and double
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552 Bug ID: 115552 Summary: wrong code at -O2 and above when strict-aliasing with uint128 and double Product: gcc Version: 12.2.0 Status: UNCONFIRMED Keywords: alias, wrong-code Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: davidhu0903ex3 at gmail dot com Target Milestone: --- Created attachment 58469 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58469=edit preproccessed file === Summary === When turning on -O2 optimization or above, the produced hash output is different when without -O2, unless adding `-fno-strict-aliasing`. To my investigation, the issue arise only when either: 1. output to a uint128_t* 2. input with a double* === Detail Explanation === The program produced the following output when compiled with -O1 60d8f0889f4b7d88 3bcf47a7eecabc5d 1b153029d058b5115490463dd8be0468 1b153029d058b5115490463dd8be0468 However, it produced different output with -O2 e09546989572a77c 95fe807a3fab3ad5 b8b1ef1e717bc0d4818be1e9343867f6 The first two lines are the case where the inputs are double* pointers. The third line is the case where the output are written to a uint128_t* pointer. The last line is the case of passing a uint128_t by value to `foo` before dump. The warning didn't complain about strict-aliasing, but rather an ambiguous message: 'd1' may be used uninitialized [-Wmaybe-uninitialized] However, those variables are already initialized in `main`: double d1 = 1; double d2 = 2; >From the generated assembly, the initialization of the inputs seems missing, so perhaps that's the reason of the warning message. However, in production our code didn't emit "-Wmaybe-uninitialized" at all, so that's what surprises us. With Compiler Explorer (godbolt.org), I can roughly locate that this behavior appear after version 10.5.0 and until 14.1.0. === Environment === Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/12/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 12.2.0-14' --with-bugurl=file:///usr/share/doc/gcc-12/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-12 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-gcn/usr --enable-offload-defaulted --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 12.2.0 (Debian 12.2.0-14) COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc' '-mtune=generic' '-march=x86-64' /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -E -quiet -v -imultiarch x86_64-linux-gnu poc.c -mtune=generic -march=x86-64 -Wall -Wextra -O2 -fpch-preprocess -fasynchronous-unwind-tables -o poc.i ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu" ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/include-fixed" ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include" #include "..." search starts here: #include <...> search starts here: /usr/lib/gcc/x86_64-linux-gnu/12/include /usr/local/include /usr/include/x86_64-linux-gnu /usr/include End of search list. COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc' '-mtune=generic' '-march=x86-64' /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -fpreprocessed poc.i -quiet -dumpbase poc.c -dumpbase-ext .c -mtune=generic -march=x86-64 -O2 -Wall -Wextra -version -fasynchronous-unwind-tables -o poc.s GNU C17 (Debian 12.2.0-14) version 12.2.0 (x86_64-linux-gnu) compiled by GNU C version 12.2.0, GMP version 6.2.1, MPFR version 4.1.1-p1, MPC version 1.3.1, isl version isl-0.25-GMP warning: MPFR header version 4.1.1-p1 differs from library version 4.2.0. GGC heuristics: --pa
Re: [PATCH v2 0/4] Libatomic: Cleanup ifunc selector and aliasing
Victor Do Nascimento writes: > Changes in V2: > > As explained in patch v2 1/4, it has become clear that the current > approach of querying assembler support for newer architectural > extensions at compile time is undesirable both from a maintainability > as well as a consistency standpoint - Different compiled versions of > Libatomic may have different features depending on the machine on > which they were built. > > These issues make for difficult testing as the explosion in number of > `#ifdef' guards makes maintenance error-prone and the dependence on > binutils version means that, as well as deploying changes for testing > in a variety of target configurations, testing must also involve > compiling the library on an increasing number of host configurations, > meaning that the chance of bugs going undetected increases (as was > proved in the pre-commit CI which, due to the use of an older version > of Binutils, picked up on a runtime-error that had hitherto gone > unnoticed). > > We therefore do away with the use of all assembly instructions > dependent on Binutils 2.42, choosing to replace them with `.inst's > instead. This eliminates the latent bug picked up by CI and will > ensure consistent builds of Libatomic across all versions of Binutils. Nice! Thanks for doing this. It seems much cleaner and more flexible than the current approach. Thanks also for the clear organisation of the series. OK for trunk. (For the record, I didn't hand-check the encodings of the .insts ...) Richard > --- > > The recent introduction of the optional LSE128 and RCPC3 architectural > extensions to AArch64 has further led to the increased flexibility of > atomic support in the architecture, with many extensions providing > support for distinct atomic operations, each with different potential > applications in mind. > > This has led to maintenance difficulties in Libatomic, in particular > regarding the way the ifunc selector is generated via a series of > macro expansions at compile-time. > > Until now, irrespective of the atomic operation in question, all atomic > functions for a particular operand size were expected to have the same > number of ifunc alternatives, meaning that a one-size-fits-all > approach could reasonably be taken for the selector. > > This meant that if, hypothetically, for a particular architecture and > operand size one particular atomic operation was to have 3 different > implementations associated with different extensions, libatomic would > likewise be required to present three ifunc alternatives for all other > atomic functions. > > The consequence in the design choice was the unnecessary use of > function aliasing and the unwieldy code which resulted from this. > > This patch series attempts to remediate this issue by making the > preprocessor macros defining the number of ifunc alternatives and > their respective selection functions dependent on the file importing > the ifunc selector-generating framework. > > all files are given `LAT_' macros, defined at the beginning > and undef'd at the end of the file. It is these macros that are > subsequently used to fine-tune the behaviors of `libatomic_i.h' and > `host-config.h'. > > In particular, the definition of the `IFUNC_NCOND(N)' and > `IFUNC_COND_' macros in host-config.h can now be guarded behind > these new file-specific macros, which ultimately control what the > `GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to. As both of > these headers are imported once per file implementing some atomic > operation, fine-tuned control is now possible. > > Regtested with both `--enable-gnu-indirect-function' and > `--disable-gnu-indirect-function' configurations on armv9.4-a target > with LRCPC3 and LSE128 support and without. > > Victor Do Nascimento (4): > Libatomic: AArch64: Convert all lse128 assembly to .insn directives > Libatomic: Define per-file identifier macros > Libatomic: Make ifunc selector behavior contingent on importing file > Libatomic: Clean up AArch64 `atomic_16.S' implementation file > > libatomic/acinclude.m4 | 18 - > libatomic/auto-config.h.in | 3 - > libatomic/cas_n.c| 2 + > libatomic/config/linux/aarch64/atomic_16.S | 511 +-- > libatomic/config/linux/aarch64/host-config.h | 35 +- > libatomic/configure | 43 -- > libatomic/configure.ac | 3 - > libatomic/exch_n.c | 2 + > libatomic/fadd_n.c | 2 + > libatomic/fand_n.c | 2 + > libatomic/fence.c| 2 + > libatomic/fenv.c
[Bug c/115096] -fstrict-aliasing miscompilation or missing warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096 Andrew Pinski changed: What|Removed |Added CC||Explorer09 at gmail dot com --- Comment #3 from Andrew Pinski --- *** Bug 115437 has been marked as a duplicate of this bug. ***
[Bug lto/80379] Redundant note: code may be misoptimized unless -fno-strict-aliasing is used
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80379 --- Comment #5 from Andi Kleen --- This bug is about printing a unnecessary message. If your code is actually miscompiled even with -fno-strict-aliasing set (so it is ignored somewhere) it is something different and you would need a test case to debug.
[PATCH v2 0/4] Libatomic: Cleanup ifunc selector and aliasing
Changes in V2: As explained in patch v2 1/4, it has become clear that the current approach of querying assembler support for newer architectural extensions at compile time is undesirable both from a maintainability as well as a consistency standpoint - Different compiled versions of Libatomic may have different features depending on the machine on which they were built. These issues make for difficult testing as the explosion in number of `#ifdef' guards makes maintenance error-prone and the dependence on binutils version means that, as well as deploying changes for testing in a variety of target configurations, testing must also involve compiling the library on an increasing number of host configurations, meaning that the chance of bugs going undetected increases (as was proved in the pre-commit CI which, due to the use of an older version of Binutils, picked up on a runtime-error that had hitherto gone unnoticed). We therefore do away with the use of all assembly instructions dependent on Binutils 2.42, choosing to replace them with `.inst's instead. This eliminates the latent bug picked up by CI and will ensure consistent builds of Libatomic across all versions of Binutils. --- The recent introduction of the optional LSE128 and RCPC3 architectural extensions to AArch64 has further led to the increased flexibility of atomic support in the architecture, with many extensions providing support for distinct atomic operations, each with different potential applications in mind. This has led to maintenance difficulties in Libatomic, in particular regarding the way the ifunc selector is generated via a series of macro expansions at compile-time. Until now, irrespective of the atomic operation in question, all atomic functions for a particular operand size were expected to have the same number of ifunc alternatives, meaning that a one-size-fits-all approach could reasonably be taken for the selector. This meant that if, hypothetically, for a particular architecture and operand size one particular atomic operation was to have 3 different implementations associated with different extensions, libatomic would likewise be required to present three ifunc alternatives for all other atomic functions. The consequence in the design choice was the unnecessary use of function aliasing and the unwieldy code which resulted from this. This patch series attempts to remediate this issue by making the preprocessor macros defining the number of ifunc alternatives and their respective selection functions dependent on the file importing the ifunc selector-generating framework. all files are given `LAT_' macros, defined at the beginning and undef'd at the end of the file. It is these macros that are subsequently used to fine-tune the behaviors of `libatomic_i.h' and `host-config.h'. In particular, the definition of the `IFUNC_NCOND(N)' and `IFUNC_COND_' macros in host-config.h can now be guarded behind these new file-specific macros, which ultimately control what the `GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to. As both of these headers are imported once per file implementing some atomic operation, fine-tuned control is now possible. Regtested with both `--enable-gnu-indirect-function' and `--disable-gnu-indirect-function' configurations on armv9.4-a target with LRCPC3 and LSE128 support and without. Victor Do Nascimento (4): Libatomic: AArch64: Convert all lse128 assembly to .insn directives Libatomic: Define per-file identifier macros Libatomic: Make ifunc selector behavior contingent on importing file Libatomic: Clean up AArch64 `atomic_16.S' implementation file libatomic/acinclude.m4 | 18 - libatomic/auto-config.h.in | 3 - libatomic/cas_n.c| 2 + libatomic/config/linux/aarch64/atomic_16.S | 511 +-- libatomic/config/linux/aarch64/host-config.h | 35 +- libatomic/configure | 43 -- libatomic/configure.ac | 3 - libatomic/exch_n.c | 2 + libatomic/fadd_n.c | 2 + libatomic/fand_n.c | 2 + libatomic/fence.c| 2 + libatomic/fenv.c | 2 + libatomic/fior_n.c | 2 + libatomic/flag.c | 2 + libatomic/fnand_n.c | 2 + libatomic/fop_n.c| 2 + libatomic/fsub_n.c | 2 + libatomic/fxor_n.c | 2 + libatomic/gcas.c | 2 + libatomic/gexch.c| 2 + libatomic/glfree.c | 2 + libatomic/gload.c| 2 + libatomic/gstore.c | 2 + libatomic/load_n.c | 2 + libatomic/store_n.c
[Bug lto/115432] Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115432 Richard Biener changed: What|Removed |Added Resolution|--- |INVALID Status|UNCONFIRMED |RESOLVED --- Comment #2 from Richard Biener --- struct file_output_stream { union { void *voidp; int fd; } data; const output_stream_vtbl* vtbl; }; struct output_stream { void* data; const output_stream_vtbl* vtbl; }; those are two unrelated types. Doing ((file_output_stream *)p)->vtbl = x; ... = ((output_stream *)p)->vtbl; is invoking undefined behavior (unless -fno-strict-aliasing).
[Bug lto/115432] Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115432 --- Comment #1 from Richard Biener --- In case output_stream is not the same or derived from file_output_stream or contains a file_output_stream object as first member you invoke undefined behavior when the calls following might read from the object via output_stream or another alltogether different type (buffer_output_stream?).
[Bug lto/115432] New: Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115432 Bug ID: 115432 Summary: Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing Product: gcc Version: 14.1.1 Status: UNCONFIRMED Severity: normal Priority: P3 Component: lto Assignee: unassigned at gcc dot gnu.org Reporter: Eric.Diaz.Fernandez at eurid dot eu Target Milestone: --- Created attachment 58403 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58403=edit All the .i files and the output of the build. Summary: -flto generated code is wrong: resulting binary doesn't contain the call to an initialisation function (making the program crash, obviously) Using -fno-strict-aliasing disables the bug Tried both on: gcc (Ubuntu 13.2.0-23ubuntu4) 13.2.0 / Ubuntu 24.04 Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-linux-gnu/13/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 13.2.0-23ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-13 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/libexec --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-libstdcxx-backtrace --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-13-uJ7kn6/gcc-13-13.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-13-uJ7kn6/gcc-13-13.2.0/debian/tmp-gcn/usr --enable-offload-defaulted --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 13.2.0 (Ubuntu 13.2.0-23ubuntu4) gcc (GCC) 14.1.1 20240522 / Arch gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/14.1.1/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /build/gcc/src/gcc/configure --enable-languages=ada,c,c++,d,fortran,go,lto,m2,objc,obj-c++,rust --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://gitea.artixlinux.org/packages/gcc/issues --with-build-config=bootstrap-lto --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-libstdcxx-backtrace --enable-link-serialization=1 --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 14.1.1 20240522 (GCC) The bug is an initialisation function missing from the generated assembly. The bug only occurrs when doing the build of the full program using LTO and only for one of the program (yadifad). The only difference I can think of between that program and the other ones (when initialisation is concerned) is that yadifad calls the dnscore_init_ex function in two locations and in two different manners: _ It can call it using a constant bitmask indicating only a minimal amount of subsystems are to be initialised. _ It can call it using a varibable bitmask indicating either a minimal amount or all of the subsystems are to be initialised. I've tried to make a sample case but it appears any modification makes the issue disappear. So all I have is the full build. I've read that you only want the .i files and I'm attaching the ones directly used here. I can't attach them all as the compressed archive is bigger than 2MB. The archive also contains the output of the build. One of the files compilation line: gcc -DHAVE_CONFIG_H -I. -I/home/guest/src/yadifa-2.6/sbin/yadifad -I../.. -D_THREAD_SAFE -D_REENTRANT -D_FILE_OFFSET_BITS=64 -I/tmp/ltonofat/sbin/yadifad -I/tmp/ltonofat/sbin/yadifad/include -I/home/guest/src/yadifa-2.6/sbin/yadifad/include -fno-ident -ansi -pedantic -Wall -Wno-unknown-pragmas -Werror=missing-field-initializers -std=gnu11 -mtune=native -DUSES_GCC -DPREFIX='"/u
[Bug lto/80379] Redundant note: code may be misoptimized unless -fno-strict-aliasing is used
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80379 Eric Diaz Fernandez changed: What|Removed |Added CC||Eric.Diaz.Fernandez at eurid dot e ||u --- Comment #4 from Eric Diaz Fernandez --- I believe I've hit that bug. I was about to post a report it when I found this one. When building one of my programs with -flto but without -fno-strict-aliasing the resulting assembly misses the call to an initialization function. The current workaround suggested to the maintainer is to not use -flto Is there anything I can add to help fixing the issue?
[Bug middle-end/115405] wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405 --- Comment #3 from Richard Biener --- It's not visible but I assume that _4 doesn't have _BitInt(17) type? The if (known_eq (offset, 0) && !reverse && poly_int_tree_p (TYPE_SIZE (type), _size) && known_eq (GET_MODE_BITSIZE (DECL_MODE (base)), type_size)) check tries to assess that no extension is required, does it work if you adjust that for the _BitInt case? OTOH the reduce_bit_field handling in VIEW_CONVERT_EXPR expansion looks misplaced - shouldn't it be before the INTEGRAL_TYPE_P handling?
[Bug middle-end/115405] wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405 Jakub Jelinek changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #2 from Jakub Jelinek --- The IL before expansion doesn't have a VCE, just MEM[(_BitInt(17) *)] where b has unsigned _BitInt(17) type. Then /* Handle expansion of non-aliased memory with non-BLKmode. That might end up in a register. */ if (mem_ref_refers_to_non_mem_p (exp)) { poly_int64 offset = mem_ref_offset (exp).force_shwi (); base = TREE_OPERAND (base, 0); poly_uint64 type_size; if (known_eq (offset, 0) && !reverse && poly_int_tree_p (TYPE_SIZE (type), _size) && known_eq (GET_MODE_BITSIZE (DECL_MODE (base)), type_size)) return expand_expr (build1 (VIEW_CONVERT_EXPR, type, base), target, tmode, modifier); in expr.cc turns that into a VCE. And VCE doesn't really EXTEND_BITINT because the upper bits generally in a VCE are undefined. Finally the NOP_EXPR expansion sees a SImode signed _BitInt(17) extension to SImode 32-bit int and assumes for something in an automatic var to be already properly extended. Dunno where the bug is, whether trying to use VCE for the _BitInt cases as in the snippet above, or VCE not doing EXTEND_BITINT, or NOP_EXPR handling that should extend even in this case.
[Bug middle-end/115405] wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405 Andrew Pinski changed: What|Removed |Added Last reconfirmed||2024-06-09 Status|UNCONFIRMED |NEW Ever confirmed|0 |1 Component|rtl-optimization|middle-end --- Comment #1 from Andrew Pinski --- Confirmed. Looks to be a mising sign extend when expanding the VCE like MEM_REF. ;; _4 = MEM[(_BitInt(17) *)]; (insn 8 7 9 (parallel [ (set (reg:SI 103) (and:SI (reg/v:SI 100 [ bD.2205 ]) (const_int 131071 [0x1]))) (clobber (reg:CC 17 flags)) ]) "/app/example.cpp":6:10 -1 (nil)) (insn 9 8 0 (set (reg:SI 99 [ _4 ]) (reg:SI 103)) "/app/example.cpp":6:10 -1 (nil)) ;; b ={v} {CLOBBER(eos)}; (nil) ;; if (_4 != -65536) (insn 10 9 11 (set (reg:CCZ 17 flags) (compare:CCZ (reg:SI 99 [ _4 ]) (const_int -65536 [0x]))) "/app/example.cpp":13:6 -1 (nil))
[Bug tree-optimization/115405] New: wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405 Bug ID: 115405 Summary: wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above Product: gcc Version: 15.0 Status: UNCONFIRMED Keywords: wrong-code Severity: normal Priority: P3 Component: tree-optimization Assignee: unassigned at gcc dot gnu.org Reporter: zsojka at seznam dot cz CC: jakub at gcc dot gnu.org Target Milestone: --- Host: x86_64-pc-linux-gnu Target: x86_64-pc-linux-gnu Created attachment 58389 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58389=edit reduced testcase Output: $ x86_64-pc-linux-gnu-gcc -O0 testcase.c $ ./a.out $ x86_64-pc-linux-gnu-gcc -O1 testcase.c $ ./a.out Aborted I think the code should have defined behavior at least at -O1 or with -fno-strict-aliasing. $ x86_64-pc-linux-gnu-gcc -v Using built-in specs. COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r15-1122-20240609121028-g8bb6b2f4ae1-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/15.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++ --enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra --disable-bootstrap --with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld --with-as=/usr/bin/x86_64-pc-linux-gnu-as --enable-libsanitizer --disable-libstdcxx-pch --prefix=/repo/gcc-trunk//binary-trunk-r15-1122-20240609121028-g8bb6b2f4ae1-checking-yes-rtl-df-extra-nobootstrap-amd64 Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 15.0.0 20240609 (experimental) (GCC)
[Bug lto/112732] during IPA pass: *free_lang_data ICE: 'verify_type' failed: type variant with 'TYPE_ALIAS_SET_KNOWN_P' with -Os -Wstrict-aliasing=2 -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112732 --- Comment #6 from GCC Commits --- The releases/gcc-12 branch has been updated by Richard Biener : https://gcc.gnu.org/g:b46486ef0316240eb3c173bda062b52333507e03 commit r12-10492-gb46486ef0316240eb3c173bda062b52333507e03 Author: Richard Biener Date: Tue Nov 28 12:36:21 2023 +0100 middle-end/112732 - stray TYPE_ALIAS_SET in type variant The following fixes a stray TYPE_ALIAS_SET in a type variant built by build_opaque_vector_type which is diagnosed by type checking enabled with -flto. PR middle-end/112732 * tree.cc (build_opaque_vector_type): Reset TYPE_ALIAS_SET of the newly built type. (cherry picked from commit f26d68d5d128c86faaceeb81b1e8f22254ad53df)
[gcc r15-934] C23: allow aliasing for types derived from structs with variable size
https://gcc.gnu.org/g:d2cfe8a73b3c4195a25cde28e1641ef36ebb08c1 commit r15-934-gd2cfe8a73b3c4195a25cde28e1641ef36ebb08c1 Author: Martin Uecker Date: Fri May 24 12:35:27 2024 +0200 C23: allow aliasing for types derived from structs with variable size Previously, we set the aliasing set of structures with variable size struct foo { int x[n]; char b; }; to zero. The reason is that such types can be compatible to diffrent structure types which are incompatible. struct foo { int x[2]; char b; }; struct foo { int x[3]; char b; }; But it is not enough to set the aliasing set to zero, because derived types would then still end up in different equivalence classes even though they might be compatible. Instead those types should be set to structural equivalency. We also add checking assertions that ensure that TYPE_CANONICAL is set correctly for all tagged types. gcc/c/ * c-decl.cc (finish_struct): Do not set TYPE_CANONICAL for structure or unions with variable size. * c-objc-common.cc (c_get_alias_set): Do not set alias set to zero. * c-typeck.cc (comptypes_verify): New function. (comptypes,comptypes_same_p,comptypes_check_enum_int): Add assertion. (comptypes_equiv_p): Add assertion that ensures that compatible types have the same equivalence class. (tagged_types_tu_compatible_p): Remove now unneeded special case. gcc/testsuite/ * gcc.dg/gnu23-tag-alias-8.c: New test. Diff: --- gcc/c/c-decl.cc | 2 +- gcc/c/c-objc-common.cc | 5 - gcc/c/c-typeck.cc| 37 +--- gcc/testsuite/gcc.dg/gnu23-tag-alias-8.c | 24 + 4 files changed, 59 insertions(+), 9 deletions(-) diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index 6e6606c9570..9f7d55c0b10 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -9749,7 +9749,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, C_TYPE_BEING_DEFINED (t) = 0; /* Set type canonical based on equivalence class. */ - if (flag_isoc23) + if (flag_isoc23 && !C_TYPE_VARIABLE_SIZE (t)) { if (c_struct_htab == NULL) c_struct_htab = hash_table::create_ggc (61); diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index 283f6a8ae26..738e899a2a9 100644 --- a/gcc/c/c-objc-common.cc +++ b/gcc/c/c-objc-common.cc @@ -420,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED) alias_set_type c_get_alias_set (tree t) { - /* Structs with variable size can alias different incompatible - structs. Let them alias anything. */ - if (RECORD_OR_UNION_TYPE_P (t) && C_TYPE_VARIABLE_SIZE (t)) -return 0; - return c_common_get_alias_set (t); } diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 09b2c265a46..48934802148 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -1167,6 +1167,28 @@ common_type (tree t1, tree t2) return c_common_type (t1, t2); } + + +/* Helper function for comptypes. For two compatible types, return 1 + if they pass consistency checks. In particular we test that + TYPE_CANONICAL is set correctly, i.e. the two types can alias. */ + +static bool +comptypes_verify (tree type1, tree type2) +{ + if (TYPE_CANONICAL (type1) != TYPE_CANONICAL (type2) + && !TYPE_STRUCTURAL_EQUALITY_P (type1) + && !TYPE_STRUCTURAL_EQUALITY_P (type2)) +{ + /* FIXME: check other types. */ + if (RECORD_OR_UNION_TYPE_P (type1) + || TREE_CODE (type1) == ENUMERAL_TYPE + || TREE_CODE (type2) == ENUMERAL_TYPE) + return false; +} + return true; +} + struct comptypes_data { bool enum_and_int_p; bool different_types_p; @@ -1188,6 +1210,8 @@ comptypes (tree type1, tree type2) struct comptypes_data data = { }; bool ret = comptypes_internal (type1, type2, ); + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + return ret ? (data.warning_needed ? 2 : 1) : 0; } @@ -1201,6 +1225,8 @@ comptypes_same_p (tree type1, tree type2) struct comptypes_data data = { }; bool ret = comptypes_internal (type1, type2, ); + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + if (data.different_types_p) return false; @@ -1218,6 +1244,8 @@ comptypes_check_enum_int (tree type1, tree type2, bool *enum_and_int_p) bool ret = comptypes_internal (type1, type2, ); *enum_and_int_p = data.enum_and_int_p; + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + return ret ? (data.warning_needed ? 2 : 1) : 0; } @@ -1232,6 +1260,8 @@ comptypes_check_different_types (tree type1, tree type2, bool ret = comptypes_internal (type1, type2, ); *different_types_p = data.different_types_p; + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + retur
[gcc r15-933] C: allow aliasing of compatible types derived from enumeral types [PR115157]
https://gcc.gnu.org/g:867d1264fe71d4291194373d1a1c409cac97a597 commit r15-933-g867d1264fe71d4291194373d1a1c409cac97a597 Author: Martin Uecker Date: Sun May 19 23:13:22 2024 +0200 C: allow aliasing of compatible types derived from enumeral types [PR115157] Aliasing of enumeral types with the underlying integer is now allowed by setting the aliasing set to zero. But this does not allow aliasing of derived types which are compatible as required by ISO C. Instead, initially set structural equality. Then set TYPE_CANONICAL and update pointers and main variants when the type is completed (as done for structures and unions in C23). PR tree-optimization/115157 PR tree-optimization/115177 gcc/c/ * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. * c-objc-common.cc (get_alias_set): Remove special case. (get_aka_type): Add special case. gcc/c-family/ * c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL for hardbools. gcc/ * godump.cc (go_output_typedef): Use TYPE_MAIN_VARIANT instead of TYPE_CANONICAL. gcc/testsuite/ * gcc.dg/enum-alias-1.c: New test. * gcc.dg/enum-alias-2.c: New test. * gcc.dg/enum-alias-3.c: New test. * gcc.dg/enum-alias-4.c: New test. Diff: --- gcc/c-family/c-attribs.cc | 1 + gcc/c/c-decl.cc | 11 +-- gcc/c/c-objc-common.cc | 7 ++- gcc/godump.cc | 10 +++--- gcc/testsuite/gcc.dg/enum-alias-1.c | 24 gcc/testsuite/gcc.dg/enum-alias-2.c | 25 + gcc/testsuite/gcc.dg/enum-alias-3.c | 26 ++ gcc/testsuite/gcc.dg/enum-alias-4.c | 22 ++ 8 files changed, 112 insertions(+), 14 deletions(-) diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc index 605469dd7dd..e3833ed5f20 100644 --- a/gcc/c-family/c-attribs.cc +++ b/gcc/c-family/c-attribs.cc @@ -1074,6 +1074,7 @@ handle_hardbool_attribute (tree *node, tree name, tree args, TREE_SET_CODE (*node, ENUMERAL_TYPE); ENUM_UNDERLYING_TYPE (*node) = orig; + TYPE_CANONICAL (*node) = TYPE_CANONICAL (orig); tree false_value; if (args) diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index b691b91b3db..6e6606c9570 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) if (t == NULL_TREE) { t = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (t); pushtag (input_location, name, t); } @@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, the forward-reference will be altered into a real type. */ ref = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (ref); if (code == ENUMERAL_TYPE) { @@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, { enumtype = make_node (ENUMERAL_TYPE); TYPE_SIZE (enumtype) = NULL_TREE; + SET_TYPE_STRUCTURAL_EQUALITY (enumtype); pushtag (loc, name, enumtype); if (fixed_underlying_type != NULL_TREE) { @@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, TYPE_SIZE (enumtype) = NULL_TREE; TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type); ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type; + TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type); + c_update_type_canonical (enumtype); layout_type (enumtype); } } @@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree attributes) ENUM_UNDERLYING_TYPE (enumtype) = c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem)); + TYPE_CANONICAL (enumtype) = + TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype)); + c_update_type_canonical (enumtype); + layout_type (enumtype); } diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index 42a62c84fe7..283f6a8ae26 100644 --- a/gcc/c/c-objc-common.cc +++ b/gcc/c/c-objc-common.cc @@ -130,6 +130,8 @@ get_aka_type (tree type) result = get_aka_type (orig_type); } + else if (TREE_CODE (type) == ENUMERAL_TYPE) +return type; else { tree canonical = TYPE_CANONICAL (type); @@ -418,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED)
Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Thu, May 30, 2024 at 12:48 AM Martin Uecker wrote: > > > Hi Ian, > > can you give me a green light for the go changes. The C FE > changes were approved. > > The only change with respect to the last version are > the removal of the unneeded null check for the > main variant (as discussed) and that I also removed the > > container->decls_seen.add (TREE_TYPE (decl)); > > and the corresponding check, because I think it is > redundant if we also test for the main variant. > (it wasn't with TYPE_CANONICAL because this was > only added conditionally). > > The other code in the file only checks for added > declarations not types, so should not depend on this. Apologies. I thought that I had already said that the Go changes are fine if the libgo tests still pass. Anyhow, that is the case: if the tests pass, the change is fine. Thanks. Ian
Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]
Hi Ian, can you give me a green light for the go changes. The C FE changes were approved. The only change with respect to the last version are the removal of the unneeded null check for the main variant (as discussed) and that I also removed the container->decls_seen.add (TREE_TYPE (decl)); and the corresponding check, because I think it is redundant if we also test for the main variant. (it wasn't with TYPE_CANONICAL because this was only added conditionally). The other code in the file only checks for added declarations not types, so should not depend on this. Martin Am Freitag, dem 24.05.2024 um 17:39 +0200 schrieb Martin Uecker: > This is another version of this patch with two changes: > > - I added a fix (with test) for PR 115177 which is just the same > issue for hardbools which are internally implemented as enums. > > - I fixed the golang issue. Since the addition of the main variant > to the seen decls is unconditional I removed also the addition > of the type itself which now seems unnecessary. > > Bootstrapped and regression tested on x86_64. > > Martin > > > > C: allow aliasing of compatible types derived from enumeral types > [PR115157] > > Aliasing of enumeral types with the underlying integer is now allowed > by setting the aliasing set to zero. But this does not allow aliasing > of derived types which are compatible as required by ISO C. Instead, > initially set structural equality. Then set TYPE_CANONICAL and update > pointers and main variants when the type is completed (as done for > structures and unions in C23). > > PR 115157 > PR 115177 > > gcc/c/ > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > * c-obj-common.cc (get_alias_set): Remove special case. > (get_aka_type): Add special case. > > gcc/c-family/ > * c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL > for hardbools. > > gcc/ > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > of TYPE_CANONICAL. > > gcc/testsuite/ > * gcc.dg/enum-alias-1.c: New test. > * gcc.dg/enum-alias-2.c: New test. > * gcc.dg/enum-alias-3.c: New test. > * gcc.dg/enum-alias-4.c: New test. > > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc > index 04e39b41bdf..033395093b6 100644 > --- a/gcc/c-family/c-attribs.cc > +++ b/gcc/c-family/c-attribs.cc > @@ -1074,6 +1074,7 @@ handle_hardbool_attribute (tree *node, tree name, tree > args, > >TREE_SET_CODE (*node, ENUMERAL_TYPE); >ENUM_UNDERLYING_TYPE (*node) = orig; > + TYPE_CANONICAL (*node) = TYPE_CANONICAL (orig); > >tree false_value; >if (args) > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > index b691b91b3db..6e6606c9570 100644 > --- a/gcc/c/c-decl.cc > +++ b/gcc/c/c-decl.cc > @@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, > int warned) > if (t == NULL_TREE) > { > t = make_node (code); > - if (flag_isoc23 && code != ENUMERAL_TYPE) > + if (flag_isoc23 || code == ENUMERAL_TYPE) > SET_TYPE_STRUCTURAL_EQUALITY (t); > pushtag (input_location, name, t); > } > @@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, > tree name, > the forward-reference will be altered into a real type. */ > >ref = make_node (code); > - if (flag_isoc23 && code != ENUMERAL_TYPE) > + if (flag_isoc23 || code == ENUMERAL_TYPE) > SET_TYPE_STRUCTURAL_EQUALITY (ref); >if (code == ENUMERAL_TYPE) > { > @@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents > *the_enum, tree name, > { >enumtype = make_node (ENUMERAL_TYPE); >TYPE_SIZE (enumtype) = NULL_TREE; > + SET_TYPE_STRUCTURAL_EQUALITY (enumtype); >pushtag (loc, name, enumtype); >if (fixed_underlying_type != NULL_TREE) > { > @@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents > *the_enum, tree name, > TYPE_SIZE (enumtype) = NULL_TREE; > TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type); > ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type; > + TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type); > + c_update_type_canonical (enumtype); > layout_type (enumtype); > } > } > @@ -10094,6
[gcc r15-912] C23: fix aliasing for structures/unions with incomplete types
https://gcc.gnu.org/g:86b98d939989427ff025bcfd536ad361fcdc699c commit r15-912-g86b98d939989427ff025bcfd536ad361fcdc699c Author: Martin Uecker Date: Sat Mar 30 19:49:48 2024 +0100 C23: fix aliasing for structures/unions with incomplete types When incomplete structure/union types are completed later, compatibility of struct types that contain pointers to such types changes. When forming equivalence classes for TYPE_CANONICAL, we therefor need to be conservative and treat all structs with the same tag which are pointer targets as equivalent for purposed of determining equivalency of structure/union types which contain such types as member. This avoids having to update TYPE_CANONICAL of such structure/unions recursively. The pointer types themselves are updated in c_update_type_canonical. gcc/c/ * c-typeck.cc (comptypes_internal): Add flag to track whether a struct is the target of a pointer. (tagged_types_tu_compatible): When forming equivalence classes, treat nested pointed-to structs as equivalent. gcc/testsuite/ * gcc.dg/c23-tag-incomplete-alias-1.c: New test. Diff: --- gcc/c/c-typeck.cc | 43 +-- gcc/testsuite/gcc.dg/c23-tag-incomplete-alias-1.c | 36 +++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index ad4c7add562..09b2c265a46 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -1172,6 +1172,7 @@ struct comptypes_data { bool different_types_p; bool warning_needed; bool anon_field; + bool pointedto; bool equiv; const struct tagged_tu_seen_cache* cache; @@ -1235,8 +1236,36 @@ comptypes_check_different_types (tree type1, tree type2, } -/* Like comptypes, but if it returns nonzero for struct and union - types considered equivalent for aliasing purposes. */ +/* Like comptypes, but if it returns true for struct and union types + considered equivalent for aliasing purposes, i.e. for setting + TYPE_CANONICAL after completing a struct or union. + + This function must return false only for types which are not + compatible according to C language semantics (cf. comptypes), + otherwise the middle-end would make incorrect aliasing decisions. + It may return true for some similar types that are not compatible + according to those stricter rules. + + In particular, we ignore size expression in arrays so that the + following structs are in the same equivalence class: + + struct foo { char (*buf)[]; }; + struct foo { char (*buf)[3]; }; + struct foo { char (*buf)[4]; }; + + We also treat unions / structs with members which are pointers to + structures or unions with the same tag as equivalent (if they are not + incompatible for other reasons). Although incomplete structure + or union types are not compatible to any other type, they may become + compatible to different types when completed. To avoid having to update + TYPE_CANONICAL at this point, we only consider the tag when forming + the equivalence classes. For example, the following types with tag + 'foo' are all considered equivalent: + + struct bar; + struct foo { struct bar *x }; + struct foo { struct bar { int a; } *x }; + struct foo { struct bar { char b; } *x }; */ bool comptypes_equiv_p (tree type1, tree type2) @@ -1357,6 +1386,7 @@ comptypes_internal (const_tree type1, const_tree type2, /* Do not remove mode information. */ if (TYPE_MODE (t1) != TYPE_MODE (t2)) return false; + data->pointedto = true; return comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2), data); case FUNCTION_TYPE: @@ -1375,7 +1405,7 @@ comptypes_internal (const_tree type1, const_tree type2, if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) data->different_types_p = true; - /* Ignore size mismatches. */ + /* Ignore size mismatches when forming equivalence classes. */ if (data->equiv) return true; /* Sizes must match unless one is missing or variable. */ @@ -1515,6 +1545,12 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2, if (TYPE_NAME (t1) != TYPE_NAME (t2)) return false; + /* When forming equivalence classes for TYPE_CANONICAL in C23, we treat + structs with the same tag as equivalent, but only when they are targets + of pointers inside other structs. */ + if (data->equiv && data->pointedto) +return true; + if (!data->anon_field && NULL_TREE == TYPE_NAME (t1)) return false; @@ -1610,6 +1646,7 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2, return false; data->anon_field = !DECL_NAME (s1); + data->pointedto = false; data->cache = if (!comptypes_internal (TREE_TYPE (s1), TREE_TYPE (s2), d
Re: [C23 PATCH]: allow aliasing for types derived from structs with variable size
On Sun, 26 May 2024, Martin Uecker wrote: > +/* Helper function for comptypes. For two compatible types, return 1 > + if they pass consistency checks. In particular we test that > + TYPE_CANONICAL ist set correctly, i.e. the two types can alias. */ s/ist/is/. OK with that fix. -- Joseph S. Myers josmy...@redhat.com
Re: [C23 PATCH, v2] fix aliasing for structures/unions with incomplete types
On Sun, 26 May 2024, Martin Uecker wrote: > This is the patch I sent previously, but I tried to improve the > description and added a long comment. This patch is needed so > that we do not have to update TYPE_CANONICAL of structures / unions > when a tagged type is completed that is (recursively) pointed to > by a member of the structure / union. > > Bootstrapped and regression tested on x86_64. > > > C23: fix aliasing for structures/unions with incomplete types > > When incomplete structure/union types are completed later, compatibility > of struct types that contain pointers to such types changes. When forming > equivalence classes for TYPE_CANONICAL, we therefor need to be > conservative > and treat all structs with the same tag which are pointer targets as > equivalent for purposed of determining equivalency of structure/union > types which contain such types as member. This avoids having to update > TYPE_CANONICAL of such structure/unions recursively. The pointer types > themselves are updated in c_update_type_canonical. > > gcc/c/ > * c-typeck.cc (comptypes_internal): Add flag to track > whether a struct is the target of a pointer. > (tagged_types_tu_compatible): When forming equivalence > classes, treat nested pointed-to structs as equivalent. > > gcc/testsuite/ > * gcc.dg/c23-tag-incomplete-alias-1.c: New test. This patch is OK. -- Joseph S. Myers josmy...@redhat.com
Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Fri, 24 May 2024, Martin Uecker wrote: > This is another version of this patch with two changes: > > - I added a fix (with test) for PR 115177 which is just the same > issue for hardbools which are internally implemented as enums. > > - I fixed the golang issue. Since the addition of the main variant > to the seen decls is unconditional I removed also the addition > of the type itself which now seems unnecessary. > > Bootstrapped and regression tested on x86_64. The front-end changes and the testcases are OK. -- Joseph S. Myers josmy...@redhat.com
[C23 PATCH]: allow aliasing for types derived from structs with variable size
This is similar to the enum issue, where setting the alias set to zero is insufficient because also derived types need to be able to alias. After this change, it is also possible to add checking assertions that verify TYPE_CANONICAL for all tagged types at end of each call to the comptypes family. At least for the test suite this then shows no additional issues for tagged types.  (We still make incorrect aliasing decisions at least for types derived from functions types (PR114959) but this is an ancient issue. We probably should set TYPE_CANONICAL in the FE similar to what is done for tagged types in C23.) Bootstrapped and regression tested on x86_64. C23: allow aliasing for types derived from structs with variable size Previously, we set the aliasing set of structures with variable size struct foo { int x[n]; char b; }; to zero. The reason is that such types can be compatible to diffrent structure types which are incompatible. struct foo { int x[2]; char b; }; struct foo { int x[3]; char b; }; But it is not enough to set the aliasing set to zero, because derived types would then still end up in different equivalence classes even though they might be compatible. Instead those types should be set to structural equivalency. We also add checking assertions that ensure that TYPE_CANONICAL is set correctly for all tagged types gcc/c/ * c-decl.cc (finish_struct): Do not set TYPE_CANONICAL for structure or unions with variable size. * c-objc-common.cc (c_get_alias_set): Do not set alias set to zero. * c-typeck.cc (comptypes_verify): New function. (comptypes,comptypes_same_p,comptypes_check_enum_int): Add assertion. (comptypes_equiv_p): Add assertion that ensures that compatible types have the same equivalence class. (tagged_types_tu_compatible_p): Remove now unneeded special case. gcc/testsuite/ * gcc.dg/gnu23-tag-alias-8.c: New test. diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index 6e6606c9570..9f7d55c0b10 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -9749,7 +9749,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, C_TYPE_BEING_DEFINED (t) = 0; /* Set type canonical based on equivalence class. */ - if (flag_isoc23) + if (flag_isoc23 && !C_TYPE_VARIABLE_SIZE (t)) { if (c_struct_htab == NULL) c_struct_htab = hash_table::create_ggc (61); diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index 551ec6f4b65..29127d037e1 100644 --- a/gcc/c/c-objc-common.cc +++ b/gcc/c/c-objc-common.cc @@ -420,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED) alias_set_type c_get_alias_set (tree t) { - /* Structs with variable size can alias different incompatible - structs. Let them alias anything. */ - if (RECORD_OR_UNION_TYPE_P (t) && C_TYPE_VARIABLE_SIZE (t)) -return 0; - return c_common_get_alias_set (t); } diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 2d092357e0f..c07e2f2b5cf 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -1167,6 +1167,28 @@ common_type (tree t1, tree t2) return c_common_type (t1, t2); } + + +/* Helper function for comptypes. For two compatible types, return 1 + if they pass consistency checks. In particular we test that + TYPE_CANONICAL ist set correctly, i.e. the two types can alias. */ + +static bool +comptypes_verify (tree type1, tree type2) +{ + if (TYPE_CANONICAL (type1) != TYPE_CANONICAL (type2) + && !TYPE_STRUCTURAL_EQUALITY_P (type1) + && !TYPE_STRUCTURAL_EQUALITY_P (type2)) +{ + /* FIXME: check other types. */ + if (RECORD_OR_UNION_TYPE_P (type1) + || TREE_CODE (type1) == ENUMERAL_TYPE + || TREE_CODE (type2) == ENUMERAL_TYPE) + return false; +} + return true; +} + struct comptypes_data { bool enum_and_int_p; bool different_types_p; @@ -1187,6 +1209,8 @@ comptypes (tree type1, tree type2) struct comptypes_data data = { }; bool ret = comptypes_internal (type1, type2, ); + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + return ret ? (data.warning_needed ? 2 : 1) : 0; } @@ -1200,6 +1224,8 @@ comptypes_same_p (tree type1, tree type2) struct comptypes_data data = { }; bool ret = comptypes_internal (type1, type2, ); + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + if (data.different_types_p) return false; @@ -1217,6 +1243,8 @@ comptypes_check_enum_int (tree type1, tree type2, bool *enum_and_int_p) bool ret = comptypes_internal (type1, type2, ); *enum_and_int_p = data.enum_and_int_p; + gcc_checking_assert (!ret || comptypes_verify (type1, type2)); + return ret ? (data.warning_needed ? 2 : 1) : 0; } @@ -1231,6 +1259,8 @@ comptypes_check_different_types (tree type1, tree type2,
[C23 PATCH, v2] fix aliasing for structures/unions with incomplete types
This is the patch I sent previously, but I tried to improve the description and added a long comment. This patch is needed so that we do not have to update TYPE_CANONICAL of structures / unions when a tagged type is completed that is (recursively) pointed to by a member of the structure / union. Bootstrapped and regression tested on x86_64. C23: fix aliasing for structures/unions with incomplete types When incomplete structure/union types are completed later, compatibility of struct types that contain pointers to such types changes. When forming equivalence classes for TYPE_CANONICAL, we therefor need to be conservative and treat all structs with the same tag which are pointer targets as equivalent for purposed of determining equivalency of structure/union types which contain such types as member. This avoids having to update TYPE_CANONICAL of such structure/unions recursively. The pointer types themselves are updated in c_update_type_canonical. gcc/c/ * c-typeck.cc (comptypes_internal): Add flag to track whether a struct is the target of a pointer. (tagged_types_tu_compatible): When forming equivalence classes, treat nested pointed-to structs as equivalent. gcc/testsuite/ * gcc.dg/c23-tag-incomplete-alias-1.c: New test. diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index c07e2f2b5cf..7a14ef1868f 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -1194,6 +1194,7 @@ struct comptypes_data { bool different_types_p; bool warning_needed; bool anon_field; + bool pointedto; bool equiv; const struct tagged_tu_seen_cache* cache; @@ -1265,8 +1266,36 @@ comptypes_check_different_types (tree type1, tree type2, } -/* Like comptypes, but if it returns nonzero for struct and union - types considered equivalent for aliasing purposes. */ +/* Like comptypes, but if it returns true for struct and union types + considered equivalent for aliasing purposes, i.e. for setting + TYPE_CANONICAL after completing a struct or union. + + This function must return false only for types which are not + compatible according to C language semantics (cf. comptypes), + otherwise the middle-end would make incorrect aliasing decisions. + It may return true for some similar types that are not compatible + according to those stricter rules. + + In particular, we ignore size expression in arrays so that the + following structs are in the same equivalence class: + + struct foo { char (*buf)[]; }; + struct foo { char (*buf)[3]; }; + struct foo { char (*buf)[4]; }; + + We also treat unions / structs with members which are pointers to + structures or unions with the same tag as equivalent (if they are not + incompatible for other reasons). Although incomplete structure + or union types are not compatible to any other type, they may become + compatible to different types when completed. To avoid having to update + TYPE_CANONICAL at this point, we only consider the tag when forming + the equivalence classes. For example, the following types with tag + 'foo' are all considered equivalent: + + struct bar; + struct foo { struct bar *x }; + struct foo { struct bar { int a; } *x }; + struct foo { struct bar { char b; } *x }; */ bool comptypes_equiv_p (tree type1, tree type2) @@ -1391,6 +1420,7 @@ comptypes_internal (const_tree type1, const_tree type2, /* Do not remove mode information. */ if (TYPE_MODE (t1) != TYPE_MODE (t2)) return false; + data->pointedto = true; return comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2), data); case FUNCTION_TYPE: @@ -1409,7 +1439,7 @@ comptypes_internal (const_tree type1, const_tree type2, if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) data->different_types_p = true; - /* Ignore size mismatches. */ + /* Ignore size mismatches when forming equivalence classes. */ if (data->equiv) return true; /* Sizes must match unless one is missing or variable. */ @@ -1549,6 +1579,12 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2, if (TYPE_NAME (t1) != TYPE_NAME (t2)) return false; + /* When forming equivalence classes for TYPE_CANONICAL in C23, we treat + structs with the same tag as equivalent, but only when they are targets + of pointers inside other structs. */ + if (data->equiv && data->pointedto) +return true; + if (!data->anon_field && NULL_TREE == TYPE_NAME (t1)) return false; @@ -1641,6 +1677,7 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2, return false; data->anon_field = !DECL_NAME (s1); + data->pointedto = false; data->cache = if (!comptypes_internal (TREE_TYPE (s1), TREE_TYPE (s2), data)) diff --git a/gcc/testsuite/gcc.dg/c23-tag-incom
Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Fri, May 24, 2024 at 05:39:45PM +0200, Martin Uecker wrote: > PR 115157 > PR 115177 > > gcc/c/ > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > * c-obj-common.cc (get_alias_set): Remove special case. > (get_aka_type): Add special case. > > gcc/c-family/ > * c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL > for hardbools. > > gcc/ > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > of TYPE_CANONICAL. Just a nit: s/use/Use/ Jakub
[C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]
This is another version of this patch with two changes: - I added a fix (with test) for PR 115177 which is just the same issue for hardbools which are internally implemented as enums. - I fixed the golang issue. Since the addition of the main variant to the seen decls is unconditional I removed also the addition of the type itself which now seems unnecessary. Bootstrapped and regression tested on x86_64. Martin C: allow aliasing of compatible types derived from enumeral types [PR115157] Aliasing of enumeral types with the underlying integer is now allowed by setting the aliasing set to zero. But this does not allow aliasing of derived types which are compatible as required by ISO C. Instead, initially set structural equality. Then set TYPE_CANONICAL and update pointers and main variants when the type is completed (as done for structures and unions in C23). PR 115157 PR 115177 gcc/c/ * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. * c-obj-common.cc (get_alias_set): Remove special case. (get_aka_type): Add special case. gcc/c-family/ * c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL for hardbools. gcc/ * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead of TYPE_CANONICAL. gcc/testsuite/ * gcc.dg/enum-alias-1.c: New test. * gcc.dg/enum-alias-2.c: New test. * gcc.dg/enum-alias-3.c: New test. * gcc.dg/enum-alias-4.c: New test. diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc index 04e39b41bdf..033395093b6 100644 --- a/gcc/c-family/c-attribs.cc +++ b/gcc/c-family/c-attribs.cc @@ -1074,6 +1074,7 @@ handle_hardbool_attribute (tree *node, tree name, tree args, TREE_SET_CODE (*node, ENUMERAL_TYPE); ENUM_UNDERLYING_TYPE (*node) = orig; + TYPE_CANONICAL (*node) = TYPE_CANONICAL (orig); tree false_value; if (args) diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index b691b91b3db..6e6606c9570 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) if (t == NULL_TREE) { t = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (t); pushtag (input_location, name, t); } @@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, the forward-reference will be altered into a real type. */ ref = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (ref); if (code == ENUMERAL_TYPE) { @@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, { enumtype = make_node (ENUMERAL_TYPE); TYPE_SIZE (enumtype) = NULL_TREE; + SET_TYPE_STRUCTURAL_EQUALITY (enumtype); pushtag (loc, name, enumtype); if (fixed_underlying_type != NULL_TREE) { @@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, TYPE_SIZE (enumtype) = NULL_TREE; TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type); ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type; + TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type); + c_update_type_canonical (enumtype); layout_type (enumtype); } } @@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree attributes) ENUM_UNDERLYING_TYPE (enumtype) = c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem)); + TYPE_CANONICAL (enumtype) = + TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype)); + c_update_type_canonical (enumtype); + layout_type (enumtype); } diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index b7c72d2609c..551ec6f4b65 100644 --- a/gcc/c/c-objc-common.cc +++ b/gcc/c/c-objc-common.cc @@ -130,6 +130,8 @@ get_aka_type (tree type) result = get_aka_type (orig_type); } + else if (TREE_CODE (type) == ENUMERAL_TYPE) +return type; else { tree canonical = TYPE_CANONICAL (type); @@ -418,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED) alias_set_type c_get_alias_set (tree t) { - /* Allow aliasing between enumeral types and the underlying - integer type. This is required since those are compatible types. */ - if (TREE_CODE (t) == ENUMERAL_TYPE) -return get_alias_set (ENUM_UNDERLYING_TYPE (t)); - /* Structs with variable size can alias different incompatible
Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Thu, 23 May 2024, Ian Lance Taylor wrote: > On Thu, May 23, 2024 at 2:48 PM Martin Uecker wrote: > > > > Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor: > > > On Thu, May 23, 2024 at 2:00 PM Joseph Myers wrote: > > > > > > > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > > > > > > > C: allow aliasing of compatible types derived from enumeral types > > > > > [PR115157] > > > > > > > > > > Aliasing of enumeral types with the underlying integer is now > > > > > allowed > > > > > by setting the aliasing set to zero. But this does not allow > > > > > aliasing > > > > > of derived types which are compatible as required by ISO C. > > > > > Instead, > > > > > initially set structural equality. Then set TYPE_CANONICAL and > > > > > update > > > > > pointers and main variants when the type is completed (as done for > > > > > structures and unions in C23). > > > > > > > > > > PR 115157 > > > > > > > > > > gcc/c/ > > > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / > > > > > TYPE_CANONICAL. > > > > > * c-obj-common.cc (get_alias_set): Remove special case. > > > > > (get_aka_type): Add special case. > > > > > > > > > > gcc/ > > > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT > > > > > instead > > > > > of TYPE_CANONICAL. > > > > > > > > > > gcc/testsuite/ > > > > > * gcc.dg/enum-alias-1.c: New test. > > > > > * gcc.dg/enum-alias-2.c: New test. > > > > > * gcc.dg/enum-alias-3.c: New test. > > > > > > > > OK, in the absence of objections on middle-end or Go grounds within the > > > > next week. > > > > > > The godump.cc patch is > > > > > >&& (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE > > > || !container->decls_seen.contains > > > - (TYPE_CANONICAL (TREE_TYPE (decl) > > > + (TYPE_MAIN_VARIANT (TREE_TYPE > > > (decl) > > > { > > > > > > What is the problem you are seeing? > > > > Test failures in godump-1.c > > > > > > > > This patch isn't right: > > > > > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This > > > patch is changing the latter X but not the former. They should be > > > consistent. > > > > Maybe the X == NULL_TREE can be removed if we > > add TYPE_MAIN_VARIANTs instead? > > If TYPE_MAIN_VARIANT is never NULL_TREE, then I agree that the > NULL_TREE test can be removed. TYPE_MAIN_VARIANT is indeed never NULL_TREE. Richard.
Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Thu, May 23, 2024 at 2:48 PM Martin Uecker wrote: > > Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor: > > On Thu, May 23, 2024 at 2:00 PM Joseph Myers wrote: > > > > > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > > > > > C: allow aliasing of compatible types derived from enumeral types > > > > [PR115157] > > > > > > > > Aliasing of enumeral types with the underlying integer is now > > > > allowed > > > > by setting the aliasing set to zero. But this does not allow > > > > aliasing > > > > of derived types which are compatible as required by ISO C. > > > > Instead, > > > > initially set structural equality. Then set TYPE_CANONICAL and > > > > update > > > > pointers and main variants when the type is completed (as done for > > > > structures and unions in C23). > > > > > > > > PR 115157 > > > > > > > > gcc/c/ > > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / > > > > TYPE_CANONICAL. > > > > * c-obj-common.cc (get_alias_set): Remove special case. > > > > (get_aka_type): Add special case. > > > > > > > > gcc/ > > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT > > > > instead > > > > of TYPE_CANONICAL. > > > > > > > > gcc/testsuite/ > > > > * gcc.dg/enum-alias-1.c: New test. > > > > * gcc.dg/enum-alias-2.c: New test. > > > > * gcc.dg/enum-alias-3.c: New test. > > > > > > OK, in the absence of objections on middle-end or Go grounds within the > > > next week. > > > > The godump.cc patch is > > > >&& (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE > > || !container->decls_seen.contains > > - (TYPE_CANONICAL (TREE_TYPE (decl) > > + (TYPE_MAIN_VARIANT (TREE_TYPE (decl) > > { > > > > What is the problem you are seeing? > > Test failures in godump-1.c > > > > > This patch isn't right: > > > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This > > patch is changing the latter X but not the former. They should be > > consistent. > > Maybe the X == NULL_TREE can be removed if we > add TYPE_MAIN_VARIANTs instead? If TYPE_MAIN_VARIANT is never NULL_TREE, then I agree that the NULL_TREE test can be removed. Ian
Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]
Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor: > On Thu, May 23, 2024 at 2:00 PM Joseph Myers wrote: > > > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > > > C: allow aliasing of compatible types derived from enumeral types > > > [PR115157] > > > > > > Aliasing of enumeral types with the underlying integer is now allowed > > > by setting the aliasing set to zero. But this does not allow aliasing > > > of derived types which are compatible as required by ISO C. Instead, > > > initially set structural equality. Then set TYPE_CANONICAL and update > > > pointers and main variants when the type is completed (as done for > > > structures and unions in C23). > > > > > > PR 115157 > > > > > > gcc/c/ > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / > > > TYPE_CANONICAL. > > > * c-obj-common.cc (get_alias_set): Remove special case. > > > (get_aka_type): Add special case. > > > > > > gcc/ > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > > > of TYPE_CANONICAL. > > > > > > gcc/testsuite/ > > > * gcc.dg/enum-alias-1.c: New test. > > > * gcc.dg/enum-alias-2.c: New test. > > > * gcc.dg/enum-alias-3.c: New test. > > > > OK, in the absence of objections on middle-end or Go grounds within the > > next week. > > The godump.cc patch is > >&& (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE > || !container->decls_seen.contains > - (TYPE_CANONICAL (TREE_TYPE (decl) > + (TYPE_MAIN_VARIANT (TREE_TYPE (decl) > { > > What is the problem you are seeing? Test failures in godump-1.c > > This patch isn't right: > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This > patch is changing the latter X but not the former. They should be > consistent. Maybe the X == NULL_TREE can be removed if we add TYPE_MAIN_VARIANTs instead? > > 2) At the bottom of that conditional block is code that adds a value > to container->decls_seen. Today that code is adding TYPE_CANONICAL. > If we change the condition to test TYPE_MAIN_VARIANT, then we need to > add TYPE_MAIN_VARIANT to decls_seen. Yes, obviously this is wrong. Thanks! Martin > > Hope that makes sense. > > I don't know why the patch is required, but it's fine with those > changes as long as the libgo tests continue to pass. > > Ian
Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Thu, May 23, 2024 at 2:00 PM Joseph Myers wrote: > > On Tue, 21 May 2024, Martin Uecker wrote: > > > > C: allow aliasing of compatible types derived from enumeral types > > [PR115157] > > > > Aliasing of enumeral types with the underlying integer is now allowed > > by setting the aliasing set to zero. But this does not allow aliasing > > of derived types which are compatible as required by ISO C. Instead, > > initially set structural equality. Then set TYPE_CANONICAL and update > > pointers and main variants when the type is completed (as done for > > structures and unions in C23). > > > > PR 115157 > > > > gcc/c/ > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > > * c-obj-common.cc (get_alias_set): Remove special case. > > (get_aka_type): Add special case. > > > > gcc/ > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > > of TYPE_CANONICAL. > > > > gcc/testsuite/ > > * gcc.dg/enum-alias-1.c: New test. > > * gcc.dg/enum-alias-2.c: New test. > > * gcc.dg/enum-alias-3.c: New test. > > OK, in the absence of objections on middle-end or Go grounds within the > next week. The godump.cc patch is && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE || !container->decls_seen.contains - (TYPE_CANONICAL (TREE_TYPE (decl) + (TYPE_MAIN_VARIANT (TREE_TYPE (decl) { What is the problem you are seeing? This patch isn't right: 1) The code is saying if "X == NULL_TREE || !already_seen(X)". This patch is changing the latter X but not the former. They should be consistent. 2) At the bottom of that conditional block is code that adds a value to container->decls_seen. Today that code is adding TYPE_CANONICAL. If we change the condition to test TYPE_MAIN_VARIANT, then we need to add TYPE_MAIN_VARIANT to decls_seen. Hope that makes sense. I don't know why the patch is required, but it's fine with those changes as long as the libgo tests continue to pass. Ian
Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]
On Tue, 21 May 2024, Martin Uecker wrote: > > C: allow aliasing of compatible types derived from enumeral types > [PR115157] > > Aliasing of enumeral types with the underlying integer is now allowed > by setting the aliasing set to zero. But this does not allow aliasing > of derived types which are compatible as required by ISO C. Instead, > initially set structural equality. Then set TYPE_CANONICAL and update > pointers and main variants when the type is completed (as done for > structures and unions in C23). > > PR 115157 > > gcc/c/ > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. > * c-obj-common.cc (get_alias_set): Remove special case. > (get_aka_type): Add special case. > > gcc/ > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead > of TYPE_CANONICAL. > > gcc/testsuite/ > * gcc.dg/enum-alias-1.c: New test. > * gcc.dg/enum-alias-2.c: New test. > * gcc.dg/enum-alias-3.c: New test. OK, in the absence of objections on middle-end or Go grounds within the next week. -- Joseph S. Myers josmy...@redhat.com
[C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]
For enum and integer we allow aliasing by specifically returning via a langhook the aliasing set of the underlying type. But this is not sufficient for derived types, i.e. pointers to enums and pointers to compatible integers also need to have the same aliasing set. We also allow forward declarations of enums which is a GNU extension, but I think this has to work consistently too, so we here have the same issue as in C23 with other tagged types. The solution in this patch is similar to what we do in C23, i.e. we start out with structural equality and then set TYPE_CANONICAL to the underlying type. The only way to make the TYPE_CANONICAL system work with the C rules for type compatility seems to set TYPE_CANONICAL to the same type for all types in a compatibility equivalence class (as compatibility is not transitive this puts together similar types that are not compatible). This is the underlying type in this case. As all types in such an equivalence class have the same representation, so this should always work in my opinion (but maybe there is some middle end aspects I am still missing). When testing, I so far only found two minor issues, i.e. when computing the 'aka' type in diagnostics and an issue with godump.cc (not sure I fixed this correctly). Beyond this patch, we need also some change for function types in general and there are problably also some other issues related to incomplete arrays as well (I added some checking to 'comptypes' to check that all types ruled compatible by the C FE also have either structural equality, or have the same TYPE_CANONICAL, and this brings up some more inconsistencies). Thoughts? Bootstrapped and regression tested on x86_64 (only C, C++ so far). C: allow aliasing of compatible types derived from enumeral types [PR115157] Aliasing of enumeral types with the underlying integer is now allowed by setting the aliasing set to zero. But this does not allow aliasing of derived types which are compatible as required by ISO C. Instead, initially set structural equality. Then set TYPE_CANONICAL and update pointers and main variants when the type is completed (as done for structures and unions in C23). PR 115157 gcc/c/ * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum, finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL. * c-obj-common.cc (get_alias_set): Remove special case. (get_aka_type): Add special case. gcc/ * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead of TYPE_CANONICAL. gcc/testsuite/ * gcc.dg/enum-alias-1.c: New test. * gcc.dg/enum-alias-2.c: New test. * gcc.dg/enum-alias-3.c: New test. diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index b691b91b3db..6e6606c9570 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) if (t == NULL_TREE) { t = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (t); pushtag (input_location, name, t); } @@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, the forward-reference will be altered into a real type. */ ref = make_node (code); - if (flag_isoc23 && code != ENUMERAL_TYPE) + if (flag_isoc23 || code == ENUMERAL_TYPE) SET_TYPE_STRUCTURAL_EQUALITY (ref); if (code == ENUMERAL_TYPE) { @@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, { enumtype = make_node (ENUMERAL_TYPE); TYPE_SIZE (enumtype) = NULL_TREE; + SET_TYPE_STRUCTURAL_EQUALITY (enumtype); pushtag (loc, name, enumtype); if (fixed_underlying_type != NULL_TREE) { @@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents *the_enum, tree name, TYPE_SIZE (enumtype) = NULL_TREE; TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type); ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type; + TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type); + c_update_type_canonical (enumtype); layout_type (enumtype); } } @@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree attributes) ENUM_UNDERLYING_TYPE (enumtype) = c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem)); + TYPE_CANONICAL (enumtype) = + TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype)); + c_update_type_canonical (enumtype); + layout_type (enumtype); } diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index b7c72d2609c..55
[COMMITTED 26/31] ada: Fix strict aliasing violation in parameter passing
From: Eric Botcazou This fixes a long-standing (implicit) violation of the strict aliasing rules that occurs when the result of a call to an instance of Unchecked_Conversion is directly passed as an actual parameter in a call to a subprogram and the passing mechanism is by reference. In this case, the reference passed to the subprogram may be to a type that has nothing to do with the type of the underlying object, which is the definition of such a violation. This implements the following two-pronged approach: first, the problematic cases are detected and a reference to a temporary is passed instead of the direct reference to the underlying object; second, the implementation of pragma Universal_Aliasing is enhanced so that it is propagated from the component type of an array type to the array type itself, or else can be applied to the array type directly, and may therefore be used to prevent the violation from occurring in the first place, when the array type is involved in the Unchecked_Conversion. gcc/ada/ * gcc-interface/decl.cc (gnat_to_gnu_entity) : Set TYPE_TYPELESS_STORAGE on the array types if Universal_Aliasing is set on the type or its component type. : Likewise. For other aggregate types, set TYPE_TYPELESS_STORAGE in this case. (set_typeless_storage_on_aggregate_type): New function. (set_universal_aliasing_on_type): Likewise. * gcc-interface/trans.cc (Call_to_gnu): Add const to local variable. Adjust comment. Pass GNAT_NAME in the call to addressable_p and add a bypass for atomic types in case it returns false. (addressable_p): Add GNAT_EXPR third parameter with default value and add a default value to the existing second parameter. : Return false if the expression comes from a function call and if the alias sets of source and target types are both distinct from zero and each other. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/gcc-interface/decl.cc | 40 ++- gcc/ada/gcc-interface/trans.cc | 60 -- 2 files changed, 82 insertions(+), 18 deletions(-) diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc index 0987d534e69..ab54d2ccf13 100644 --- a/gcc/ada/gcc-interface/decl.cc +++ b/gcc/ada/gcc-interface/decl.cc @@ -205,6 +205,8 @@ static Entity_Id Gigi_Cloned_Subtype (Entity_Id); static tree gnu_ext_name_for_subprog (Entity_Id, tree); static void set_nonaliased_component_on_array_type (tree); static void set_reverse_storage_order_on_array_type (tree); +static void set_typeless_storage_on_aggregate_type (tree); +static void set_universal_aliasing_on_type (tree); static bool same_discriminant_p (Entity_Id, Entity_Id); static bool array_type_has_nonaliased_component (tree, Entity_Id); static bool compile_time_known_address_p (Node_Id); @@ -2385,6 +2387,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) set_reverse_storage_order_on_array_type (tem); if (array_type_has_nonaliased_component (tem, gnat_entity)) set_nonaliased_component_on_array_type (tem); + if (Universal_Aliasing (gnat_entity) + || Universal_Aliasing (Component_Type (gnat_entity))) + set_typeless_storage_on_aggregate_type (tem); } /* If this is a packed type implemented specially, then process the @@ -2790,6 +2795,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) set_reverse_storage_order_on_array_type (gnu_type); if (array_type_has_nonaliased_component (gnu_type, gnat_entity)) set_nonaliased_component_on_array_type (gnu_type); + if (Universal_Aliasing (gnat_entity) + || Universal_Aliasing (Component_Type (gnat_entity))) + set_typeless_storage_on_aggregate_type (gnu_type); /* Clear the TREE_OVERFLOW flag, if any, for null arrays. */ if (gnu_null_ranges[index]) @@ -4757,7 +4765,17 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) /* Record whether a pragma Universal_Aliasing was specified. */ if (Universal_Aliasing (gnat_entity) && !TYPE_IS_DUMMY_P (gnu_type)) - TYPE_UNIVERSAL_ALIASING_P (gnu_type) = 1; + { + /* Set TYPE_TYPELESS_STORAGE if this is an aggregate type and +TYPE_UNIVERSAL_ALIASING_P otherwise, since the former is not +available in the latter case Both will effectively put alias +set 0 on the type, but the former is more robust because it +will be streamed in LTO mode. */ + if (AGGREGATE_TYPE_P (gnu_type)) + set_typeless_storage_on_aggregate_type (gnu_type); + else + set_universal_aliasing_on_type (
[gcc r15-743] ada: Fix strict aliasing violation in parameter passing
https://gcc.gnu.org/g:f20a57edefe0cb185e57f82a15e887887b98d3ab commit r15-743-gf20a57edefe0cb185e57f82a15e887887b98d3ab Author: Eric Botcazou Date: Thu Feb 29 09:14:27 2024 +0100 ada: Fix strict aliasing violation in parameter passing This fixes a long-standing (implicit) violation of the strict aliasing rules that occurs when the result of a call to an instance of Unchecked_Conversion is directly passed as an actual parameter in a call to a subprogram and the passing mechanism is by reference. In this case, the reference passed to the subprogram may be to a type that has nothing to do with the type of the underlying object, which is the definition of such a violation. This implements the following two-pronged approach: first, the problematic cases are detected and a reference to a temporary is passed instead of the direct reference to the underlying object; second, the implementation of pragma Universal_Aliasing is enhanced so that it is propagated from the component type of an array type to the array type itself, or else can be applied to the array type directly, and may therefore be used to prevent the violation from occurring in the first place, when the array type is involved in the Unchecked_Conversion. gcc/ada/ * gcc-interface/decl.cc (gnat_to_gnu_entity) : Set TYPE_TYPELESS_STORAGE on the array types if Universal_Aliasing is set on the type or its component type. : Likewise. For other aggregate types, set TYPE_TYPELESS_STORAGE in this case. (set_typeless_storage_on_aggregate_type): New function. (set_universal_aliasing_on_type): Likewise. * gcc-interface/trans.cc (Call_to_gnu): Add const to local variable. Adjust comment. Pass GNAT_NAME in the call to addressable_p and add a bypass for atomic types in case it returns false. (addressable_p): Add GNAT_EXPR third parameter with default value and add a default value to the existing second parameter. : Return false if the expression comes from a function call and if the alias sets of source and target types are both distinct from zero and each other. Diff: --- gcc/ada/gcc-interface/decl.cc | 40 +++- gcc/ada/gcc-interface/trans.cc | 60 ++ 2 files changed, 82 insertions(+), 18 deletions(-) diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc index 0987d534e69..ab54d2ccf13 100644 --- a/gcc/ada/gcc-interface/decl.cc +++ b/gcc/ada/gcc-interface/decl.cc @@ -205,6 +205,8 @@ static Entity_Id Gigi_Cloned_Subtype (Entity_Id); static tree gnu_ext_name_for_subprog (Entity_Id, tree); static void set_nonaliased_component_on_array_type (tree); static void set_reverse_storage_order_on_array_type (tree); +static void set_typeless_storage_on_aggregate_type (tree); +static void set_universal_aliasing_on_type (tree); static bool same_discriminant_p (Entity_Id, Entity_Id); static bool array_type_has_nonaliased_component (tree, Entity_Id); static bool compile_time_known_address_p (Node_Id); @@ -2385,6 +2387,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) set_reverse_storage_order_on_array_type (tem); if (array_type_has_nonaliased_component (tem, gnat_entity)) set_nonaliased_component_on_array_type (tem); + if (Universal_Aliasing (gnat_entity) + || Universal_Aliasing (Component_Type (gnat_entity))) + set_typeless_storage_on_aggregate_type (tem); } /* If this is a packed type implemented specially, then process the @@ -2790,6 +2795,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) set_reverse_storage_order_on_array_type (gnu_type); if (array_type_has_nonaliased_component (gnu_type, gnat_entity)) set_nonaliased_component_on_array_type (gnu_type); + if (Universal_Aliasing (gnat_entity) + || Universal_Aliasing (Component_Type (gnat_entity))) + set_typeless_storage_on_aggregate_type (gnu_type); /* Clear the TREE_OVERFLOW flag, if any, for null arrays. */ if (gnu_null_ranges[index]) @@ -4757,7 +4765,17 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) /* Record whether a pragma Universal_Aliasing was specified. */ if (Universal_Aliasing (gnat_entity) && !TYPE_IS_DUMMY_P (gnu_type)) - TYPE_UNIVERSAL_ALIASING_P (gnu_type) = 1; + { + /* Set TYPE_TYPELESS_STORAGE if this is an aggregate type and +TYPE_UNIVERSAL_ALIASING_P otherwise, since the former is not +available in the latter case Both will effectively p
[COMMITTED 28/31] ada: Fix strict aliasing violation in parameter passing (continued)
From: Eric Botcazou This fixes another long-standing (implicit) violation of the strict aliasing rules that occurs when the result of a value conversion is directly passed as an actual parameter in a call to a subprogram and the passing mechanism is by reference. In this case, the reference passed to the subprogram may be to a type that is too different from the type of the underlying object, which is the definition of such a violation. The change reworks and strengthens the previous fix as follows: first, the detection of these violations is moved into a dedicated predicate; second, an assertion is added to check that none of them has been missed, which is triggered by either -fchecking or -fstrict-aliasing, as the closely related assertion that is present in relate_alias_sets. The assertion uncovered two internal sources of violations: implementation types for packed array types with peculiar index types and interface types, which are fixed by propagating alias sets in the first case and resorting to universal aliasing in the second case. Finally, an unconditional warning is implemented to inform the user that the temporary is created and to suggest a possible solution to prevent that. gcc/ada/ * gcc-interface/decl.cc (gnat_to_gnu_entity) : For a packed type implemented specially, temporarily save the XUA type as equivalent to the entity before processing the implementation type. For this implementation type, if its component type is the same as that of the original type, copy the alias set from the latter. : Resort to universal aliasing for all interface types. * gcc-interface/trans.cc (Call_to_gnu): Add GNU_ACTUAL_TYPE local variable and rename existing one to GNU_UNPADDED_ACTUAL_TYPE. If the formal is passed by reference and the actual is a conversion, call aliasable_p to detect aliasing violations, issue a warning upon finding one and create the temporary in the target type. Add an assertion that no such violation has been missed above. (addressable_p): Revert latest changes. (aliasable_p): New predicate. * gcc-interface/utils2.cc (build_binary_op) : When creating a new array type on the fly, preserve the alias set of the operation type. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/gcc-interface/decl.cc | 48 ++--- gcc/ada/gcc-interface/trans.cc | 167 +++- gcc/ada/gcc-interface/utils2.cc | 6 +- 3 files changed, 159 insertions(+), 62 deletions(-) diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc index ab54d2ccf13..6e40a157734 100644 --- a/gcc/ada/gcc-interface/decl.cc +++ b/gcc/ada/gcc-interface/decl.cc @@ -2119,6 +2119,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) case E_Array_Type: { + const Entity_Id OAT = Original_Array_Type (gnat_entity); const Entity_Id PAT = Packed_Array_Impl_Type (gnat_entity); const bool convention_fortran_p = (Convention (gnat_entity) == Convention_Fortran); @@ -2392,14 +2393,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) set_typeless_storage_on_aggregate_type (tem); } - /* If this is a packed type implemented specially, then process the - implementation type so it is elaborated in the proper scope. */ - if (Present (PAT)) - gnat_to_gnu_entity (PAT, NULL_TREE, false); - - /* Otherwise, if an alignment is specified, use it if valid and, if - the alignment was requested with an explicit clause, state so. */ - else if (Known_Alignment (gnat_entity)) + /* If an alignment is specified for an array that is not a packed type + implemented specially, use the alignment if it is valid and, if it + was requested with an explicit clause, preserve the information. */ + if (Known_Alignment (gnat_entity) && No (PAT)) { SET_TYPE_ALIGN (tem, validate_alignment (Alignment (gnat_entity), @@ -2418,7 +2415,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) TYPE_BIT_PACKED_ARRAY_TYPE_P (tem) = (Is_Packed_Array_Impl_Type (gnat_entity) -? Is_Bit_Packed_Array (Original_Array_Type (gnat_entity)) +? Is_Bit_Packed_Array (OAT) : Is_Bit_Packed_Array (gnat_entity)); if (Treat_As_Volatile (gnat_entity)) @@ -2447,8 +2444,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) TYPE_ARRAY_MAX_SIZE (tem) = gnu_max_size; /* See the above description for the rationale. */ - create_type_decl (create_concat_name (gnat_entity, "XUA"), tem, - artificial_p, debug_info_p, gnat_entity); + tree gnu_tmp_decl +
[gcc r15-745] ada: Fix strict aliasing violation in parameter passing (continued)
https://gcc.gnu.org/g:ccdef2aef6463159a596ef1a4afe6ddce9d0f2ea commit r15-745-gccdef2aef6463159a596ef1a4afe6ddce9d0f2ea Author: Eric Botcazou Date: Sun Mar 10 13:22:55 2024 +0100 ada: Fix strict aliasing violation in parameter passing (continued) This fixes another long-standing (implicit) violation of the strict aliasing rules that occurs when the result of a value conversion is directly passed as an actual parameter in a call to a subprogram and the passing mechanism is by reference. In this case, the reference passed to the subprogram may be to a type that is too different from the type of the underlying object, which is the definition of such a violation. The change reworks and strengthens the previous fix as follows: first, the detection of these violations is moved into a dedicated predicate; second, an assertion is added to check that none of them has been missed, which is triggered by either -fchecking or -fstrict-aliasing, as the closely related assertion that is present in relate_alias_sets. The assertion uncovered two internal sources of violations: implementation types for packed array types with peculiar index types and interface types, which are fixed by propagating alias sets in the first case and resorting to universal aliasing in the second case. Finally, an unconditional warning is implemented to inform the user that the temporary is created and to suggest a possible solution to prevent that. gcc/ada/ * gcc-interface/decl.cc (gnat_to_gnu_entity) : For a packed type implemented specially, temporarily save the XUA type as equivalent to the entity before processing the implementation type. For this implementation type, if its component type is the same as that of the original type, copy the alias set from the latter. : Resort to universal aliasing for all interface types. * gcc-interface/trans.cc (Call_to_gnu): Add GNU_ACTUAL_TYPE local variable and rename existing one to GNU_UNPADDED_ACTUAL_TYPE. If the formal is passed by reference and the actual is a conversion, call aliasable_p to detect aliasing violations, issue a warning upon finding one and create the temporary in the target type. Add an assertion that no such violation has been missed above. (addressable_p): Revert latest changes. (aliasable_p): New predicate. * gcc-interface/utils2.cc (build_binary_op) : When creating a new array type on the fly, preserve the alias set of the operation type. Diff: --- gcc/ada/gcc-interface/decl.cc | 48 gcc/ada/gcc-interface/trans.cc | 167 +--- gcc/ada/gcc-interface/utils2.cc | 6 +- 3 files changed, 159 insertions(+), 62 deletions(-) diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc index ab54d2ccf13..6e40a157734 100644 --- a/gcc/ada/gcc-interface/decl.cc +++ b/gcc/ada/gcc-interface/decl.cc @@ -2119,6 +2119,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) case E_Array_Type: { + const Entity_Id OAT = Original_Array_Type (gnat_entity); const Entity_Id PAT = Packed_Array_Impl_Type (gnat_entity); const bool convention_fortran_p = (Convention (gnat_entity) == Convention_Fortran); @@ -2392,14 +2393,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) set_typeless_storage_on_aggregate_type (tem); } - /* If this is a packed type implemented specially, then process the - implementation type so it is elaborated in the proper scope. */ - if (Present (PAT)) - gnat_to_gnu_entity (PAT, NULL_TREE, false); - - /* Otherwise, if an alignment is specified, use it if valid and, if - the alignment was requested with an explicit clause, state so. */ - else if (Known_Alignment (gnat_entity)) + /* If an alignment is specified for an array that is not a packed type + implemented specially, use the alignment if it is valid and, if it + was requested with an explicit clause, preserve the information. */ + if (Known_Alignment (gnat_entity) && No (PAT)) { SET_TYPE_ALIGN (tem, validate_alignment (Alignment (gnat_entity), @@ -2418,7 +2415,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) TYPE_BIT_PACKED_ARRAY_TYPE_P (tem) = (Is_Packed_Array_Impl_Type (gnat_entity) -? Is_Bit_Packed_Array (Original_Array_Type (gnat_entity)) +? Is_Bit_Packed_Array (OAT) : Is_Bit_Packed_Array (gnat_entity)); if (Treat_As_Volatile (gnat_entity)) @@ -2447,8 +2444,9 @@ gnat_to_gnu_entity (E
[COMMITTED 01/30] ada: Rework and augment documentation on strict aliasing
From: Eric Botcazou The documentation was originally centered around pragma No_Strict_Aliasing and pragma Universal_Aliasing was mentioned only as an afterthought. It also contained a warning about the usage of overlays implemented by means of address clauses that has been obsolete for long. gcc/ada/ * doc/gnat_rm/implementation_defined_pragmas.rst (Universal_Aliasing): Remove reference to No_Strict_Aliasing. * doc/gnat_ugn/gnat_and_program_execution.rst (Optimization and Strict Aliasinng): Simplify first example and make it more consistent with the second. Add description of the effects of pragma Universal_Aliasing and document new warning issued for unchecked conversions. Remove obsolete stuff. * gnat_rm.texi: Regenerate. * gnat_ugn.texi: Regenerate. Tested on x86_64-pc-linux-gnu, committed on master. --- .../implementation_defined_pragmas.rst| 7 +- .../gnat_ugn/gnat_and_program_execution.rst | 296 + gcc/ada/gnat_rm.texi | 7 +- gcc/ada/gnat_ugn.texi | 306 ++ 4 files changed, 353 insertions(+), 263 deletions(-) diff --git a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst index 7f221e32344..bcbd85984dc 100644 --- a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst +++ b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst @@ -6949,10 +6949,9 @@ Syntax: ``type_LOCAL_NAME`` must refer to a type declaration in the current declarative part. The effect is to inhibit strict type-based aliasing -optimization for the given type. In other words, the effect is as though -access types designating this type were subject to pragma No_Strict_Aliasing. -For a detailed description of the strict aliasing optimization, and the -situations in which it must be suppressed, see the section on +optimizations for the given type. For a detailed description of the +strict type-based aliasing optimizations and the situations in which +they need to be suppressed, see the section on ``Optimization and Strict Aliasing`` in the :title:`GNAT User's Guide`. .. _Pragma-Unmodified: diff --git a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst index 35e34772658..d502da87eb0 100644 --- a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst +++ b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst @@ -2072,37 +2072,36 @@ the following example: .. code-block:: ada - procedure R is + procedure M is type Int1 is new Integer; +I1 : Int1; + type Int2 is new Integer; -type Int1A is access Int1; -type Int2A is access Int2; -Int1V : Int1A; -Int2V : Int2A; +type A2 is access Int2; +V2 : A2; ... begin ... for J in Data'Range loop - if Data (J) = Int1V.all then - Int2V.all := Int2V.all + 1; + if Data (J) = I1 then + V2.all := V2.all + 1; end if; end loop; ... - end R; + end; -In this example, since the variable ``Int1V`` can only access objects -of type ``Int1``, and ``Int2V`` can only access objects of type -``Int2``, there is no possibility that the assignment to -``Int2V.all`` affects the value of ``Int1V.all``. This means that -the compiler optimizer can "know" that the value ``Int1V.all`` is constant -for all iterations of the loop and avoid the extra memory reference -required to dereference it each time through the loop. +In this example, since ``V2`` can only access objects of type ``Int2`` +and ``I1`` is not one of them, there is no possibility that the assignment +to ``V2.all`` affects the value of ``I1``. This means that the compiler +optimizer can infer that the value ``I1`` is constant for all iterations +of the loop and load it from memory only once, before entering the loop, +instead of in every iteration (this is called load hoisting). -This kind of optimization, called strict aliasing analysis, is +This kind of optimizations, based on strict type-based aliasing, is triggered by specifying an optimization level of :switch:`-O2` or -higher or :switch:`-Os` and allows GNAT to generate more efficient code -when access values are involved. +higher (or :switch:`-Os`) and allows the compiler to generate more +efficient code. However, although this optimization is always correct in terms of the formal semantics of the Ada Reference Manual, difficulties can @@ -2111,173 +2110,214 @@ the typing system. Consider the following complete program example: .. code-block:: ada - package p1 is - type int1 is new integer; - type int2 is new integer; - type a1 is access int1; - type a2 is access int2; - end p1; + package P1 is + type Int1 is n
[COMMITTED 05/30] ada: One more adjustment coming from aliasing considerations
From: Eric Botcazou It is needed on PowerPC platforms because of specific calling conventions. gcc/ada/ * libgnat/g-sothco.ads (In_Addr): Add aspect Universal_Aliasing. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/g-sothco.ads | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gcc/ada/libgnat/g-sothco.ads b/gcc/ada/libgnat/g-sothco.ads index 8c219333649..da1e6f5bcdd 100644 --- a/gcc/ada/libgnat/g-sothco.ads +++ b/gcc/ada/libgnat/g-sothco.ads @@ -123,10 +123,13 @@ package GNAT.Sockets.Thin_Common is type In_Addr is record S_B1, S_B2, S_B3, S_B4 : C.unsigned_char; - end record with Convention => C, Alignment => C.int'Alignment; + end record + with Convention => C, Alignment => C.int'Alignment, Universal_Aliasing; -- IPv4 address, represented as a network-order C.int. Note that the -- underlying operating system may assume that values of this type have - -- C.int alignment, so we need to provide a suitable alignment clause here. + -- C.int's alignment, so we need to provide a suitable alignment clause. + -- We also need to inhibit strict type-based aliasing optimizations in + -- order to implement the following unchecked conversions efficiently. function To_In_Addr is new Ada.Unchecked_Conversion (C.int, In_Addr); function To_Int is new Ada.Unchecked_Conversion (In_Addr, C.int); -- 2.43.2
[gcc r15-664] ada: One more adjustment coming from aliasing considerations
https://gcc.gnu.org/g:eef3025547ce55cbf6a9018b495ef5c9a562047a commit r15-664-geef3025547ce55cbf6a9018b495ef5c9a562047a Author: Eric Botcazou Date: Tue Mar 19 10:56:34 2024 +0100 ada: One more adjustment coming from aliasing considerations It is needed on PowerPC platforms because of specific calling conventions. gcc/ada/ * libgnat/g-sothco.ads (In_Addr): Add aspect Universal_Aliasing. Diff: --- gcc/ada/libgnat/g-sothco.ads | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gcc/ada/libgnat/g-sothco.ads b/gcc/ada/libgnat/g-sothco.ads index 8c2193336491..da1e6f5bcddf 100644 --- a/gcc/ada/libgnat/g-sothco.ads +++ b/gcc/ada/libgnat/g-sothco.ads @@ -123,10 +123,13 @@ package GNAT.Sockets.Thin_Common is type In_Addr is record S_B1, S_B2, S_B3, S_B4 : C.unsigned_char; - end record with Convention => C, Alignment => C.int'Alignment; + end record + with Convention => C, Alignment => C.int'Alignment, Universal_Aliasing; -- IPv4 address, represented as a network-order C.int. Note that the -- underlying operating system may assume that values of this type have - -- C.int alignment, so we need to provide a suitable alignment clause here. + -- C.int's alignment, so we need to provide a suitable alignment clause. + -- We also need to inhibit strict type-based aliasing optimizations in + -- order to implement the following unchecked conversions efficiently. function To_In_Addr is new Ada.Unchecked_Conversion (C.int, In_Addr); function To_Int is new Ada.Unchecked_Conversion (In_Addr, C.int);
[gcc r15-660] ada: Rework and augment documentation on strict aliasing
https://gcc.gnu.org/g:5d6c099ffaa384425f33d4e3a52f55149b9bc99a commit r15-660-g5d6c099ffaa384425f33d4e3a52f55149b9bc99a Author: Eric Botcazou Date: Wed Mar 13 17:05:12 2024 +0100 ada: Rework and augment documentation on strict aliasing The documentation was originally centered around pragma No_Strict_Aliasing and pragma Universal_Aliasing was mentioned only as an afterthought. It also contained a warning about the usage of overlays implemented by means of address clauses that has been obsolete for long. gcc/ada/ * doc/gnat_rm/implementation_defined_pragmas.rst (Universal_Aliasing): Remove reference to No_Strict_Aliasing. * doc/gnat_ugn/gnat_and_program_execution.rst (Optimization and Strict Aliasinng): Simplify first example and make it more consistent with the second. Add description of the effects of pragma Universal_Aliasing and document new warning issued for unchecked conversions. Remove obsolete stuff. * gnat_rm.texi: Regenerate. * gnat_ugn.texi: Regenerate. Diff: --- .../doc/gnat_rm/implementation_defined_pragmas.rst | 7 +- .../doc/gnat_ugn/gnat_and_program_execution.rst| 296 +++- gcc/ada/gnat_rm.texi | 7 +- gcc/ada/gnat_ugn.texi | 306 - 4 files changed, 353 insertions(+), 263 deletions(-) diff --git a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst index 7f221e323446..bcbd85984dc0 100644 --- a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst +++ b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst @@ -6949,10 +6949,9 @@ Syntax: ``type_LOCAL_NAME`` must refer to a type declaration in the current declarative part. The effect is to inhibit strict type-based aliasing -optimization for the given type. In other words, the effect is as though -access types designating this type were subject to pragma No_Strict_Aliasing. -For a detailed description of the strict aliasing optimization, and the -situations in which it must be suppressed, see the section on +optimizations for the given type. For a detailed description of the +strict type-based aliasing optimizations and the situations in which +they need to be suppressed, see the section on ``Optimization and Strict Aliasing`` in the :title:`GNAT User's Guide`. .. _Pragma-Unmodified: diff --git a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst index 35e347726589..d502da87eb0b 100644 --- a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst +++ b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst @@ -2072,37 +2072,36 @@ the following example: .. code-block:: ada - procedure R is + procedure M is type Int1 is new Integer; +I1 : Int1; + type Int2 is new Integer; -type Int1A is access Int1; -type Int2A is access Int2; -Int1V : Int1A; -Int2V : Int2A; +type A2 is access Int2; +V2 : A2; ... begin ... for J in Data'Range loop - if Data (J) = Int1V.all then - Int2V.all := Int2V.all + 1; + if Data (J) = I1 then + V2.all := V2.all + 1; end if; end loop; ... - end R; + end; -In this example, since the variable ``Int1V`` can only access objects -of type ``Int1``, and ``Int2V`` can only access objects of type -``Int2``, there is no possibility that the assignment to -``Int2V.all`` affects the value of ``Int1V.all``. This means that -the compiler optimizer can "know" that the value ``Int1V.all`` is constant -for all iterations of the loop and avoid the extra memory reference -required to dereference it each time through the loop. +In this example, since ``V2`` can only access objects of type ``Int2`` +and ``I1`` is not one of them, there is no possibility that the assignment +to ``V2.all`` affects the value of ``I1``. This means that the compiler +optimizer can infer that the value ``I1`` is constant for all iterations +of the loop and load it from memory only once, before entering the loop, +instead of in every iteration (this is called load hoisting). -This kind of optimization, called strict aliasing analysis, is +This kind of optimizations, based on strict type-based aliasing, is triggered by specifying an optimization level of :switch:`-O2` or -higher or :switch:`-Os` and allows GNAT to generate more efficient code -when access values are involved. +higher (or :switch:`-Os`) and allows the compiler to generate more +efficient code. However, although this optimization is always correct in terms of the formal semantics of the Ada Reference Manual, difficulties can @@ -2111,173 +2110,214 @@ the typing system. Consider the following complet
[COMMITTED 30/35] ada: Further adjustments coming from aliasing considerations
From: Eric Botcazou They are needed on 32-bit platforms because of different calling conventions and again in the units implementing AltiVec and Streams support. gcc/ada/ * libgnat/g-alvevi.ads: Add pragma Universal_Aliasing for all the view types. * libgnat/s-stratt.ads: Likewise for Fat_Pointer type. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/libgnat/g-alvevi.ads | 11 +++ gcc/ada/libgnat/s-stratt.ads | 3 +++ 2 files changed, 14 insertions(+) diff --git a/gcc/ada/libgnat/g-alvevi.ads b/gcc/ada/libgnat/g-alvevi.ads index b2beac7284c..b0f58790adf 100644 --- a/gcc/ada/libgnat/g-alvevi.ads +++ b/gcc/ada/libgnat/g-alvevi.ads @@ -58,6 +58,7 @@ package GNAT.Altivec.Vector_Views is type VUC_View is record Values : Varray_unsigned_char; end record; + pragma Universal_Aliasing (VUC_View); type Varray_signed_char is array (Vchar_Range) of signed_char; for Varray_signed_char'Alignment use VECTOR_ALIGNMENT; @@ -65,6 +66,7 @@ package GNAT.Altivec.Vector_Views is type VSC_View is record Values : Varray_signed_char; end record; + pragma Universal_Aliasing (VSC_View); type Varray_bool_char is array (Vchar_Range) of bool_char; for Varray_bool_char'Alignment use VECTOR_ALIGNMENT; @@ -72,6 +74,7 @@ package GNAT.Altivec.Vector_Views is type VBC_View is record Values : Varray_bool_char; end record; + pragma Universal_Aliasing (VBC_View); -- -- short components -- @@ -85,6 +88,7 @@ package GNAT.Altivec.Vector_Views is type VUS_View is record Values : Varray_unsigned_short; end record; + pragma Universal_Aliasing (VUS_View); type Varray_signed_short is array (Vshort_Range) of signed_short; for Varray_signed_short'Alignment use VECTOR_ALIGNMENT; @@ -92,6 +96,7 @@ package GNAT.Altivec.Vector_Views is type VSS_View is record Values : Varray_signed_short; end record; + pragma Universal_Aliasing (VSS_View); type Varray_bool_short is array (Vshort_Range) of bool_short; for Varray_bool_short'Alignment use VECTOR_ALIGNMENT; @@ -99,6 +104,7 @@ package GNAT.Altivec.Vector_Views is type VBS_View is record Values : Varray_bool_short; end record; + pragma Universal_Aliasing (VBS_View); -- int components -- @@ -112,6 +118,7 @@ package GNAT.Altivec.Vector_Views is type VUI_View is record Values : Varray_unsigned_int; end record; + pragma Universal_Aliasing (VUI_View); type Varray_signed_int is array (Vint_Range) of signed_int; for Varray_signed_int'Alignment use VECTOR_ALIGNMENT; @@ -119,6 +126,7 @@ package GNAT.Altivec.Vector_Views is type VSI_View is record Values : Varray_signed_int; end record; + pragma Universal_Aliasing (VSI_View); type Varray_bool_int is array (Vint_Range) of bool_int; for Varray_bool_int'Alignment use VECTOR_ALIGNMENT; @@ -126,6 +134,7 @@ package GNAT.Altivec.Vector_Views is type VBI_View is record Values : Varray_bool_int; end record; + pragma Universal_Aliasing (VBI_View); -- -- float components -- @@ -139,6 +148,7 @@ package GNAT.Altivec.Vector_Views is type VF_View is record Values : Varray_float; end record; + pragma Universal_Aliasing (VF_View); -- -- pixel components -- @@ -152,5 +162,6 @@ package GNAT.Altivec.Vector_Views is type VP_View is record Values : Varray_pixel; end record; + pragma Universal_Aliasing (VP_View); end GNAT.Altivec.Vector_Views; diff --git a/gcc/ada/libgnat/s-stratt.ads b/gcc/ada/libgnat/s-stratt.ads index 1d4c82d17ab..eee19f4bdce 100644 --- a/gcc/ada/libgnat/s-stratt.ads +++ b/gcc/ada/libgnat/s-stratt.ads @@ -74,6 +74,9 @@ package System.Stream_Attributes is P2 : System.Address; end record; + pragma Universal_Aliasing (Fat_Pointer); + -- This avoids a copy for the aforementioned unchecked conversions + -- Treatment of enumeration types -- -- 2.43.2
[COMMITTED 19/35] ada: Couple of adjustments coming from aliasing considerations
From: Eric Botcazou The first adjustment is to the expansion of implementation types for array types with peculiar index types, for which the aliased property set on the component of the original type must be copied; the implicit base type also needs to be properly marked if the implementation type is constrained. The second adjustment is to selected types in the runtime, which need to be marked with pragma Universal_Aliasing because of their special usage. gcc/ada/ * exp_pakd.adb (Create_Packed_Array_Impl_Type): For non-bit-packed array types, propagate the aliased property of the component. (Install_PAT): Set fields on the implicit base type of an array. * libgnat/a-stream.ads (private part): Add pragma Universal_Aliasing for Stream_Element. * libgnat/g-alleve.ads: Add pragma Universal_Aliasing for all the vector types. * libgnat/g-alleve__hard.ads: Likewise. Tested on x86_64-pc-linux-gnu, committed on master. --- gcc/ada/exp_pakd.adb | 12 +-- gcc/ada/libgnat/a-stream.ads | 3 ++ gcc/ada/libgnat/g-alleve.ads | 54 ++ gcc/ada/libgnat/g-alleve__hard.ads | 11 ++ 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/gcc/ada/exp_pakd.adb b/gcc/ada/exp_pakd.adb index 3f26c3527fa..59dfe5df8df 100644 --- a/gcc/ada/exp_pakd.adb +++ b/gcc/ada/exp_pakd.adb @@ -598,6 +598,14 @@ package body Exp_Pakd is Set_Associated_Node_For_Itype (PAT, Typ); Set_Original_Array_Type (PAT, Typ); + -- In the case of a constrained array type, also set fields on the + -- implicit base type built during the analysis of its declaration. + + if Ekind (PAT) = E_Array_Subtype then +Set_Is_Packed_Array_Impl_Type (Etype (PAT), True); +Set_Original_Array_Type (Etype (PAT), Base_Type (Typ)); + end if; + -- Propagate representation aspects Set_Is_Atomic (PAT, Is_Atomic(Typ)); @@ -818,7 +826,7 @@ package body Exp_Pakd is Subtype_Marks => Indexes, Component_Definition => Make_Component_Definition (Loc, - Aliased_Present=> False, + Aliased_Present=> Has_Aliased_Components (Typ), Subtype_Indication => New_Occurrence_Of (Ctyp, Loc))); @@ -828,7 +836,7 @@ package body Exp_Pakd is Discrete_Subtype_Definitions => Indexes, Component_Definition => Make_Component_Definition (Loc, -Aliased_Present=> False, +Aliased_Present=> Has_Aliased_Components (Typ), Subtype_Indication => New_Occurrence_Of (Ctyp, Loc))); end if; diff --git a/gcc/ada/libgnat/a-stream.ads b/gcc/ada/libgnat/a-stream.ads index 0a0cabce3f2..dcb5a9aa81c 100644 --- a/gcc/ada/libgnat/a-stream.ads +++ b/gcc/ada/libgnat/a-stream.ads @@ -84,4 +84,7 @@ private for Stream_Element_Array'Read use Read_SEA; for Stream_Element_Array'Write use Write_SEA; + pragma Universal_Aliasing (Stream_Element); + -- This type is used to stream any other type + end Ada.Streams; diff --git a/gcc/ada/libgnat/g-alleve.ads b/gcc/ada/libgnat/g-alleve.ads index 0f3ec36d0f1..4e22a3e6387 100644 --- a/gcc/ada/libgnat/g-alleve.ads +++ b/gcc/ada/libgnat/g-alleve.ads @@ -313,22 +313,62 @@ private --- -- We simply use the natural array definitions corresponding to each - -- user-level vector type. + -- user-level vector type. We need to put pragma Universal_Aliasing + -- on these types because the common operations are implemented by + -- means of Unchecked_Conversion betwwen different representations. - type LL_VUI is new VUI_View; - type LL_VSI is new VSI_View; - type LL_VBI is new VBI_View; + -- + -- char Core Components -- + -- + + type LL_VUC is new VUC_View; + pragma Universal_Aliasing (LL_VUC); + + type LL_VSC is new VSC_View; + pragma Universal_Aliasing (LL_VSC); + + type LL_VBC is new VBC_View; + pragma Universal_Aliasing (LL_VBC); + + --- + -- short Core Components -- + --- type LL_VUS is new VUS_View; + pragma Universal_Aliasing (LL_VUS); + type LL_VSS is new VSS_View; + pragma Universal_Aliasing (LL_VSS); + type LL_VBS is new VBS_View; + pragma Universal_Aliasing (LL_VBS); - type LL_VUC is new VUC_View; - type LL_VSC is new VSC_View; - type LL_VBC is new VBC_View; + - + -- int Core Components -- + - + + type LL_VUI is new VUI_View; + pragma Universal_Aliasing (LL_VUI); + + type LL_VSI is new
[gcc r15-605] ada: Couple of adjustments coming from aliasing considerations
https://gcc.gnu.org/g:485d595d22c7800eb214034c9b58211ab232dbbf commit r15-605-g485d595d22c7800eb214034c9b58211ab232dbbf Author: Eric Botcazou Date: Sun Mar 10 15:51:21 2024 +0100 ada: Couple of adjustments coming from aliasing considerations The first adjustment is to the expansion of implementation types for array types with peculiar index types, for which the aliased property set on the component of the original type must be copied; the implicit base type also needs to be properly marked if the implementation type is constrained. The second adjustment is to selected types in the runtime, which need to be marked with pragma Universal_Aliasing because of their special usage. gcc/ada/ * exp_pakd.adb (Create_Packed_Array_Impl_Type): For non-bit-packed array types, propagate the aliased property of the component. (Install_PAT): Set fields on the implicit base type of an array. * libgnat/a-stream.ads (private part): Add pragma Universal_Aliasing for Stream_Element. * libgnat/g-alleve.ads: Add pragma Universal_Aliasing for all the vector types. * libgnat/g-alleve__hard.ads: Likewise. Diff: --- gcc/ada/exp_pakd.adb | 12 +++-- gcc/ada/libgnat/a-stream.ads | 3 +++ gcc/ada/libgnat/g-alleve.ads | 54 +- gcc/ada/libgnat/g-alleve__hard.ads | 11 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/gcc/ada/exp_pakd.adb b/gcc/ada/exp_pakd.adb index 3f26c3527fa4..59dfe5df8df4 100644 --- a/gcc/ada/exp_pakd.adb +++ b/gcc/ada/exp_pakd.adb @@ -598,6 +598,14 @@ package body Exp_Pakd is Set_Associated_Node_For_Itype (PAT, Typ); Set_Original_Array_Type (PAT, Typ); + -- In the case of a constrained array type, also set fields on the + -- implicit base type built during the analysis of its declaration. + + if Ekind (PAT) = E_Array_Subtype then +Set_Is_Packed_Array_Impl_Type (Etype (PAT), True); +Set_Original_Array_Type (Etype (PAT), Base_Type (Typ)); + end if; + -- Propagate representation aspects Set_Is_Atomic (PAT, Is_Atomic(Typ)); @@ -818,7 +826,7 @@ package body Exp_Pakd is Subtype_Marks => Indexes, Component_Definition => Make_Component_Definition (Loc, - Aliased_Present=> False, + Aliased_Present=> Has_Aliased_Components (Typ), Subtype_Indication => New_Occurrence_Of (Ctyp, Loc))); @@ -828,7 +836,7 @@ package body Exp_Pakd is Discrete_Subtype_Definitions => Indexes, Component_Definition => Make_Component_Definition (Loc, -Aliased_Present=> False, +Aliased_Present=> Has_Aliased_Components (Typ), Subtype_Indication => New_Occurrence_Of (Ctyp, Loc))); end if; diff --git a/gcc/ada/libgnat/a-stream.ads b/gcc/ada/libgnat/a-stream.ads index 0a0cabce3f2b..dcb5a9aa81ce 100644 --- a/gcc/ada/libgnat/a-stream.ads +++ b/gcc/ada/libgnat/a-stream.ads @@ -84,4 +84,7 @@ private for Stream_Element_Array'Read use Read_SEA; for Stream_Element_Array'Write use Write_SEA; + pragma Universal_Aliasing (Stream_Element); + -- This type is used to stream any other type + end Ada.Streams; diff --git a/gcc/ada/libgnat/g-alleve.ads b/gcc/ada/libgnat/g-alleve.ads index 0f3ec36d0f1b..4e22a3e63876 100644 --- a/gcc/ada/libgnat/g-alleve.ads +++ b/gcc/ada/libgnat/g-alleve.ads @@ -313,22 +313,62 @@ private --- -- We simply use the natural array definitions corresponding to each - -- user-level vector type. + -- user-level vector type. We need to put pragma Universal_Aliasing + -- on these types because the common operations are implemented by + -- means of Unchecked_Conversion betwwen different representations. - type LL_VUI is new VUI_View; - type LL_VSI is new VSI_View; - type LL_VBI is new VBI_View; + -- + -- char Core Components -- + -- + + type LL_VUC is new VUC_View; + pragma Universal_Aliasing (LL_VUC); + + type LL_VSC is new VSC_View; + pragma Universal_Aliasing (LL_VSC); + + type LL_VBC is new VBC_View; + pragma Universal_Aliasing (LL_VBC); + + --- + -- short Core Components -- + --- type LL_VUS is new VUS_View; + pragma Universal_Aliasing (LL_VUS); + type LL_VSS is new VSS_View; + pragma Universal_Aliasing (LL_VSS); + type LL_VBS is new VBS_View; + pragma Universal_Aliasi
[gcc r15-616] ada: Further adjustments coming from aliasing considerations
https://gcc.gnu.org/g:9ff83f013eb1ea2eac11c17cc3be2024e96101a5 commit r15-616-g9ff83f013eb1ea2eac11c17cc3be2024e96101a5 Author: Eric Botcazou Date: Thu Mar 14 12:58:29 2024 +0100 ada: Further adjustments coming from aliasing considerations They are needed on 32-bit platforms because of different calling conventions and again in the units implementing AltiVec and Streams support. gcc/ada/ * libgnat/g-alvevi.ads: Add pragma Universal_Aliasing for all the view types. * libgnat/s-stratt.ads: Likewise for Fat_Pointer type. Diff: --- gcc/ada/libgnat/g-alvevi.ads | 11 +++ gcc/ada/libgnat/s-stratt.ads | 3 +++ 2 files changed, 14 insertions(+) diff --git a/gcc/ada/libgnat/g-alvevi.ads b/gcc/ada/libgnat/g-alvevi.ads index b2beac7284c7..b0f58790adf8 100644 --- a/gcc/ada/libgnat/g-alvevi.ads +++ b/gcc/ada/libgnat/g-alvevi.ads @@ -58,6 +58,7 @@ package GNAT.Altivec.Vector_Views is type VUC_View is record Values : Varray_unsigned_char; end record; + pragma Universal_Aliasing (VUC_View); type Varray_signed_char is array (Vchar_Range) of signed_char; for Varray_signed_char'Alignment use VECTOR_ALIGNMENT; @@ -65,6 +66,7 @@ package GNAT.Altivec.Vector_Views is type VSC_View is record Values : Varray_signed_char; end record; + pragma Universal_Aliasing (VSC_View); type Varray_bool_char is array (Vchar_Range) of bool_char; for Varray_bool_char'Alignment use VECTOR_ALIGNMENT; @@ -72,6 +74,7 @@ package GNAT.Altivec.Vector_Views is type VBC_View is record Values : Varray_bool_char; end record; + pragma Universal_Aliasing (VBC_View); -- -- short components -- @@ -85,6 +88,7 @@ package GNAT.Altivec.Vector_Views is type VUS_View is record Values : Varray_unsigned_short; end record; + pragma Universal_Aliasing (VUS_View); type Varray_signed_short is array (Vshort_Range) of signed_short; for Varray_signed_short'Alignment use VECTOR_ALIGNMENT; @@ -92,6 +96,7 @@ package GNAT.Altivec.Vector_Views is type VSS_View is record Values : Varray_signed_short; end record; + pragma Universal_Aliasing (VSS_View); type Varray_bool_short is array (Vshort_Range) of bool_short; for Varray_bool_short'Alignment use VECTOR_ALIGNMENT; @@ -99,6 +104,7 @@ package GNAT.Altivec.Vector_Views is type VBS_View is record Values : Varray_bool_short; end record; + pragma Universal_Aliasing (VBS_View); -- int components -- @@ -112,6 +118,7 @@ package GNAT.Altivec.Vector_Views is type VUI_View is record Values : Varray_unsigned_int; end record; + pragma Universal_Aliasing (VUI_View); type Varray_signed_int is array (Vint_Range) of signed_int; for Varray_signed_int'Alignment use VECTOR_ALIGNMENT; @@ -119,6 +126,7 @@ package GNAT.Altivec.Vector_Views is type VSI_View is record Values : Varray_signed_int; end record; + pragma Universal_Aliasing (VSI_View); type Varray_bool_int is array (Vint_Range) of bool_int; for Varray_bool_int'Alignment use VECTOR_ALIGNMENT; @@ -126,6 +134,7 @@ package GNAT.Altivec.Vector_Views is type VBI_View is record Values : Varray_bool_int; end record; + pragma Universal_Aliasing (VBI_View); -- -- float components -- @@ -139,6 +148,7 @@ package GNAT.Altivec.Vector_Views is type VF_View is record Values : Varray_float; end record; + pragma Universal_Aliasing (VF_View); -- -- pixel components -- @@ -152,5 +162,6 @@ package GNAT.Altivec.Vector_Views is type VP_View is record Values : Varray_pixel; end record; + pragma Universal_Aliasing (VP_View); end GNAT.Altivec.Vector_Views; diff --git a/gcc/ada/libgnat/s-stratt.ads b/gcc/ada/libgnat/s-stratt.ads index 1d4c82d17abd..eee19f4bdced 100644 --- a/gcc/ada/libgnat/s-stratt.ads +++ b/gcc/ada/libgnat/s-stratt.ads @@ -74,6 +74,9 @@ package System.Stream_Attributes is P2 : System.Address; end record; + pragma Universal_Aliasing (Fat_Pointer); + -- This avoids a copy for the aforementioned unchecked conversions + -- Treatment of enumeration types --
[PATCH 3/4] Libatomic: Clean up AArch64 ifunc aliasing
Following improvements to the way ifuncs are selected based on detected architectural features, we are able to do away with many of the aliases that were previously needed for subsets of atomic functions that were not implemented in a given extension. This may be clarified by virtue of an example. Before, LSE128 functions carried the suffix _i1 and LSE2 functions the _i2. Using a single ifunc selector for all atomic functions meant that if LSE128 was detected, the _i1 function variant would be used indiscriminately, irrespective of whether or not a function had an LSE128-specific implementation. Aliasing was thus needed to redirect calls to these missing functions to their _i2 LSE2 alternatives. The more architectural extensions for which support was added, the more complex the aliasing chain. With the per-file configuration of ifuncs, we do away with the need for such aliasing. libatomic/ChangeLog: * config/linux/aarch64/atomic_16.S: Remove unnecessary aliasing. --- libatomic/config/linux/aarch64/atomic_16.S | 41 -- 1 file changed, 41 deletions(-) diff --git a/libatomic/config/linux/aarch64/atomic_16.S b/libatomic/config/linux/aarch64/atomic_16.S index 1517e9e78df..16ff03057ab 100644 --- a/libatomic/config/linux/aarch64/atomic_16.S +++ b/libatomic/config/linux/aarch64/atomic_16.S @@ -732,47 +732,6 @@ ENTRY_ALIASED (test_and_set_16) END (test_and_set_16) -/* Alias entry points which are the same in LSE2 and LSE128. */ - -#if HAVE_IFUNC -# if !HAVE_FEAT_LSE128 -ALIAS (exchange_16, LSE128, LSE2) -ALIAS (fetch_or_16, LSE128, LSE2) -ALIAS (fetch_and_16, LSE128, LSE2) -ALIAS (or_fetch_16, LSE128, LSE2) -ALIAS (and_fetch_16, LSE128, LSE2) -# endif -ALIAS (load_16, LSE128, LSE2) -ALIAS (store_16, LSE128, LSE2) -ALIAS (compare_exchange_16, LSE128, LSE2) -ALIAS (fetch_add_16, LSE128, LSE2) -ALIAS (add_fetch_16, LSE128, LSE2) -ALIAS (fetch_sub_16, LSE128, LSE2) -ALIAS (sub_fetch_16, LSE128, LSE2) -ALIAS (fetch_xor_16, LSE128, LSE2) -ALIAS (xor_fetch_16, LSE128, LSE2) -ALIAS (fetch_nand_16, LSE128, LSE2) -ALIAS (nand_fetch_16, LSE128, LSE2) -ALIAS (test_and_set_16, LSE128, LSE2) - -/* Alias entry points which are the same in baseline and LSE2. */ - -ALIAS (exchange_16, LSE2, CORE) -ALIAS (fetch_add_16, LSE2, CORE) -ALIAS (add_fetch_16, LSE2, CORE) -ALIAS (fetch_sub_16, LSE2, CORE) -ALIAS (sub_fetch_16, LSE2, CORE) -ALIAS (fetch_or_16, LSE2, CORE) -ALIAS (or_fetch_16, LSE2, CORE) -ALIAS (fetch_and_16, LSE2, CORE) -ALIAS (and_fetch_16, LSE2, CORE) -ALIAS (fetch_xor_16, LSE2, CORE) -ALIAS (xor_fetch_16, LSE2, CORE) -ALIAS (fetch_nand_16, LSE2, CORE) -ALIAS (nand_fetch_16, LSE2, CORE) -ALIAS (test_and_set_16, LSE2, CORE) -#endif - /* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code. */ #define FEATURE_1_AND 0xc000 #define FEATURE_1_BTI 1 -- 2.34.1
[PATCH 0/4] Libatomic: Cleanup ifunc selector and aliasing
The recent introduction of the optional LSE128 and RCPC3 architectural extensions to AArch64 has further led to the increased flexibility of atomic support in the architecture, with many extensions providing support for distinct atomic operations, each with different potential applications in mind. This has led to maintenance difficulties in Libatomic, in particular regarding the way the ifunc selector is generated via a series of macro expansions at compile-time. Until now, irrespective of the atomic operation in question, all atomic functions for a particular operand size were expected to have the same number of ifunc alternatives, meaning that a one-size-fits-all approach could reasonably be taken for the selector. This meant that if, hypothetically, for a particular architecture and operand size one particular atomic operation was to have 3 different implementations associated with different extensions, libatomic would likewise be required to present three ifunc alternatives for all other atomic functions. The consequence in the design choice was the unnecessary use of function aliasing and the unwieldy code which resulted from this. This patch series attempts to remediate this issue by making the preprocessor macros defining the number of ifunc alternatives and their respective selection functions dependent on the file importing the ifunc selector-generating framework. all files are given `LAT_' macros, defined at the beginning and undef'd at the end of the file. It is these macros that are subsequently used to fine-tune the behaviors of `libatomic_i.h' and `host-config.h'. In particular, the definition of the `IFUNC_NCOND(N)' and `IFUNC_COND_' macros in host-config.h can now be guarded behind these new file-specific macros, which ultimately control what the `GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to. As both of these headers are imported once per file implementing some atomic operation, fine-tuned control is now possible. Regtested with both `--enable-gnu-indirect-function' and `--disable-gnu-indirect-function' configurations on armv9.4-a target with LRCPC3 and LSE128 support and without. Victor Do Nascimento (4): Libatomic: Define per-file identifier macros Libatomic: Make ifunc selector behavior contingent on importing file Libatomic: Clean up AArch64 ifunc aliasing Libatomic: Clean up AArch64 `atomic_16.S' implementation file libatomic/cas_n.c| 2 + libatomic/config/linux/aarch64/atomic_16.S | 623 +-- libatomic/config/linux/aarch64/host-config.h | 35 +- libatomic/exch_n.c | 2 + libatomic/fadd_n.c | 2 + libatomic/fand_n.c | 2 + libatomic/fence.c| 2 + libatomic/fenv.c | 2 + libatomic/fior_n.c | 2 + libatomic/flag.c | 2 + libatomic/fnand_n.c | 2 + libatomic/fop_n.c| 2 + libatomic/fsub_n.c | 2 + libatomic/fxor_n.c | 2 + libatomic/gcas.c | 2 + libatomic/gexch.c| 2 + libatomic/glfree.c | 2 + libatomic/gload.c| 2 + libatomic/gstore.c | 2 + libatomic/load_n.c | 2 + libatomic/store_n.c | 2 + libatomic/tas_n.c| 2 + 22 files changed, 357 insertions(+), 341 deletions(-) -- 2.34.1
[Bug c/115096] -fstrict-aliasing miscompilation or missing warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096 --- Comment #2 from Andrew Pinski --- > the warning described in that bug is not occurring. That is because in that case it had literally `(unsigned char**)` while in this case it is spread across function calls and that would require a lot of work to get the warning happening. Note there are a few open bug reports specifically asking for a runtime catching of this too; but that is also hard as you need to record the aliasing set of when stores and loads happen.
[Bug c/115096] -fstrict-aliasing miscompilation or missing warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096 Andrew Pinski changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |INVALID --- Comment #1 from Andrew Pinski --- So this is not a miscompiling as the pointer types "char*" and "unsigned char*" are two distant pointer types and are not considered to be aliasing and you are not accessing the pointer via a character type (GCC has an exception for void* types though which you could do the store in and get the correct result). There is no warning here because the casting between pointer types is not the issue but rather the access. Since test is passed a pointer to a "char*" type and only in test you do the cast to a pointer to a "unsigned char*" type. a warning in this case would 100% require full path analysis of the code. Note you could rewrite the code to be: ``` static int helper(void **cursor) { // first character ('a') is OK if (*(unsigned char*)*cursor > 'z') { return -11; } *cursor = ((unsigned char*)*cursor) + 1; // second character ('b') is OK if (*(unsigned char*)*cursor > 'z') { return -12; } // increment to third character ('.')... *cursor = ((unsigned char*)*cursor) + 1; return 0; } ``` And it would work correctly as I mentioned GCC has an exception on the `void*` type which is allowed to alias all other pointer types. (Note I am not 100% sure if C standard says that is still undefined code or not but GCC makes it as such).
[Bug c/115096] New: -fstrict-aliasing miscompilation or missing warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096 Bug ID: 115096 Summary: -fstrict-aliasing miscompilation or missing warning Product: gcc Version: 12.3.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: Philip.Taff at itron dot com Target Milestone: --- Created attachment 58209 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58209=edit minimal test case I'm not sure whether this code is a violation of strict-aliasing, in which case a warning would be great, or it is valid, in which case it is being miscompiled, removing some of the increments. According to Bug #29901 a similar cast does break strict-aliasing rules, but the warning described in that bug is not occurring. Built with: $ gcc -O1 -fstrict-aliasing -Wall -Wextra test.c No compiler warnings or errors. Executed with: ("ab." is the test data) $ ./a.out ab. GCC/system information: $ gcc -v Using built-in specs. COLLECT_GCC=gcc-12 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/12/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 12.3.0-1ubuntu1~22.04' --with-bugurl=file:///usr/share/doc/gcc-12/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-12 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-12-ALHxjy/gcc-12-12.3.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-12-ALHxjy/gcc-12-12.3.0/debian/tmp-gcn/usr --enable-offload-defaulted --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 12.3.0 (Ubuntu 12.3.0-1ubuntu1~22.04) COLLECT_GCC_OPTIONS='-O1' '-fstrict-aliasing' '-Wall' '-Wextra' '-v' '-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'a-' /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -E -quiet -v -imultiarch x86_64-linux-gnu test.c -mtune=generic -march=x86-64 -Wall -Wextra -fstrict-aliasing -O1 -fpch-preprocess -fasynchronous-unwind-tables -fstack-protector-strong -Wformat-security -fstack-clash-protection -fcf-protection -o a-test.i ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu" ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/include-fixed" ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include" #include "..." search starts here: #include <...> search starts here: /usr/lib/gcc/x86_64-linux-gnu/12/include /usr/local/include /usr/include/x86_64-linux-gnu /usr/include End of search list. COLLECT_GCC_OPTIONS='-O1' '-fstrict-aliasing' '-Wall' '-Wextra' '-v' '-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'a-' /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -fpreprocessed a-test.i -quiet -dumpdir a- -dumpbase test.c -dumpbase-ext .c -mtune=generic -march=x86-64 -O1 -Wall -Wextra -version -fstrict-aliasing -fasynchronous-unwind-tables -fstack-protector-strong -Wformat-security -fstack-clash-protection -fcf-protection -o a-test.s GNU C17 (Ubuntu 12.3.0-1ubuntu1~22.04) version 12.3.0 (x86_64-linux-gnu) compiled by GNU C version 12.3.0, GMP version 6.2.1, MPFR version 4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 GNU C17 (Ubuntu 12.3.0-1ubuntu1~22.04) version 12.3.0 (x86_64-linux-gnu) compiled by GNU C version 12.3.0, GMP version 6.2.1, MPFR version 4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 Compiler executable checksum: d93534411cccf908c6f3926ba41cb359 COLLECT_GCC_OPTIONS='-O1' '-fstrict-aliasing' '-Wall' '-Wextra' '-v' '-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'a-' as -v --64 -o a-test.o a-test.s GNU assembler version 2.38 (x86_64-linux-gnu) using BFD version (GNU Binutils for Ubuntu) 2.38 COMPILER_PATH=/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-g
[Bug lto/112732] during IPA pass: *free_lang_data ICE: 'verify_type' failed: type variant with 'TYPE_ALIAS_SET_KNOWN_P' with -Os -Wstrict-aliasing=2 -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112732 --- Comment #5 from GCC Commits --- The releases/gcc-13 branch has been updated by Richard Biener : https://gcc.gnu.org/g:664ab9c6e8a5d031031596100997e025e5334e86 commit r13-8721-g664ab9c6e8a5d031031596100997e025e5334e86 Author: Richard Biener Date: Tue Nov 28 12:36:21 2023 +0100 middle-end/112732 - stray TYPE_ALIAS_SET in type variant The following fixes a stray TYPE_ALIAS_SET in a type variant built by build_opaque_vector_type which is diagnosed by type checking enabled with -flto. PR middle-end/112732 * tree.cc (build_opaque_vector_type): Reset TYPE_ALIAS_SET of the newly built type. (cherry picked from commit f26d68d5d128c86faaceeb81b1e8f22254ad53df)
[Bug tree-optimization/113630] [11/12 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 Richard Biener changed: What|Removed |Added CC||rguenth at gcc dot gnu.org Known to fail||13.2.0 Status|ASSIGNED|NEW Known to work||13.2.1 Summary|[11/12/13 Regression] |[11/12 Regression] |-fno-strict-aliasing|-fno-strict-aliasing |introduces out-of-bounds|introduces out-of-bounds |memory access |memory access Assignee|rguenth at gcc dot gnu.org |unassigned at gcc dot gnu.org --- Comment #9 from Richard Biener --- Backported as far as I want.
[Bug tree-optimization/113630] [11/12/13 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 --- Comment #8 from GCC Commits --- The releases/gcc-13 branch has been updated by Richard Biener : https://gcc.gnu.org/g:47cd06042237bf2d4f05b8355362bc038f6fa445 commit r13-8693-g47cd06042237bf2d4f05b8355362bc038f6fa445 Author: Richard Biener Date: Wed Jan 31 11:28:50 2024 +0100 tree-optimization/113630 - invalid code hoisting The following avoids code hoisting (but also PRE insertion) of expressions that got value-numbered to another one that are not a valid replacement (but still compute the same value). This time because the access path ends in a structure with different size, meaning we consider a related access as not trapping because of the size of the base of the access. PR tree-optimization/113630 * tree-ssa-pre.cc (compute_avail): Avoid registering a reference with a representation with not matching base access size. * gcc.dg/torture/pr113630.c: New testcase. (cherry picked from commit 724b64304ff5c8ac08a913509afd6fde38d7b767)
[PATCH] c++, v5: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Thu, Apr 25, 2024 at 11:30:48AM -0400, Jason Merrill wrote: > Hmm, maybe maybe_clone_body shouldn't clear DECL_SAVED_TREE for aliases, but > rather set it to some stub like void_node? I'll try that in stage1. > Though with all these changes, it's probably better to go with your first > patch for GCC 14 and delay this approach to 15. Your v1 patch is OK for 14. Just to record, following patch passed bootstrap/regtest on x86_64-linux and i686-linux. But I've committed the v1 version instead with the addition of comdat2.C and comdat5.C testcases from this patch now and in stage1 will post an incremental diff. Thanks. 2024-04-25 Jakub Jelinek Jason Merrill PR lto/113208 * decl2.cc (tentative_decl_linkage): Call maybe_make_one_only for implicit instantiations of maybe in charge ctors/dtors declared inline. (c_parse_final_cleanups): Don't skip used same body aliases which have non-NULL DECL_SAVED_TREE on the alias target. Formatting fixes. * optimize.cc (can_alias_cdtor): Adjust condition, for HAVE_COMDAT_GROUP && DECL_ONE_ONLY && DECL_WEAK return true even if not DECL_INTERFACE_KNOWN. * decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already set. * g++.dg/abi/comdat2.C: New test. * g++.dg/abi/comdat3.C: New test. * g++.dg/abi/comdat4.C: New test. * g++.dg/abi/comdat5.C: New test. * g++.dg/lto/pr113208_0.C: New test. * g++.dg/lto/pr113208_1.C: New file. * g++.dg/lto/pr113208.h: New file. --- gcc/cp/decl2.cc.jj 2024-04-24 18:28:22.299513620 +0200 +++ gcc/cp/decl2.cc 2024-04-25 16:19:17.385547357 +0200 @@ -3312,16 +3312,23 @@ tentative_decl_linkage (tree decl) linkage of all functions, and as that causes writes to the data mapped in from the PCH file, it's advantageous to mark the functions at this point. */ - if (DECL_DECLARED_INLINE_P (decl) - && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + if (DECL_DECLARED_INLINE_P (decl)) { - /* This function must have external linkage, as -otherwise DECL_INTERFACE_KNOWN would have been -set. */ - gcc_assert (TREE_PUBLIC (decl)); - comdat_linkage (decl); - DECL_INTERFACE_KNOWN (decl) = 1; + if (!DECL_IMPLICIT_INSTANTIATION (decl) + || DECL_DEFAULTED_FN (decl)) + { + /* This function must have external linkage, as +otherwise DECL_INTERFACE_KNOWN would have been +set. */ + gcc_assert (TREE_PUBLIC (decl)); + comdat_linkage (decl); + DECL_INTERFACE_KNOWN (decl) = 1; + } + else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)) + /* For implicit instantiations of cdtors try to make + it comdat, so that maybe_clone_body can use aliases. + See PR113208. */ + maybe_make_one_only (decl); } } else if (VAR_P (decl)) @@ -5264,7 +5271,19 @@ c_parse_final_cleanups (void) generate_tls_wrapper (decl); if (!DECL_SAVED_TREE (decl)) - continue; + { + cgraph_node *node; + tree tgt; + /* Even when maybe_clone_body created same body alias +has no DECL_SAVED_TREE, if its alias target does, +don't skip it. */ + if (!DECL_CLONED_FUNCTION (decl) + || !(node = cgraph_node::get (decl)) + || !node->cpp_implicit_alias + || !(tgt = node->get_alias_target_tree ()) + || !DECL_SAVED_TREE (tgt)) + continue; + } cgraph_node *node = cgraph_node::get_create (decl); @@ -5292,7 +5311,7 @@ c_parse_final_cleanups (void) node = node->get_alias_target (); node->call_for_symbol_thunks_and_aliases (clear_decl_external, - NULL, true); + NULL, true); /* If we mark !DECL_EXTERNAL one of the symbols in some comdat group, we need to mark all symbols in the same comdat group that way. */ @@ -5302,7 +5321,7 @@ c_parse_final_cleanups (void) next != node; next = dyn_cast (next->same_comdat_group)) next->call_for_symbol_thunks_and_aliases (clear_decl_external, - NULL, true); + NULL, true); } /* If we're going to need to write this
[gcc r14-10132] c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
https://gcc.gnu.org/g:c39654e7a431992773b48d61f804494b0d70855f commit r14-10132-gc39654e7a431992773b48d61f804494b0d70855f Author: Jakub Jelinek Date: Thu Apr 25 20:37:10 2024 +0200 c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208] When expand_or_defer_fn is called at_eof time, it calls import_export_decl and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a couple of places to try to optimize cdtors which are known to have the same body by making the complete cdtor an alias to base cdtor (and in that case also uses *[CD]5* as comdat group name instead of the normal comdat group names specific to each mangled name). Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN, maybe_clone_body and can_alias_cdtor use: if (DECL_ONE_ONLY (fn)) cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone)); ... bool can_alias = can_alias_cdtor (fn); ... /* Tell cgraph if both ctors or both dtors are known to have the same body. */ if (can_alias && fns[0] && idx == 1 && cgraph_node::get_create (fns[0])->create_same_body_alias (clone, fns[0])) { alias = true; if (DECL_ONE_ONLY (fns[0])) { /* For comdat base and complete cdtors put them into the same, *[CD]5* comdat group instead of *[CD][12]*. */ comdat_group = cdtor_comdat_group (fns[1], fns[0]); cgraph_node::get_create (fns[0])->set_comdat_group (comdat_group); if (symtab_node::get (clone)->same_comdat_group) symtab_node::get (clone)->remove_from_same_comdat_group (); symtab_node::get (clone)->add_to_same_comdat_group (symtab_node::get (fns[0])); } } and /* Don't use aliases for weak/linkonce definitions unless we can put both symbols in the same COMDAT group. */ return (DECL_INTERFACE_KNOWN (fn) && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) && (!DECL_ONE_ONLY (fn) || (HAVE_COMDAT_GROUP && DECL_WEAK (fn; The following testcase regressed with Marek's r14-5979 change, when pr113208_0.C is compiled where the ctor is marked constexpr, we no longer perform this optimization, where _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it, instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in _ZN6vectorI12QualityValueEC1ERKS1_ comdat group. Now, the linker seems to somehow cope with that, eventhough it probably keeps both copies of the ctor, but seems LTO can't cope with that and Honza doesn't know what it should do in that case (linker decides that the prevailing symbol is _ZN6vectorI12QualityValueEC2ERKS1_ (from the _ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and _ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU, from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)). Note, the case where some constructor is marked constexpr in one TU and not in another one happens pretty often in libstdc++ when one mixes -std= flags used to compile different compilation units. The reason the optimization doesn't trigger when the constructor is constexpr is that expand_or_defer_fn is called in that case much earlier than when it is not constexpr; in the former case it is called when we try to constant evaluate that constructor. But DECL_INTERFACE_KNOWN is false in that case and comdat_linkage hasn't been called either (I think it is desirable, because comdat group is stored in the cgraph node and am not sure it is a good idea to create cgraph nodes for something that might not be needed later on at all), so maybe_clone_body clones the bodies, but doesn't make them as aliases. The following patch is an attempt to redo this optimization when import_export_decl is called at_eof time on the base/complete cdtor (or deleting dtor). It will not do anything if maybe_clone_body hasn't been called uyet (the TREE_ASM_WRITTEN check on the DECL_MAYBE_IN_CHARGE_CDTOR_P), or when one or both of the base/complete cdtors have been lowered already, or when maybe_clone_body called maybe_thunk_body and it was successful. Otherwise retries the can_alias_cdtor check and makes the complete cdtor alias to the base cdtor with a
Re: [PATCH] c++, v4: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On 4/25/24 07:22, Jakub Jelinek wrote: On Thu, Apr 25, 2024 at 02:02:32PM +0200, Jakub Jelinek wrote: I've tried the following patch, but unfortunately that lead to large number of regressions: +FAIL: g++.dg/cpp0x/initlist25.C -std=c++17 (test for excess errors) So the reduced testcase for this is template struct A { T a1; U a2; template constexpr A(V &, W &) : a1(x), a2(y) {} }; template struct B; namespace std { template struct initializer_list { int *_M_array; decltype (sizeof 0) _M_len; }; } template struct C { void foo (std::initializer_list>); }; template struct D; template , typename = B> struct E { E (const char *); ~E (); }; int main () { C, E> m; m.foo ({{"t", "t"}, {"y", "y"}}); } Without the patch I've just posted or even with the earlier version of the patch the _ZN1AIK1EIc1DIcE1BIcEES5_EC[12]IRA2_KcSB_Lb1EEEOT_OT0_ ctors were emitted, but with this patch they are unresolved externals. The reason is that the code actually uses (calls) the _ZN1AIK1EIc1DIcE1BIcEES5_EC1IRA2_KcSB_Lb1EEEOT_OT0_ __ct_comp constructor, that one has TREE_USED, while the _ZN1AIK1EIc1DIcE1BIcEES5_EC2IRA2_KcSB_Lb1EEEOT_OT0_ __ct_base constructor is not TREE_USED. But the c_parse_final_cleanups loop over FOR_EACH_VEC_SAFE_ELT (deferred_fns, i, decl) will ignore the TREE_USED __ct_comp because it is an alias and so has !DECL_SAVED_TREE: 5273 if (!DECL_SAVED_TREE (decl)) 5274continue; Hmm, maybe maybe_clone_body shouldn't clear DECL_SAVED_TREE for aliases, but rather set it to some stub like void_node? Though with all these changes, it's probably better to go with your first patch for GCC 14 and delay this approach to 15. Your v1 patch is OK for 14. Jason
Re: [PATCH] c++, v4: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Thu, Apr 25, 2024 at 02:02:32PM +0200, Jakub Jelinek wrote: > I've tried the following patch, but unfortunately that lead to large > number of regressions: > +FAIL: g++.dg/cpp0x/initlist25.C -std=c++17 (test for excess errors) So the reduced testcase for this is template struct A { T a1; U a2; template constexpr A(V &, W &) : a1(x), a2(y) {} }; template struct B; namespace std { template struct initializer_list { int *_M_array; decltype (sizeof 0) _M_len; }; } template struct C { void foo (std::initializer_list>); }; template struct D; template , typename = B> struct E { E (const char *); ~E (); }; int main () { C, E> m; m.foo ({{"t", "t"}, {"y", "y"}}); } Without the patch I've just posted or even with the earlier version of the patch the _ZN1AIK1EIc1DIcE1BIcEES5_EC[12]IRA2_KcSB_Lb1EEEOT_OT0_ ctors were emitted, but with this patch they are unresolved externals. The reason is that the code actually uses (calls) the _ZN1AIK1EIc1DIcE1BIcEES5_EC1IRA2_KcSB_Lb1EEEOT_OT0_ __ct_comp constructor, that one has TREE_USED, while the _ZN1AIK1EIc1DIcE1BIcEES5_EC2IRA2_KcSB_Lb1EEEOT_OT0_ __ct_base constructor is not TREE_USED. But the c_parse_final_cleanups loop over FOR_EACH_VEC_SAFE_ELT (deferred_fns, i, decl) will ignore the TREE_USED __ct_comp because it is an alias and so has !DECL_SAVED_TREE: 5273 if (!DECL_SAVED_TREE (decl)) 5274continue; With the following incremental patch the tests in make check-g++ (haven't tried the coroutine one) which failed with the earlier patch now pass. --- gcc/cp/decl2.cc.jj 2024-04-25 10:52:21.057535959 +0200 +++ gcc/cp/decl2.cc 2024-04-25 16:19:17.385547357 +0200 @@ -5271,7 +5271,19 @@ c_parse_final_cleanups (void) generate_tls_wrapper (decl); if (!DECL_SAVED_TREE (decl)) - continue; + { + cgraph_node *node; + tree tgt; + /* Even when maybe_clone_body created same body alias +has no DECL_SAVED_TREE, if its alias target does, +don't skip it. */ + if (!DECL_CLONED_FUNCTION (decl) + || !(node = cgraph_node::get (decl)) + || !node->cpp_implicit_alias + || !(tgt = node->get_alias_target_tree ()) + || !DECL_SAVED_TREE (tgt)) + continue; + } cgraph_node *node = cgraph_node::get_create (decl); @@ -5299,7 +5311,7 @@ c_parse_final_cleanups (void) node = node->get_alias_target (); node->call_for_symbol_thunks_and_aliases (clear_decl_external, - NULL, true); + NULL, true); /* If we mark !DECL_EXTERNAL one of the symbols in some comdat group, we need to mark all symbols in the same comdat group that way. */ @@ -5309,7 +5321,7 @@ c_parse_final_cleanups (void) next != node; next = dyn_cast (next->same_comdat_group)) next->call_for_symbol_thunks_and_aliases (clear_decl_external, - NULL, true); + NULL, true); } /* If we're going to need to write this function out, and Jakub
[PATCH] c++, v4: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 24, 2024 at 08:43:46PM -0400, Jason Merrill wrote: > > Then can_alias_cdtor would return false, because it ends with: > >/* Don't use aliases for weak/linkonce definitions unless we can put both > > symbols in the same COMDAT group. */ > >return (DECL_INTERFACE_KNOWN (fn) > >&& (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) > >&& (!DECL_ONE_ONLY (fn) > >|| (HAVE_COMDAT_GROUP && DECL_WEAK (fn; > > Should we change that DECL_INTERFACE_KNOWN (fn) in there to > > (DECL_INTERFACE_KNOWN (fn) || something) then and what that > > something should be? HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)? > > Yes, I think reorganize to > > ((DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn) && !DECL_ONE_ONLY (fn)) > || (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)) I've tried the following patch, but unfortunately that lead to large number of regressions: +FAIL: g++.dg/coroutines/torture/co-yield-04-complex-local-state.C (test for excess errors) +FAIL: g++.dg/coroutines/torture/func-params-08.C (test for excess errors) +FAIL: g++.dg/coroutines/torture/func-params-09-awaitable-parms.C (test for excess errors) +FAIL: g++.dg/cpp0x/constexpr-initlist.C -std=c++11 (test for excess errors) +FAIL: g++.dg/cpp0x/constexpr-initlist.C -std=c++14 (test for excess errors) +FAIL: g++.dg/cpp0x/constexpr-initlist.C -std=c++17 (test for excess errors) +FAIL: g++.dg/cpp0x/constexpr-initlist.C -std=c++20 (test for excess errors) +FAIL: g++.dg/cpp0x/constexpr-initlist.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp0x/constexpr-initlist.C -std=c++26 (test for excess errors) +FAIL: g++.dg/cpp0x/initlist25.C -std=c++11 (test for excess errors) +FAIL: g++.dg/cpp0x/initlist25.C -std=c++14 (test for excess errors) +FAIL: g++.dg/cpp0x/initlist25.C -std=c++17 (test for excess errors) +FAIL: g++.dg/cpp0x/initlist25.C -std=c++20 (test for excess errors) +FAIL: g++.dg/cpp0x/initlist25.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp0x/initlist25.C -std=c++26 (test for excess errors) +FAIL: g++.dg/cpp1y/pr95226.C -std=c++20 (test for excess errors) +FAIL: g++.dg/cpp1y/pr95226.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp1y/pr95226.C -std=c++26 (test for excess errors) +FAIL: g++.dg/cpp1z/decomp12.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp1z/decomp12.C -std=c++26 (test for excess errors) +FAIL: g++.dg/cpp1z/eval-order2.C -std=c++20 (test for excess errors) +FAIL: g++.dg/cpp1z/eval-order2.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp1z/eval-order2.C -std=c++26 (test for excess errors) +FAIL: g++.dg/cpp2a/srcloc17.C -std=c++20 (test for excess errors) +FAIL: g++.dg/cpp2a/srcloc17.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp2a/srcloc17.C -std=c++26 (test for excess errors) +FAIL: g++.old-deja/g++.jason/template31.C -std=c++20 (test for excess errors) +FAIL: g++.old-deja/g++.jason/template31.C -std=c++23 (test for excess errors) +FAIL: g++.old-deja/g++.jason/template31.C -std=c++26 (test for excess errors) +FAIL: 20_util/unique_ptr/creation/for_overwrite.cc -std=gnu++26 (test for excess errors) +FAIL: 23_containers/span/cons_1_assert_neg.cc -std=gnu++20 (test for excess errors) +FAIL: 23_containers/span/cons_1_assert_neg.cc -std=gnu++26 (test for excess errors) +FAIL: 23_containers/span/cons_2_assert_neg.cc -std=gnu++20 (test for excess errors) +FAIL: 23_containers/span/cons_2_assert_neg.cc -std=gnu++26 (test for excess errors) +FAIL: std/ranges/repeat/1.cc -std=gnu++23 (test for excess errors) +FAIL: std/ranges/repeat/1.cc -std=gnu++26 (test for excess errors) Errors are like: func-params-08.C:(.text._ZNSt12_Vector_baseIiSaIiEEC2Ev[_ZNSt12_Vector_baseIiSaIiEEC5Ev]+0x14): undefined reference to `_ZNSt12_Vector_baseIiSaIiEE12_Vector_implC1EvQ26is_default_constructible_vIN9__gnu_cxx14__alloc_traitsIT0_NS5_10value_typeEE6rebindIT_E5otherEE' Though, libstdc++.so.6 abilist is the same. Trying to debug it now. 2024-04-24 Jakub Jelinek Jason Merrill PR lto/113208 * decl2.cc (tentative_decl_linkage): Call maybe_make_one_only for implicit instantiations of maybe in charge ctors/dtors declared inline. * optimize.cc (can_alias_cdtor): Adjust condition, for HAVE_COMDAT_GROUP && DECL_ONE_ONLY && DECL_WEAK return true even if not DECL_INTERFACE_KNOWN. * decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already set. * g++.dg/abi/comdat2.C: New test. * g++.dg/abi/comdat3.C: New test. * g++.dg/abi/comdat4.C: New test. * g++.dg/abi/comdat5.C: New test. * g++.dg/lto/pr113208_0.C: New test. * g++.dg/lto/pr113208_1.C: New file. * g++.dg/lto/pr113208.h: New file. --- gcc/cp/decl2.cc.jj 2024-04-24 18:28:22.299513620 +0200 +++ gcc/cp/decl2.cc 2024-04-25 10:04:18.049476567 +0200 @@ -3312,16 +3312,23 @@
Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On 4/24/24 15:47, Jakub Jelinek wrote: On Wed, Apr 24, 2024 at 06:39:33PM -0400, Jason Merrill wrote: --- gcc/cp/decl2.cc.jj 2024-04-23 14:49:41.933186265 +0200 +++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200 @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) to mark the functions at this point. */ if (DECL_DECLARED_INLINE_P (decl) && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + || DECL_DEFAULTED_FN (decl) + /* For implicit instantiations of cdtors, +if import_export_decl would use comdat linkage, +make sure to use it right away, so that maybe_clone_body +can use aliases. See PR113208. */ + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) + && (flag_implicit_templates + || flag_implicit_inline_templates) + && flag_weak + && TARGET_SUPPORTS_ALIASES))) It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an explicit instantiation later, and likewise for comdat_linkage when !flag_weak; instead of adding this condition to the if, how about adding an else like else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)) /* For implicit instantiations of cdtors, if import_export_decl would use comdat linkage, make sure to use it right away, so that maybe_clone_body can use aliases. See PR113208. */ maybe_make_one_only (decl); ? Then can_alias_cdtor would return false, because it ends with: /* Don't use aliases for weak/linkonce definitions unless we can put both symbols in the same COMDAT group. */ return (DECL_INTERFACE_KNOWN (fn) && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) && (!DECL_ONE_ONLY (fn) || (HAVE_COMDAT_GROUP && DECL_WEAK (fn; Should we change that DECL_INTERFACE_KNOWN (fn) in there to (DECL_INTERFACE_KNOWN (fn) || something) then and what that something should be? HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)? Yes, I think reorganize to ((DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn) && !DECL_ONE_ONLY (fn)) || (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)) Jason
Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 24, 2024 at 06:39:33PM -0400, Jason Merrill wrote: > > --- gcc/cp/decl2.cc.jj 2024-04-23 14:49:41.933186265 +0200 > > +++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200 > > @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) > > to mark the functions at this point. */ > > if (DECL_DECLARED_INLINE_P (decl) > > && (!DECL_IMPLICIT_INSTANTIATION (decl) > > - || DECL_DEFAULTED_FN (decl))) > > + || DECL_DEFAULTED_FN (decl) > > + /* For implicit instantiations of cdtors, > > +if import_export_decl would use comdat linkage, > > +make sure to use it right away, so that maybe_clone_body > > +can use aliases. See PR113208. */ > > + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) > > + && (flag_implicit_templates > > + || flag_implicit_inline_templates) > > + && flag_weak > > + && TARGET_SUPPORTS_ALIASES))) > > It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an > explicit instantiation later, and likewise for comdat_linkage when > !flag_weak; instead of adding this condition to the if, how about adding an > else like > > > else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)) > > /* For implicit instantiations of cdtors, > > if import_export_decl would use comdat linkage, > > make sure to use it right away, so that maybe_clone_body > > can use aliases. See PR113208. */ > > maybe_make_one_only (decl); > > ? Then can_alias_cdtor would return false, because it ends with: /* Don't use aliases for weak/linkonce definitions unless we can put both symbols in the same COMDAT group. */ return (DECL_INTERFACE_KNOWN (fn) && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) && (!DECL_ONE_ONLY (fn) || (HAVE_COMDAT_GROUP && DECL_WEAK (fn; Should we change that DECL_INTERFACE_KNOWN (fn) in there to (DECL_INTERFACE_KNOWN (fn) || something) then and what that something should be? HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)? Jakub
Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On 4/24/24 09:16, Jakub Jelinek wrote: On Wed, Apr 24, 2024 at 10:16:05AM +0100, Jonathan Wakely wrote: That fixes the testcases too, but seems to regress +FAIL: libstdc++-abi/abi_check There are explicit instantiation definitions that should instantiate those types: src/c++17/fs_dir.cc:template class std::__shared_ptr; src/c++17/fs_dir.cc:template class std::__shared_ptr; src/c++17/fs_path.cc:template class std::__shared_ptr; So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o So this boils down to (cvise reduced): namespace __gnu_cxx { enum _Lock_policy { _S_single, _S_mutex, _S_atomic } const __default_lock_policy = _S_atomic; } namespace std { using __gnu_cxx::__default_lock_policy; using __gnu_cxx::_Lock_policy; template struct __shared_ptr { constexpr __shared_ptr() {} }; namespace filesystem { struct _Dir; struct directory_iterator { __shared_ptr<_Dir> _M_dir; }; void end() { directory_iterator(); } } extern template class __shared_ptr; } namespace fs = std::filesystem; template class std::__shared_ptr; at -O2, previously it used to emit _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE1EEC2Ev but now no longer does with the yesterday posted PR113208 patch. The following updated patch fixes that by calling note_vague_linkage_fn for the cdtor clones from maybe_clone_body if the flags suggest that the maybe-in-charge cdtor had tentative_decl_linkage -> note_vague_linkage_fn called too. And then I've noticed that in some cases the updated comdat group set by maybe_clone_body (*C5* or *D5*) was then overwritten again by maybe_make_one_only. So the patch tweaks cxx_comdat_group, so that when some comdat group has been chosen already it doesn't try to use some different one. Bootstrapped/regtested on x86_64-linux and i686-linux, this one doesn't regress anything unlike the earlier patch. 2024-04-24 Jakub Jelinek PR lto/113208 * decl2.cc (tentative_decl_linkage): Use comdat_linkage also for implicit instantiations of maybe in charge ctors/dtors if -fimplicit-templates or -fimplicit-inline-templates and -fweak and target supports aliases. * optimize.cc (maybe_clone_body): Call note_vague_linkage_fn on clone if fn has DECL_INTERFACE_KNOWN, DECL_NOT_REALLY_EXTERN and DECL_DEFER_OUTPUT flags set. * decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already set. * g++.dg/abi/comdat2.C: New test. * g++.dg/abi/comdat3.C: New test. * g++.dg/lto/pr113208_0.C: New test. * g++.dg/lto/pr113208_1.C: New file. * g++.dg/lto/pr113208.h: New file. --- gcc/cp/decl2.cc.jj 2024-04-23 14:49:41.933186265 +0200 +++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200 @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) to mark the functions at this point. */ if (DECL_DECLARED_INLINE_P (decl) && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + || DECL_DEFAULTED_FN (decl) + /* For implicit instantiations of cdtors, +if import_export_decl would use comdat linkage, +make sure to use it right away, so that maybe_clone_body +can use aliases. See PR113208. */ + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) + && (flag_implicit_templates + || flag_implicit_inline_templates) + && flag_weak + && TARGET_SUPPORTS_ALIASES))) It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an explicit instantiation later, and likewise for comdat_linkage when !flag_weak; instead of adding this condition to the if, how about adding an else like else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)) /* For implicit instantiations of cdtors, if import_export_decl would use comdat linkage, make sure to use it right away, so that maybe_clone_body can use aliases. See PR113208. */ maybe_make_one_only (decl); ? Jason
[PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 24, 2024 at 10:16:05AM +0100, Jonathan Wakely wrote: > > That fixes the testcases too, but seems to regress > > +FAIL: libstdc++-abi/abi_check > There are explicit instantiation definitions that should instantiate > those types: > > src/c++17/fs_dir.cc:template class std::__shared_ptr; > src/c++17/fs_dir.cc:template class > std::__shared_ptr; > src/c++17/fs_path.cc:template class std::__shared_ptr fs::filesystem_error::_Impl>; > > So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o So this boils down to (cvise reduced): namespace __gnu_cxx { enum _Lock_policy { _S_single, _S_mutex, _S_atomic } const __default_lock_policy = _S_atomic; } namespace std { using __gnu_cxx::__default_lock_policy; using __gnu_cxx::_Lock_policy; template struct __shared_ptr { constexpr __shared_ptr() {} }; namespace filesystem { struct _Dir; struct directory_iterator { __shared_ptr<_Dir> _M_dir; }; void end() { directory_iterator(); } } extern template class __shared_ptr; } namespace fs = std::filesystem; template class std::__shared_ptr; at -O2, previously it used to emit _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE1EEC2Ev but now no longer does with the yesterday posted PR113208 patch. The following updated patch fixes that by calling note_vague_linkage_fn for the cdtor clones from maybe_clone_body if the flags suggest that the maybe-in-charge cdtor had tentative_decl_linkage -> note_vague_linkage_fn called too. And then I've noticed that in some cases the updated comdat group set by maybe_clone_body (*C5* or *D5*) was then overwritten again by maybe_make_one_only. So the patch tweaks cxx_comdat_group, so that when some comdat group has been chosen already it doesn't try to use some different one. Bootstrapped/regtested on x86_64-linux and i686-linux, this one doesn't regress anything unlike the earlier patch. 2024-04-24 Jakub Jelinek PR lto/113208 * decl2.cc (tentative_decl_linkage): Use comdat_linkage also for implicit instantiations of maybe in charge ctors/dtors if -fimplicit-templates or -fimplicit-inline-templates and -fweak and target supports aliases. * optimize.cc (maybe_clone_body): Call note_vague_linkage_fn on clone if fn has DECL_INTERFACE_KNOWN, DECL_NOT_REALLY_EXTERN and DECL_DEFER_OUTPUT flags set. * decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already set. * g++.dg/abi/comdat2.C: New test. * g++.dg/abi/comdat3.C: New test. * g++.dg/lto/pr113208_0.C: New test. * g++.dg/lto/pr113208_1.C: New file. * g++.dg/lto/pr113208.h: New file. --- gcc/cp/decl2.cc.jj 2024-04-23 14:49:41.933186265 +0200 +++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200 @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) to mark the functions at this point. */ if (DECL_DECLARED_INLINE_P (decl) && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + || DECL_DEFAULTED_FN (decl) + /* For implicit instantiations of cdtors, +if import_export_decl would use comdat linkage, +make sure to use it right away, so that maybe_clone_body +can use aliases. See PR113208. */ + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) + && (flag_implicit_templates + || flag_implicit_inline_templates) + && flag_weak + && TARGET_SUPPORTS_ALIASES))) { /* This function must have external linkage, as otherwise DECL_INTERFACE_KNOWN would have been --- gcc/cp/optimize.cc.jj 2024-04-24 13:44:26.456634100 +0200 +++ gcc/cp/optimize.cc 2024-04-24 14:46:13.050371557 +0200 @@ -717,6 +717,10 @@ maybe_clone_body (tree fn) else expand_or_defer_fn (clone); first = false; + if (DECL_INTERFACE_KNOWN (fn) + && DECL_NOT_REALLY_EXTERN (fn) + && DECL_DEFER_OUTPUT (fn)) + note_vague_linkage_fn (clone); } pop_from_top_level (); --- gcc/cp/decl.cc.jj 2024-04-23 08:31:05.515161033 +0200 +++ gcc/cp/decl.cc 2024-04-24 15:15:34.401951491 +0200 @@ -19254,6 +19254,14 @@ cxx_comdat_group (tree decl) else break; } + /* If a ctor/dtor has already set the comdat group by +maybe_clone_body, don't override it. */ + if (SUPPORTS_ONE_ONLY + && TREE_CODE (decl) == FUNCTION_DECL + && DECL_CLONED_FUNCTION_P (decl) + && SUPPORTS_ONE_ONLY) + if (tree comdat = DECL_COMDAT_GROUP (decl)) + return comdat; } return decl; --- gcc/testsuite/g++.dg/abi/comdat2.C.jj 2024-04-24 13:44:11.054849035
Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Tue, 23 Apr 2024 at 17:05, Jakub Jelinek wrote: > > On Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote: > > > > The following testcase regressed with Marek's r14-5979 change, > > > > when pr113208_0.C is compiled where the ctor is marked constexpr, > > > > we no longer perform this optimization, where > > > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the > > > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and > > > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it, > > > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in > > > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same > > > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in > > > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group. > > > > This seems like an ABI bug that could use a non-LTO testcase. > > Well, except for the issues it causes to LTO I think it is compatible, > worst case we get the body of the ctor duplicated in the executable > and the linker picks some of the weak symbols as the symbol definitions. > Anyway, I've added a non-LTO testcase for that in the patch below. > > > Hmm, cloning the bodies and then discarding them later seems like more extra > > work than creating the cgraph nodes. > > So, I've tried to handle that in tentative_decl_linkage, like that function > already handles functions declared inline except for implicit template > instantiations. If we expect that import_export_decl will do comdat_linkage > for the ctor later on do it right away. > > That fixes the testcases too, but seems to regress > +FAIL: libstdc++-abi/abi_check > on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from > libstdc++.so.6: > _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev > _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev > _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev > _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev > _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev > _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev > _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev > _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev > > Will need to study why that happened, it might be that it was ok because > I think the filesystem stuff is unlike the rest compiled with no exported > templates, but would need at least some hacks in libstdc++ to preserve > previously exported symbols. There are explicit instantiation definitions that should instantiate those types: src/c++17/fs_dir.cc:template class std::__shared_ptr; src/c++17/fs_dir.cc:template class std::__shared_ptr; src/c++17/fs_path.cc:template class std::__shared_ptr; So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o > Still, feels like a risky change this late if it wouldn't break ABI of other > libraries. > > 2024-04-23 Jakub Jelinek > > PR lto/113208 > * decl2.cc (tentative_decl_linkage): Use comdat_linkage also > for implicit instantiations of maybe in charge ctors/dtors > if -fimplicit-templates or -fimplicit-inline-templates and > -fweak and target supports aliases. > > * g++.dg/abi/comdat2.C: New test. > * g++.dg/lto/pr113208_0.C: New test. > * g++.dg/lto/pr113208_1.C: New file. > * g++.dg/lto/pr113208.h: New file. > > --- gcc/cp/decl2.cc.jj 2024-04-22 15:16:55.328548807 +0200 > +++ gcc/cp/decl2.cc 2024-04-23 09:52:18.993250442 +0200 > @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) > to mark the functions at this point. */ > if (DECL_DECLARED_INLINE_P (decl) > && (!DECL_IMPLICIT_INSTANTIATION (decl) > - || DECL_DEFAULTED_FN (decl))) > + || DECL_DEFAULTED_FN (decl) > + /* For implicit instantiations of cdtors, > +if import_export_decl would use comdat linkage, > +make sure to use it right away, so that maybe_clone_body > +can use aliases. See PR113208. */ > + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) > + && (flag_implicit_templates > + || flag_implicit_inline_templates) > + && flag_weak > + && TARGET_SUPPORTS_ALIASES))) > { > /* This function must have external linkage, as > otherwise DECL_INTERFACE_KNOWN would have been > --- gcc/testsuite/g++.dg/abi/comdat2.C.jj 2024-04-23 10:04:28.485964610 > +0200 > +++ gcc/testsuite/g++.dg/abi/comdat2.C 2024-04-23 10:05:24.757171194 +0200 > @@ -0,0 +1,26
Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote: > > > The following testcase regressed with Marek's r14-5979 change, > > > when pr113208_0.C is compiled where the ctor is marked constexpr, > > > we no longer perform this optimization, where > > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the > > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and > > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it, > > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in > > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same > > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in > > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group. > > This seems like an ABI bug that could use a non-LTO testcase. Well, except for the issues it causes to LTO I think it is compatible, worst case we get the body of the ctor duplicated in the executable and the linker picks some of the weak symbols as the symbol definitions. Anyway, I've added a non-LTO testcase for that in the patch below. > Hmm, cloning the bodies and then discarding them later seems like more extra > work than creating the cgraph nodes. So, I've tried to handle that in tentative_decl_linkage, like that function already handles functions declared inline except for implicit template instantiations. If we expect that import_export_decl will do comdat_linkage for the ctor later on do it right away. That fixes the testcases too, but seems to regress +FAIL: libstdc++-abi/abi_check on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from libstdc++.so.6: _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev Will need to study why that happened, it might be that it was ok because I think the filesystem stuff is unlike the rest compiled with no exported templates, but would need at least some hacks in libstdc++ to preserve previously exported symbols. Still, feels like a risky change this late if it wouldn't break ABI of other libraries. 2024-04-23 Jakub Jelinek PR lto/113208 * decl2.cc (tentative_decl_linkage): Use comdat_linkage also for implicit instantiations of maybe in charge ctors/dtors if -fimplicit-templates or -fimplicit-inline-templates and -fweak and target supports aliases. * g++.dg/abi/comdat2.C: New test. * g++.dg/lto/pr113208_0.C: New test. * g++.dg/lto/pr113208_1.C: New file. * g++.dg/lto/pr113208.h: New file. --- gcc/cp/decl2.cc.jj 2024-04-22 15:16:55.328548807 +0200 +++ gcc/cp/decl2.cc 2024-04-23 09:52:18.993250442 +0200 @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) to mark the functions at this point. */ if (DECL_DECLARED_INLINE_P (decl) && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + || DECL_DEFAULTED_FN (decl) + /* For implicit instantiations of cdtors, +if import_export_decl would use comdat linkage, +make sure to use it right away, so that maybe_clone_body +can use aliases. See PR113208. */ + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) + && (flag_implicit_templates + || flag_implicit_inline_templates) + && flag_weak + && TARGET_SUPPORTS_ALIASES))) { /* This function must have external linkage, as otherwise DECL_INTERFACE_KNOWN would have been --- gcc/testsuite/g++.dg/abi/comdat2.C.jj 2024-04-23 10:04:28.485964610 +0200 +++ gcc/testsuite/g++.dg/abi/comdat2.C 2024-04-23 10:05:24.757171194 +0200 @@ -0,0 +1,26 @@ +// PR lto/113208 +// { dg-do compile { target { c++11 && { *-*-*gnu* } } } } +// { dg-additional-options "-O2 -fkeep-inline-functions" } +// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } } +// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } } +// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } } + +template +struct A { + int foo () const; + A (int, int); +}; +template +struct B : A { + constexpr B (const B ) : A (1, x.foo ()) {} + B () : A (1, 2) {} +}; +struct C; +struct D : B {}; +void
[Committed] s390x: Fix vec_xl/vec_xst type aliasing [PR114676]
The requirements of the vec_xl/vec_xst intrinsincs wrt aliasing of the pointer argument are not really documented. As it turns out, users are likely to get it wrong. With this patch we let the pointer argument alias everything in order to make it more robust for users. Committed to mainline. Will be cherry-picked for stable branches as well. gcc/ChangeLog: PR target/114676 * config/s390/s390-c.cc (s390_expand_overloaded_builtin): Use a MEM_REF with an addend of type ptr_type_node. gcc/testsuite/ChangeLog: PR target/114676 * gcc.target/s390/zvector/pr114676.c: New test. Suggested-by: Jakub Jelinek --- gcc/config/s390/s390-c.cc | 16 +--- .../gcc.target/s390/zvector/pr114676.c| 19 +++ 2 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/s390/zvector/pr114676.c diff --git a/gcc/config/s390/s390-c.cc b/gcc/config/s390/s390-c.cc index 8d3d1a467a8..1bb6e810766 100644 --- a/gcc/config/s390/s390-c.cc +++ b/gcc/config/s390/s390-c.cc @@ -498,11 +498,11 @@ s390_expand_overloaded_builtin (location_t loc, /* Build a vector type with the alignment of the source location in order to enable correct alignment hints to be generated for vl. */ - tree mem_type = build_aligned_type (return_type, - TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[1]; + unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[1]))); + tree mem_type = build_aligned_type (return_type, align); return build2 (MEM_REF, mem_type, fold_build_pointer_plus ((*arglist)[1], (*arglist)[0]), - build_int_cst (TREE_TYPE ((*arglist)[1]), 0)); + build_int_cst (ptr_type_node, 0)); } case S390_OVERLOADED_BUILTIN_s390_vec_xst: case S390_OVERLOADED_BUILTIN_s390_vec_xstd2: @@ -511,11 +511,13 @@ s390_expand_overloaded_builtin (location_t loc, /* Build a vector type with the alignment of the target location in order to enable correct alignment hints to be generated for vst. */ - tree mem_type = build_aligned_type (TREE_TYPE((*arglist)[0]), - TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[2]; + unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[2]))); + tree mem_type = build_aligned_type (TREE_TYPE ((*arglist)[0]), align); return build2 (MODIFY_EXPR, mem_type, - build1 (INDIRECT_REF, mem_type, - fold_build_pointer_plus ((*arglist)[2], (*arglist)[1])), + build2 (MEM_REF, mem_type, + fold_build_pointer_plus ((*arglist)[2], + (*arglist)[1]), + build_int_cst (ptr_type_node, 0)), (*arglist)[0]); } case S390_OVERLOADED_BUILTIN_s390_vec_load_pair: diff --git a/gcc/testsuite/gcc.target/s390/zvector/pr114676.c b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c new file mode 100644 index 000..bdc66b2920a --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c @@ -0,0 +1,19 @@ +/* { dg-do run { target { s390*-*-* } } } */ +/* { dg-options "-O3 -mzarch -march=z14 -mzvector" } */ + +#include + +void __attribute__((noinline)) foo (int *mem) +{ + vec_xst ((vector float){ 1.0f, 2.0f, 3.0f, 4.0f }, 0, (float*)mem); +} + +int +main () +{ + int m[4] = { 0 }; + foo (m); + if (m[3] == 0) +__builtin_abort (); + return 0; +} -- 2.44.0
[gcc r14-10087] s390x: Fix vec_xl/vec_xst type aliasing [PR114676]
https://gcc.gnu.org/g:42189f21b22c43ac8ab46edf5f6a7b4d99bc86a5 commit r14-10087-g42189f21b22c43ac8ab46edf5f6a7b4d99bc86a5 Author: Andreas Krebbel Date: Tue Apr 23 10:05:46 2024 +0200 s390x: Fix vec_xl/vec_xst type aliasing [PR114676] The requirements of the vec_xl/vec_xst intrinsincs wrt aliasing of the pointer argument are not really documented. As it turns out, users are likely to get it wrong. With this patch we let the pointer argument alias everything in order to make it more robust for users. gcc/ChangeLog: PR target/114676 * config/s390/s390-c.cc (s390_expand_overloaded_builtin): Use a MEM_REF with an addend of type ptr_type_node. gcc/testsuite/ChangeLog: PR target/114676 * gcc.target/s390/zvector/pr114676.c: New test. Suggested-by: Jakub Jelinek Diff: --- gcc/config/s390/s390-c.cc| 16 +--- gcc/testsuite/gcc.target/s390/zvector/pr114676.c | 19 +++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/gcc/config/s390/s390-c.cc b/gcc/config/s390/s390-c.cc index 8d3d1a467a8..1bb6e810766 100644 --- a/gcc/config/s390/s390-c.cc +++ b/gcc/config/s390/s390-c.cc @@ -498,11 +498,11 @@ s390_expand_overloaded_builtin (location_t loc, /* Build a vector type with the alignment of the source location in order to enable correct alignment hints to be generated for vl. */ - tree mem_type = build_aligned_type (return_type, - TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[1]; + unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[1]))); + tree mem_type = build_aligned_type (return_type, align); return build2 (MEM_REF, mem_type, fold_build_pointer_plus ((*arglist)[1], (*arglist)[0]), - build_int_cst (TREE_TYPE ((*arglist)[1]), 0)); + build_int_cst (ptr_type_node, 0)); } case S390_OVERLOADED_BUILTIN_s390_vec_xst: case S390_OVERLOADED_BUILTIN_s390_vec_xstd2: @@ -511,11 +511,13 @@ s390_expand_overloaded_builtin (location_t loc, /* Build a vector type with the alignment of the target location in order to enable correct alignment hints to be generated for vst. */ - tree mem_type = build_aligned_type (TREE_TYPE((*arglist)[0]), - TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[2]; + unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[2]))); + tree mem_type = build_aligned_type (TREE_TYPE ((*arglist)[0]), align); return build2 (MODIFY_EXPR, mem_type, - build1 (INDIRECT_REF, mem_type, - fold_build_pointer_plus ((*arglist)[2], (*arglist)[1])), + build2 (MEM_REF, mem_type, + fold_build_pointer_plus ((*arglist)[2], + (*arglist)[1]), + build_int_cst (ptr_type_node, 0)), (*arglist)[0]); } case S390_OVERLOADED_BUILTIN_s390_vec_load_pair: diff --git a/gcc/testsuite/gcc.target/s390/zvector/pr114676.c b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c new file mode 100644 index 000..bdc66b2920a --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c @@ -0,0 +1,19 @@ +/* { dg-do run { target { s390*-*-* } } } */ +/* { dg-options "-O3 -mzarch -march=z14 -mzvector" } */ + +#include + +void __attribute__((noinline)) foo (int *mem) +{ + vec_xst ((vector float){ 1.0f, 2.0f, 3.0f, 4.0f }, 0, (float*)mem); +} + +int +main () +{ + int m[4] = { 0 }; + foo (m); + if (m[3] == 0) +__builtin_abort (); + return 0; +}
Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On 4/22/24 08:42, Jakub Jelinek wrote: On Wed, Apr 17, 2024 at 09:42:47AM +0200, Jakub Jelinek wrote: When expand_or_defer_fn is called at_eof time, it calls import_export_decl and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a couple of places to try to optimize cdtors which are known to have the same body by making the complete cdtor an alias to base cdtor (and in that case also uses *[CD]5* as comdat group name instead of the normal comdat group names specific to each mangled name). Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN, maybe_clone_body and can_alias_cdtor use: if (DECL_ONE_ONLY (fn)) cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone)); ... bool can_alias = can_alias_cdtor (fn); ... /* Tell cgraph if both ctors or both dtors are known to have the same body. */ if (can_alias && fns[0] && idx == 1 && cgraph_node::get_create (fns[0])->create_same_body_alias (clone, fns[0])) { alias = true; if (DECL_ONE_ONLY (fns[0])) { /* For comdat base and complete cdtors put them into the same, *[CD]5* comdat group instead of *[CD][12]*. */ comdat_group = cdtor_comdat_group (fns[1], fns[0]); cgraph_node::get_create (fns[0])->set_comdat_group (comdat_group); if (symtab_node::get (clone)->same_comdat_group) symtab_node::get (clone)->remove_from_same_comdat_group (); symtab_node::get (clone)->add_to_same_comdat_group (symtab_node::get (fns[0])); } } and /* Don't use aliases for weak/linkonce definitions unless we can put both symbols in the same COMDAT group. */ return (DECL_INTERFACE_KNOWN (fn) && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) && (!DECL_ONE_ONLY (fn) || (HAVE_COMDAT_GROUP && DECL_WEAK (fn; The following testcase regressed with Marek's r14-5979 change, when pr113208_0.C is compiled where the ctor is marked constexpr, we no longer perform this optimization, where _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it, instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in _ZN6vectorI12QualityValueEC1ERKS1_ comdat group. This seems like an ABI bug that could use a non-LTO testcase. Now, the linker seems to somehow cope with that, eventhough it probably keeps both copies of the ctor, but seems LTO can't cope with that and Honza doesn't know what it should do in that case (linker decides that the prevailing symbol is _ZN6vectorI12QualityValueEC2ERKS1_ (from the _ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and _ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU, from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)). Note, the case where some constructor is marked constexpr in one TU and not in another one happens pretty often in libstdc++ when one mixes -std= flags used to compile different compilation units. The reason the optimization doesn't trigger when the constructor is constexpr is that expand_or_defer_fn is called in that case much earlier than when it is not constexpr; in the former case it is called when we try to constant evaluate that constructor. But DECL_INTERFACE_KNOWN is false in that case and comdat_linkage hasn't been called either (I think it is desirable, because comdat group is stored in the cgraph node and am not sure it is a good idea to create cgraph nodes for something that might not be needed later on at all), so maybe_clone_body clones the bodies, but doesn't make them as aliases. Hmm, cloning the bodies and then discarding them later seems like more extra work than creating the cgraph nodes. Jason
[PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 17, 2024 at 09:42:47AM +0200, Jakub Jelinek wrote: > When expand_or_defer_fn is called at_eof time, it calls import_export_decl > and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a > couple of places to try to optimize cdtors which are known to have the > same body by making the complete cdtor an alias to base cdtor (and in > that case also uses *[CD]5* as comdat group name instead of the normal > comdat group names specific to each mangled name). > Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN, > maybe_clone_body and can_alias_cdtor use: > if (DECL_ONE_ONLY (fn)) > cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group > (clone)); > ... > bool can_alias = can_alias_cdtor (fn); > ... > /* Tell cgraph if both ctors or both dtors are known to have > the same body. */ > if (can_alias > && fns[0] > && idx == 1 > && cgraph_node::get_create (fns[0])->create_same_body_alias >(clone, fns[0])) > { > alias = true; > if (DECL_ONE_ONLY (fns[0])) > { > /* For comdat base and complete cdtors put them > into the same, *[CD]5* comdat group instead of > *[CD][12]*. */ > comdat_group = cdtor_comdat_group (fns[1], fns[0]); > cgraph_node::get_create (fns[0])->set_comdat_group > (comdat_group); > if (symtab_node::get (clone)->same_comdat_group) > symtab_node::get (clone)->remove_from_same_comdat_group (); > symtab_node::get (clone)->add_to_same_comdat_group > (symtab_node::get (fns[0])); > } > } > and > /* Don't use aliases for weak/linkonce definitions unless we can put both > symbols in the same COMDAT group. */ > return (DECL_INTERFACE_KNOWN (fn) > && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn)) > && (!DECL_ONE_ONLY (fn) > || (HAVE_COMDAT_GROUP && DECL_WEAK (fn; > The following testcase regressed with Marek's r14-5979 change, > when pr113208_0.C is compiled where the ctor is marked constexpr, > we no longer perform this optimization, where > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it, > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group. > Now, the linker seems to somehow cope with that, eventhough it > probably keeps both copies of the ctor, but seems LTO can't cope > with that and Honza doesn't know what it should do in that case > (linker decides that the prevailing symbol is > _ZN6vectorI12QualityValueEC2ERKS1_ (from the > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and > _ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU, > from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)). > > Note, the case where some constructor is marked constexpr in one > TU and not in another one happens pretty often in libstdc++ when > one mixes -std= flags used to compile different compilation units. > > The reason the optimization doesn't trigger when the constructor is > constexpr is that expand_or_defer_fn is called in that case much earlier > than when it is not constexpr; in the former case it is called when we > try to constant evaluate that constructor. But DECL_INTERFACE_KNOWN > is false in that case and comdat_linkage hasn't been called either > (I think it is desirable, because comdat group is stored in the cgraph > node and am not sure it is a good idea to create cgraph nodes for > something that might not be needed later on at all), so maybe_clone_body > clones the bodies, but doesn't make them as aliases. > > The following patch is an attempt to redo this optimization when > import_export_decl is called at_eof time on the base/complete cdtor > (or deleting dtor). It will not do anything if maybe_clone_body > hasn't been called uyet (the TREE_ASM_WRITTEN check on the > DECL_MAYBE_IN_CHARGE_CDTOR_P), or when one or both of the base/complete > cdtors have been lowered already, or when maybe_clone_body called > maybe_thunk_body and it was successful. Otherwise retries the > can_alias_cdtor check and makes the complete cdtor alias to the > base cdtor with adjustments to the comdat group. Here is an updated version of the patch which changes - DECL_SAVED_TREE (fns[1]) = NULL_TREE; + release_function_body (fns[1]); as suggested by Honza. Bootstrapped/regtested on x86_64-linux and i686-linux again, ok for trunk? 2024-04-22 Jakub Jelinek PR lto/113208 * cp-tree.h (maybe_optimize_cdtor): Declare. * decl2.cc (import_export_decl): Call it for cloned cdtors. * optimize.cc