Re: [PATCH] Add %[zt][diox] support to pretty-print
On Wed, May 22, 2024 at 05:23:33PM +0800, YunQiang Su wrote: > Jakub Jelinek 于2024年5月22日周三 17:14写道: > > > > On Wed, May 22, 2024 at 05:05:30PM +0800, YunQiang Su wrote: > > > > --- gcc/gcc.cc.jj 2024-02-09 14:54:09.141489744 +0100 > > > > +++ gcc/gcc.cc 2024-02-09 22:04:37.655678742 +0100 > > > > @@ -2410,8 +2410,7 @@ read_specs (const char *filename, bool m > > > > if (*p1++ != '<' || p[-2] != '>') > > > > fatal_error (input_location, > > > > "specs %%include syntax malformed after " > > > > -"%ld characters", > > > > -(long) (p1 - buffer + 1)); > > > > +"%td characters", p1 - buffer + 1); > > > > > > > > > > Should we use %td later for gcc itself? Since we may use older > > > compiler to build gcc. > > > My major workstation is Debian Bookworm, which has GCC 12, and then I > > > get some warnings: > > > > That is fine and expected. During stage1 such warnings are intentionally > > not fatal, only in stage2+ when we know it is the same version of gcc > > we want those can be fatal. > > It may have only 1 stage in some cases. > For example we have a full binutils/libc stack, and just build a cross-gcc. > For all libraries for target, such as libgcc etc, it is OK; while for > host executables > it will be a problem. That is still ok, it is just a warning about unknown gcc format specifiers, at runtime the code from the compiler being built will be used and that handles those. We have added dozens of these over years, %td/%zd certainly aren't an exception. Just try to build with some older gcc version, say 4.8.5, and you'll see far more such warnings. But also as recommended, you shouldn't be building cross-gcc with old version of gcc, you should use same version of the native compiler to build the cross compiler. https://gcc.gnu.org/install/build.html "To build a cross compiler, we recommend first building and installing a native compiler. You can then use the native GCC compiler to build the cross compiler." Jakub
Re: [PATCH] Add %[zt][diox] support to pretty-print
On Wed, May 22, 2024 at 05:05:30PM +0800, YunQiang Su wrote: > > --- gcc/gcc.cc.jj 2024-02-09 14:54:09.141489744 +0100 > > +++ gcc/gcc.cc 2024-02-09 22:04:37.655678742 +0100 > > @@ -2410,8 +2410,7 @@ read_specs (const char *filename, bool m > > if (*p1++ != '<' || p[-2] != '>') > > fatal_error (input_location, > > "specs %%include syntax malformed after " > > -"%ld characters", > > -(long) (p1 - buffer + 1)); > > +"%td characters", p1 - buffer + 1); > > > > Should we use %td later for gcc itself? Since we may use older > compiler to build gcc. > My major workstation is Debian Bookworm, which has GCC 12, and then I > get some warnings: That is fine and expected. During stage1 such warnings are intentionally not fatal, only in stage2+ when we know it is the same version of gcc we want those can be fatal. Otherwise we could never add any new modifies... Jakub
Re: [PATCH] Don't simplify NAN/INF or out-of-range constant for FIX/UNSIGNED_FIX.
On Wed, May 22, 2024 at 09:46:41AM +0200, Richard Biener wrote: > On Wed, May 22, 2024 at 3:58 AM liuhongt wrote: > > > > According to IEEE standard, for conversions from floating point to > > integer. When a NaN or infinite operand cannot be represented in the > > destination format and this cannot otherwise be indicated, the invalid > > operation exception shall be signaled. When a numeric operand would > > convert to an integer outside the range of the destination format, the > > invalid operation exception shall be signaled if this situation cannot > > otherwise be indicated. > > > > The patch prevent simplication of the conversion from floating point > > to integer for NAN/INF/out-of-range constant when flag_trapping_math. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,} > > Ok for trunk? > > OK if there are no further comments today. As I wrote in the PR, I don't think this is the right fix for the PR, the simplify-rtx.cc change is the right thing to do, the C standard in F.4 says that the out of range conversions to integers should raise exceptions, but still says that the resulting value in those cases is unspecified. So, for the C part we should verify that with -ftrapping-math we don't constant fold it and cover it both by pure C and perhaps backend specific testcases which just search asm for the conversion instructions or even runtime test which tests that the exceptions are triggered, verify that we don't fold it either during GIMPLE opts or RTL opts (dunno whether they can be folded in e.g. C constant initializers or not). And then on the backend side, it should stop using FIX/UNSIGNED_FIX RTLs in patterns which are used for the intrinsics (and keep using them in patterns used for C scalar/vector conversions), because even with -fno-trapping-math the intrinsics should have the documented behavior, particular result value, while in C they are clearly unspecified and FIX/UNSIGNED_FIX folding even with the patch chooses some particular values which don't match those (sure, they are like that because of Java, but am not sure it is the right time to change what we do in those cases say by providing a target hook to pick a different value). The provided testcase tests the values though, so I think is inappropriate for this patch. Jakub
[PATCH] strlen: Fix up !si->full_string_p handling in count_nonzero_bytes_addr [PR115152]
Hi! The following testcase is miscompiled because strlen_pass::count_nonzero_bytes_addr doesn't handle correctly the !si->full_string_p case. If si->full_string_p, it correctly computes minlen and maxlen as minimum and maximum length of the '\0' terminated stgring and clears *nulterm (ie. makes sure !full_string_p in the ultimate caller) if minlen is equal or larger than nbytes and so '\0' isn't guaranteed to be among those bytes. But in the !si->full_string_p case, all we know is that there are [minlen,maxlen] non-zero bytes followed by unknown bytes, so effectively the maxlen is infinite (but caller cares about only the first nbytes bytes) and furthermore, we never know if there is any '\0' char among those, so *nulterm needs to be always cleared. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and affected release branches? 2024-05-21 Jakub Jelinek PR tree-optimization/115152 * tree-ssa-strlen.cc (strlen_pass::count_nonzero_bytes_addr): If !si->full_string_p, clear *nulterm and set maxlen to nbytes. * gcc.dg/pr115152.c: New test. --- gcc/tree-ssa-strlen.cc.jj 2024-04-29 11:00:45.0 +0200 +++ gcc/tree-ssa-strlen.cc 2024-05-21 13:43:31.031208000 +0200 @@ -4829,7 +4829,7 @@ strlen_pass::count_nonzero_bytes_addr (t if (maxlen + 1 < nbytes) return false; - if (nbytes <= minlen) + if (nbytes <= minlen || !si->full_string_p) *nulterm = false; if (nbytes < minlen) @@ -4839,6 +4839,9 @@ strlen_pass::count_nonzero_bytes_addr (t maxlen = nbytes; } + if (!si->full_string_p) + maxlen = nbytes; + if (minlen < lenrange[0]) lenrange[0] = minlen; if (lenrange[1] < maxlen) --- gcc/testsuite/gcc.dg/pr115152.c.jj 2024-05-21 13:46:02.793214348 +0200 +++ gcc/testsuite/gcc.dg/pr115152.c 2024-05-21 12:49:38.791626073 +0200 @@ -0,0 +1,17 @@ +/* PR tree-optimization/115152 */ +/* { dg-do run } */ +/* { dg-options "-O3 -fno-tree-fre -fno-tree-dominator-opts -fno-tree-loop-im" } */ + +int a, b, c, d; +signed char e[1] = { 1 }; + +int +main () +{ + for (a = 0; a < 3; a++) +for (b = 0; b < 2; b++) + c = e[0] = e[0] ^ d; + if (!c) +__builtin_abort (); + return 0; +} Jakub
[PATCH] ubsan: Use right address space for MEM_REF created for bool/enum sanitization [PR115172]
Hi! The following testcase is miscompiled, because -fsanitize=bool,enum creates a MEM_REF without propagating there address space qualifiers, so what should be normally loaded using say %gs:/%fs: segment prefix isn't. Together with asan it then causes that load to be sanitized. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and release branches? 2024-05-21 Jakub Jelinek PR sanitizer/115172 * ubsan.cc (instrument_bool_enum_load): If rhs is not in generic address space, use qualified version of utype with the right address space. Formatting fix. * gcc.dg/asan/pr115172.c: New test. --- gcc/ubsan.cc.jj 2024-03-22 09:23:37.695296775 +0100 +++ gcc/ubsan.cc2024-05-21 12:10:24.261454107 +0200 @@ -1776,13 +1776,17 @@ instrument_bool_enum_load (gimple_stmt_i || TREE_CODE (gimple_assign_lhs (stmt)) != SSA_NAME) return; + addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (rhs)); + if (as != TYPE_ADDR_SPACE (utype)) +utype = build_qualified_type (utype, TYPE_QUALS (utype) +| ENCODE_QUAL_ADDR_SPACE (as)); bool ends_bb = stmt_ends_bb_p (stmt); location_t loc = gimple_location (stmt); tree lhs = gimple_assign_lhs (stmt); tree ptype = build_pointer_type (TREE_TYPE (rhs)); tree atype = reference_alias_ptr_type (rhs); gimple *g = gimple_build_assign (make_ssa_name (ptype), - build_fold_addr_expr (rhs)); + build_fold_addr_expr (rhs)); gimple_set_location (g, loc); gsi_insert_before (gsi, g, GSI_SAME_STMT); tree mem = build2 (MEM_REF, utype, gimple_assign_lhs (g), --- gcc/testsuite/gcc.dg/asan/pr115172.c.jj 2024-05-21 17:28:18.302815400 +0200 +++ gcc/testsuite/gcc.dg/asan/pr115172.c2024-05-21 22:50:43.272753785 +0200 @@ -0,0 +1,20 @@ +/* PR sanitizer/115172 */ +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ +/* { dg-options "-O2 -fsanitize=address,bool -ffat-lto-objects -fdump-tree-asan1" } */ +/* { dg-final { scan-tree-dump-not "\.ASAN_CHECK " "asan1" } } */ + +#ifdef __x86_64__ +#define SEG __seg_gs +#else +#define SEG __seg_fs +#endif + +extern struct S { _Bool b; } s; +void bar (void); + +void +foo (void) +{ + if (*(volatile _Bool SEG *) (__UINTPTR_TYPE__) ) +bar (); +} Jakub
Re: [Patch] contrib/gcc-changelog/git_update_version.py: Improve diagnostic
On Tue, May 21, 2024 at 09:36:05AM +0200, Tobias Burnus wrote: > Jakub Jelinek wrote: > > On Mon, May 20, 2024 at 08:31:02AM +0200, Tobias Burnus wrote: > > > Hmm, there were now two daily bumps: [...] I really wonder why. > > Because I've done it by hand. > > Okay, that explains it. > > I still do not understand why it slipped through at the first place; I tried > old versions down to r12-709-g772e5e82e3114f and it still FAIL for the > invalid commit ("ERR: cannot find a ChangeLog location in message"). > > Thus, I wonder whether the commit hook is active at all?!? They are. But https://github.com/AdaCore/git-hooks/blob/master/hooks/updates/__init__.py#L836 with https://github.com/AdaCore/git-hooks/blob/master/hooks/updates/commits.py#L230 bypasses all commits which contain just 3 magic words in a row. And because that part is owned by AdaCore hooks, not the GCC customizations, not sure what to do about that. > > I have in ~gccadmin a gcc-changelog copy and adjusted update_version_git > > script which doesn't use contrib/gcc-changelog subdirectory from the > > checkout it makes but from the ~gccadmin directory, > [...] > > I'm already using something similar in > > my hack (just was doing it for even successful commits, but I think your > > patch is better). > > And, I think best would be if update_version_git script simply > > accepted a list of ignored commits from the command line too, > > passed it to the git_update_version.py script and that one > > added those to IGNORED_COMMITS. > > Updated version: > > * Uses my diagnostic > > * Adds an -i/--ignore argument for commits. Permits to use '-i hash1 -i > hash2' but also '-i hash1,hash2' or '-i "hash1 hash2' > > * I changed the global variable to lower case as Python's style guide states > that all uppercase variables is for constants. > > * The '=None' matches one of the current usages (no argument passed); hence, > it is now explicit and 'pylint' is happy. > > OK for mainline? Yes, thanks. > PS: I have not updated the hashes. If needed/wanted, I leave that to you, > Jakub. Once some commit is ignored, we won't be processing it anymore, so I think the -i option is all we need. Jakub
Re: [Patch] contrib/gcc-changelog/git_update_version.py: Improve diagnostic (was: [Patch] contrib/gcc-changelog/git_update_version.py: Add ignore commit, improve diagnostic)
On Mon, May 20, 2024 at 08:31:02AM +0200, Tobias Burnus wrote: > Hmm, there were now two daily bumps: > > Date: Mon May 20 00:16:30 2024 + > > Date: Sun May 19 18:15:28 2024 + > > I really wonder why. Because I've done it by hand. I have in ~gccadmin a gcc-changelog copy and adjusted update_version_git script which doesn't use contrib/gcc-changelog subdirectory from the checkout it makes but from the ~gccadmin directory, because I don't want to constantly try to add some commit number to IGNORED_COMMITS, see that it either works or doesn't (I think sometimes it needs the hash of the revert commit, at other times the commit hash referenced in the revert commit) or that further ones are needed. > From f56b1764f2b5c2c83c6852607405e5be0a763a2c Mon Sep 17 00:00:00 2001 > From: Tobias Burnus > Date: Sun, 19 May 2024 08:17:42 +0200 > Subject: [PATCH] contrib/gcc-changelog/git_update_version.py: Improve > diagnostic > > contrib/ChangeLog: > > * gcc-changelog/git_update_version.py (prepend_to_changelog_files): > Output 8 spaces rather than tab > git hash in case errors occurred. > > diff --git a/contrib/gcc-changelog/git_update_version.py > b/contrib/gcc-changelog/git_update_version.py > index 24f6c43d0b2..ec0151b83fe 100755 > --- a/contrib/gcc-changelog/git_update_version.py > +++ b/contrib/gcc-changelog/git_update_version.py > @@ -58,6 +58,7 @@ def read_timestamp(path): > > def prepend_to_changelog_files(repo, folder, git_commit, add_to_git): > if not git_commit.success: > +logging.info(f"While processing {git_commit.info.hexsha}:") > for error in git_commit.errors: > logging.info(error) > raise AssertionError() So, your commit is useful part of it, I'm already using something similar in my hack (just was doing it for even successful commits, but I think your patch is better). And, I think best would be if update_version_git script simply accepted a list of ignored commits from the command line too, passed it to the git_update_version.py script and that one added those to IGNORED_COMMITS. Because typically if the DATESTAMP/ChangeLog updates gets stuck, one doesn't just adjust IGNORED_COMMITS and wait up to another day to see if it worked, but runs the script by hand to make sure it works. --- gcc-checkout/contrib/gcc-changelog/git_update_version.py2024-05-13 16:52:57.890151748 + +++ gcc-changelog/git_update_version.py 2024-05-19 18:13:44.953648834 + @@ -41,7 +41,21 @@ IGNORED_COMMITS = ( '040e5b0edbca861196d9e2ea2af5e805769c8d5d', '8057f9aa1f7e70490064de796d7a8d42d446caf8', '109f1b28fc94c93096506e3df0c25e331cef19d0', -'39f81924d88e3cc197fc3df74204c9b5e01e12f7') +'39f81924d88e3cc197fc3df74204c9b5e01e12f7', +'d7bb8eaade3cd3aa70715c8567b4d7b08098e699', +'89feb3557a018893cfe50c2e07f91559bd3cde2b', +'ccf8d3e3d26c6ba3d5e11fffeed8d64018e9c060', +'e0c52905f666e3d23881f82dbf39466a24f009f4', +'b38472ffc1e631bd357573b44d956ce16d94e666', +'a0b13d0860848dd5f2876897ada1e22e4e681e91', +'b8c772cae97b54386f7853edf0f9897012bfa90b', +'810d35a7e054bcbb5b66d2e5924428e445f5fba9', +'0df1ee083434ac00ecb19582b1e5b25e105981b2', +'2c688f6afce4cbb414f5baab1199cd525f309fca', +'60dcb710b6b4aa22ea96abc8df6dfe9067f3d7fe', +'44968a0e00f656e9bb3e504bb2fa1a8282002015', +'d7bb8eaade3cd3aa70715c8567b4d7b08098e699', +'da73261ce7731be7f2b164f1db796878cdc23365') FORMAT = '%(asctime)s:%(levelname)s:%(name)s:%(message)s' logging.basicConfig(level=logging.INFO, format=FORMAT, @@ -125,6 +139,7 @@ def update_current_branch(ref_name): % (commit.hexsha, head.hexsha), ref_name) commits = [c for c in commits if c.info.hexsha not in IGNORED_COMMITS] for git_commit in reversed(commits): +logging.info('trying %s', git_commit.info.hexsha) prepend_to_changelog_files(repo, args.git_path, git_commit, not args.dry_mode) if args.dry_mode: Jakub
Re: [PATCH] libstdc++: increment *this instead of this
On Sat, May 18, 2024 at 02:53:20PM +0800, Kefu Chai wrote: > libstdc++-v3/ChangeLog: > > * include/bits/unicode.h (enable_borrowed_range): Call ++(*this) > instead of ++this This should be already fixed, see https://gcc.gnu.org/PR115119 Jakub
Re: [PATCH] Use DW_TAG_module for Ada
On Fri, May 03, 2024 at 11:08:04AM -0600, Tom Tromey wrote: > DWARF is not especially clear on the distinction between > DW_TAG_namespace and DW_TAG_module, but I think that DW_TAG_module is > more appropriate for Ada. This patch changes the compiler to do this. > Note that the Ada compiler does not yet create NAMESPACE_DECLs. > > gcc > > * dwarf2out.cc (gen_namespace_die): Use DW_TAG_module for Ada. Ok, thanks. > diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc > index 1b0e8b5a5b2..1e46c27cdf7 100644 > --- a/gcc/dwarf2out.cc > +++ b/gcc/dwarf2out.cc > @@ -26992,7 +26992,7 @@ gen_namespace_die (tree decl, dw_die_ref context_die) > { >/* Output a real namespace or module. */ >context_die = setup_namespace_context (decl, comp_unit_die ()); > - namespace_die = new_die (is_fortran () || is_dlang () > + namespace_die = new_die (is_fortran () || is_dlang () || is_ada () > ? DW_TAG_module : DW_TAG_namespace, > context_die, decl); >/* For Fortran modules defined in different CU don't add src coords. > */ > -- > 2.44.0 Jakub
C++ Patch ping - Re: [PATCH] c++: Fix parsing of abstract-declarator starting with ... followed by [ or ( [PR115012]
Hi! I'd like to ping the https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651199.html patch. Thanks. On Thu, May 09, 2024 at 08:12:30PM +0200, Jakub Jelinek wrote: > The C++26 P2662R3 Pack indexing paper mentions that both GCC > and MSVC don't handle T...[10] parameter declaration when T > is a pack. While that will change meaning in C++26, in C++11 .. C++23 > this ought to be valid. Also, T...(args) as well. > > The following patch handles those in cp_parser_direct_declarator. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2024-05-09 Jakub Jelinek > > PR c++/115012 > * parser.cc (cp_parser_direct_declarator): Handle > abstract declarator starting with ... followed by [ > or (. > > * g++.dg/cpp0x/variadic185.C: New test. > * g++.dg/cpp0x/variadic186.C: New test. Jakub
Re: [COMMITTED] Revert "Revert: "Enable prange support.""
On Thu, May 16, 2024 at 12:14:09PM +0200, Aldy Hernandez wrote: > Wait, what's the preferred way of reverting a patch? I followed what I saw > in: Reverting a patch (that isn't a reversion) just push git revert. The important part is not to modify the This reverts commit line from what git revert created. > commit 04ee1f788ceaa4c7f777ff3b9441ae076191439c > Author: Jeff Law > Date: Mon May 13 21:42:38 2024 -0600 > > Revert "[PATCH v2 1/3] RISC-V: movmem for RISCV with V extension" > > This reverts commit df15eb15b5f820321c81efc75f0af13ff8c0dd5b. So, this is just fine. > and here: > > commit 0c6dd4b0973738ce43e76b468a002ab5eb58aaf4 > Author: YunQiang Su > Date: Mon May 13 14:15:38 2024 +0800 > > Revert "MIPS: Support constraint 'w' for MSA instruction" > > This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8. And this too. What is not fine is hand edit the message: This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8 because foo and bar. You can do that separately, so This reverts commit 9ba01240864ac446052d97692e2199539b7c76d8. The reversion is because of foo and bar. Or being further creative: This reverts commit r13-8390-g9de6ff5ec9a46951d2. etc. > commit f6ce85502eb2e4e7bbd9b3c6c1c065a004f8f531 > Author: Hans-Peter Nilsson > Date: Wed May 8 04:11:20 2024 +0200 > > Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle > xpass from combine improvement"" > > This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7. This one is not fine. Our current infrastructure for ChangeLog generation can't deal with that and there is no agreement what to write in the ChangeLog for it anyway, whether 2 reversions turn it into Reapply commit: or 2 Revert: lines? What happens on 3rd reversion? So, one needs to manually remove the This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7. line and supply ChangeLog entry. For cases like this or the ammended lines (or say if This reverts commit or (cherry-picked from ) lines refer to invalid commit the daily DATESTAMP update then fails, I need to add manually all problematic commits to IGNORED_COMMITS, rerun it by hand and then write the ChangeLog entries by hand. See https://gcc.gnu.org/r13-8764 https://gcc.gnu.org/r15-426 https://gcc.gnu.org/r15-345 https://gcc.gnu.org/r15-344 https://gcc.gnu.org/r15-341 https://gcc.gnu.org/r14-9832 https://gcc.gnu.org/r14-9830 for what I had to do only in April/May for this. Jakub
Re: [COMMITTED] Revert "Revert: "Enable prange support.""
On Thu, May 16, 2024 at 12:01:01PM +0200, Aldy Hernandez wrote: > This reverts commit d7bb8eaade3cd3aa70715c8567b4d7b08098e699 and enables > prange > support again. Please don't do this. This breaks ChangeLog generation, will need to handle it tomorrow by hand again. Both the ammendments to the git (cherry-pick -x or revert) added message lines This reverts commit COMMITHASH. and (cherry picked from commit COMMITHASH) and revert of revert. Jakub
[committed] openmp: Diagnose using grainsize+num_tasks clauses together [PR115103]
Hi! I've noticed that while we diagnose many other OpenMP exclusive clauses, we don't diagnose grainsize together with num_tasks on taskloop construct in all of C, C++ and Fortran (the implementation simply ignored grainsize in that case) and for Fortran also don't diagnose mixing nogroup clause with reduction clause(s). Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk. 2024-05-15 Jakub Jelinek PR c/115103 gcc/c/ * c-typeck.cc (c_finish_omp_clauses): Diagnose grainsize used together with num_tasks. gcc/cp/ * semantics.cc (finish_omp_clauses): Diagnose grainsize used together with num_tasks. gcc/fortran/ * openmp.cc (resolve_omp_clauses): Diagnose grainsize used together with num_tasks or nogroup used together with reduction. gcc/testsuite/ * c-c++-common/gomp/clause-dups-1.c: Add 2 further expected errors. * gfortran.dg/gomp/pr115103.f90: New test. --- gcc/c/c-typeck.cc.jj2024-04-22 14:46:28.917086705 +0200 +++ gcc/c/c-typeck.cc 2024-05-15 15:43:23.117428045 +0200 @@ -14722,6 +14722,8 @@ c_finish_omp_clauses (tree clauses, enum tree *detach_seen = NULL; bool linear_variable_step_check = false; tree *nowait_clause = NULL; + tree *grainsize_seen = NULL; + bool num_tasks_seen = false; tree ordered_clause = NULL_TREE; tree schedule_clause = NULL_TREE; bool oacc_async = false; @@ -16021,8 +16023,6 @@ c_finish_omp_clauses (tree clauses, enum case OMP_CLAUSE_PROC_BIND: case OMP_CLAUSE_DEVICE_TYPE: case OMP_CLAUSE_PRIORITY: - case OMP_CLAUSE_GRAINSIZE: - case OMP_CLAUSE_NUM_TASKS: case OMP_CLAUSE_THREADS: case OMP_CLAUSE_SIMD: case OMP_CLAUSE_HINT: @@ -16048,6 +16048,16 @@ c_finish_omp_clauses (tree clauses, enum pc = _CLAUSE_CHAIN (c); continue; + case OMP_CLAUSE_GRAINSIZE: + grainsize_seen = pc; + pc = _CLAUSE_CHAIN (c); + continue; + + case OMP_CLAUSE_NUM_TASKS: + num_tasks_seen = true; + pc = _CLAUSE_CHAIN (c); + continue; + case OMP_CLAUSE_MERGEABLE: mergeable_seen = true; pc = _CLAUSE_CHAIN (c); @@ -16333,6 +16343,14 @@ c_finish_omp_clauses (tree clauses, enum *nogroup_seen = OMP_CLAUSE_CHAIN (*nogroup_seen); } + if (grainsize_seen && num_tasks_seen) +{ + error_at (OMP_CLAUSE_LOCATION (*grainsize_seen), + "% clause must not be used together with " + "% clause"); + *grainsize_seen = OMP_CLAUSE_CHAIN (*grainsize_seen); +} + if (detach_seen) { if (mergeable_seen) --- gcc/cp/semantics.cc.jj 2024-05-15 15:43:05.823657545 +0200 +++ gcc/cp/semantics.cc 2024-05-15 15:44:07.085844564 +0200 @@ -7098,6 +7098,7 @@ finish_omp_clauses (tree clauses, enum c bool mergeable_seen = false; bool implicit_moved = false; bool target_in_reduction_seen = false; + bool num_tasks_seen = false; bitmap_obstack_initialize (NULL); bitmap_initialize (_head, _default_obstack); @@ -7656,6 +7657,10 @@ finish_omp_clauses (tree clauses, enum c /* FALLTHRU */ case OMP_CLAUSE_NUM_TASKS: + if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_NUM_TASKS) + num_tasks_seen = true; + /* FALLTHRU */ + case OMP_CLAUSE_NUM_TEAMS: case OMP_CLAUSE_NUM_THREADS: case OMP_CLAUSE_NUM_GANGS: @@ -9244,6 +9249,17 @@ finish_omp_clauses (tree clauses, enum c *pc = OMP_CLAUSE_CHAIN (c); continue; } + pc = _CLAUSE_CHAIN (c); + continue; + case OMP_CLAUSE_GRAINSIZE: + if (num_tasks_seen) + { + error_at (OMP_CLAUSE_LOCATION (c), + "% clause must not be used together with " + "% clause"); + *pc = OMP_CLAUSE_CHAIN (c); + continue; + } pc = _CLAUSE_CHAIN (c); continue; case OMP_CLAUSE_ORDERED: --- gcc/fortran/openmp.cc.jj2024-03-14 22:06:58.273669790 +0100 +++ gcc/fortran/openmp.cc 2024-05-15 15:43:23.122427979 +0200 @@ -9175,6 +9175,13 @@ resolve_omp_clauses (gfc_code *code, gfc resolve_positive_int_expr (omp_clauses->grainsize, "GRAINSIZE"); if (omp_clauses->num_tasks) resolve_positive_int_expr (omp_clauses->num_tasks, "NUM_TASKS"); + if (omp_clauses->grainsize && omp_clauses->num_tasks) +gfc_error ("% clause at %L must not be used together with " + "% clause", _clauses->grainsize->where); + if (omp_clauses->lists[OMP_LIST_REDUCTION] && omp_clauses->nogroup) +gfc_error ("% clause at %L must not be used together with " + "% clause", + _clauses->lists[OMP_LIST_REDUCTION]->
[committed] combine: Fix up simplify_compare_const [PR115092]
Hi! The following testcases are miscompiled (with tons of GIMPLE optimization disabled) because combine sees GE comparison of 1-bit sign_extract (i.e. something with [-1, 0] value range) with (const_int -1) (which is always true) and optimizes it into NE comparison of 1-bit zero_extract ([0, 1] value range) against (const_int 0). The reason is that simplify_compare_const first (correctly) simplifies the comparison to GE (ashift:SI something (const_int 31)) (const_int -2147483648) and then an optimization for when the second operand is power of 2 triggers. That optimization is fine for power of 2s which aren't the signed minimum of the mode, or if it is NE, EQ, GEU or LTU against the signed minimum of the mode, but for GE or LT optimizing it into NE (or EQ) against const0_rtx is wrong, those cases are always true or always false (but the function doesn't have a standardized way to tell callers the comparison is now unconditional). The following patch just disables the optimization in that case. Bootstrapped/regtested on x86_64-linux and i686-linux, preapproved by Segher in the PR, committed to trunk so far. 2024-05-15 Jakub Jelinek PR rtl-optimization/114902 PR rtl-optimization/115092 * combine.cc (simplify_compare_const): Don't optimize GE op0 SIGNED_MIN or LT op0 SIGNED_MIN into NE op0 const0_rtx or EQ op0 const0_rtx. * gcc.dg/pr114902.c: New test. * gcc.dg/pr115092.c: New test. --- gcc/combine.cc.jj 2024-05-07 18:10:10.415874636 +0200 +++ gcc/combine.cc 2024-05-15 13:33:26.555081215 +0200 @@ -11852,8 +11852,10 @@ simplify_compare_const (enum rtx_code co `and'ed with that bit), we can replace this with a comparison with zero. */ if (const_op - && (code == EQ || code == NE || code == GE || code == GEU - || code == LT || code == LTU) + && (code == EQ || code == NE || code == GEU || code == LTU + /* This optimization is incorrect for signed >= INT_MIN or +< INT_MIN, those are always true or always false. */ + || ((code == GE || code == LT) && const_op > 0)) && is_a (mode, _mode) && GET_MODE_PRECISION (int_mode) - 1 < HOST_BITS_PER_WIDE_INT && pow2p_hwi (const_op & GET_MODE_MASK (int_mode)) --- gcc/testsuite/gcc.dg/pr114902.c.jj 2024-05-15 14:01:20.826717914 +0200 +++ gcc/testsuite/gcc.dg/pr114902.c 2024-05-15 14:00:39.603268571 +0200 @@ -0,0 +1,23 @@ +/* PR rtl-optimization/114902 */ +/* { dg-do run } */ +/* { dg-options "-O1 -fno-tree-fre -fno-tree-forwprop -fno-tree-ccp -fno-tree-dominator-opts" } */ + +__attribute__((noipa)) +int foo (int x) +{ + int a = ~x; + int t = a & 1; + int e = -t; + int b = e >= -1; + if (b) +return 0; + __builtin_trap (); +} + +int +main () +{ + foo (-1); + foo (0); + foo (1); +} --- gcc/testsuite/gcc.dg/pr115092.c.jj 2024-05-15 13:46:27.634649150 +0200 +++ gcc/testsuite/gcc.dg/pr115092.c 2024-05-15 13:46:12.052857268 +0200 @@ -0,0 +1,16 @@ +/* PR rtl-optimization/115092 */ +/* { dg-do run } */ +/* { dg-options "-O1 -fgcse -ftree-pre -fno-tree-dominator-opts -fno-tree-fre -fno-guess-branch-probability" } */ + +int a, b, c = 1, d, e; + +int +main () +{ + int f, g = a; + b = -2; + f = -(1 >> ((c && b) & ~a)); + if (f <= b) +d = g / e; + return 0; +} Jakub
Re: [PATCH] middle-end/111422 - wrong stack var coalescing, handle PHIs
On Wed, May 15, 2024 at 01:41:04PM +0200, Richard Biener wrote: > PR middle-end/111422 > * cfgexpand.cc (add_scope_conflicts_2): Handle PHIs > by recursing to their arguments. > --- > gcc/cfgexpand.cc | 21 + > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc > index 557cb28733b..e4d763fa998 100644 > --- a/gcc/cfgexpand.cc > +++ b/gcc/cfgexpand.cc > @@ -584,10 +584,23 @@ add_scope_conflicts_2 (tree use, bitmap work, > || INTEGRAL_TYPE_P (TREE_TYPE (use > { >gimple *g = SSA_NAME_DEF_STMT (use); > - if (is_gimple_assign (g)) > - if (tree op = gimple_assign_rhs1 (g)) > - if (TREE_CODE (op) == ADDR_EXPR) > - visit (g, TREE_OPERAND (op, 0), op, work); > + if (gassign *a = dyn_cast (g)) > + { > + if (tree op = gimple_assign_rhs1 (a)) > + if (TREE_CODE (op) == ADDR_EXPR) > + visit (a, TREE_OPERAND (op, 0), op, work); > + } > + else if (gphi *p = dyn_cast (g)) > + { > + for (unsigned i = 0; i < gimple_phi_num_args (p); ++i) > + if (TREE_CODE (use = gimple_phi_arg_def (p, i)) == SSA_NAME) > + if (gassign *a = dyn_cast (SSA_NAME_DEF_STMT (use))) > + { > + if (tree op = gimple_assign_rhs1 (a)) > + if (TREE_CODE (op) == ADDR_EXPR) > + visit (a, TREE_OPERAND (op, 0), op, work); > + } > + } Why the 2 {} pairs here? Can't it be done without them (sure, before the else if it is required)? Otherwise LGTM. Jakub
Re: [PATCHv2] Value range: Add range op for __builtin_isfinite
On Tue, May 07, 2024 at 10:37:55AM +0800, HAO CHEN GUI wrote: > The former patch adds isfinite optab for __builtin_isfinite. > https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649339.html > > Thus the builtin might not be folded at front end. The range op for > isfinite is needed for value range analysis. This patch adds them. > > Compared to last version, this version fixes a typo. > > Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no > regressions. Is it OK for the trunk? > > Thanks > Gui Haochen > > ChangeLog > Value Range: Add range op for builtin isfinite > > The former patch adds optab for builtin isfinite. Thus builtin isfinite might > not be folded at front end. So the range op for isfinite is needed for value > range analysis. This patch adds range op for builtin isfinite. > > gcc/ > * gimple-range-op.cc (class cfn_isfinite): New. > (op_cfn_finite): New variables. > (gimple_range_op_handler::maybe_builtin_call): Handle > CFN_BUILT_IN_ISFINITE. > > gcc/testsuite/ > * gcc/testsuite/gcc.dg/tree-ssa/range-isfinite.c: New test. BUILT_IN_ISFINITE is just one of many BUILT_IN_IS... builtins, would be nice to handle the others as well. E.g. isnormal/isnan/isinf, fpclassify etc. Note, the man page says for e.g. isnormal that it returns nonzero or zero, but in reality I think we implement it always inline and can check if it always returns [0,1]. Some others like isinf return [-1,1] though I think and fpclassify returns union of all the passed int values. Jakub
Re: [PATCH] [testsuite] Fix gcc.dg/pr115066.c fail on aarch64
On Tue, May 14, 2024 at 03:47:46PM +0200, Tom de Vries wrote: > On aarch64, I get this failure: > ... > FAIL: gcc.dg/pr115066.c scan-assembler \\.byte\\t0xb\\t# Define macro strx > ... > > This happens because we expect to match: > ... > .byte 0xb # Define macro strx > ... > but instead we get: > ... > .byte 0xb // Define macro strx > ... > > Fix this by not explicitly matching the comment marker. > > Tested on aarch64 and x86_64. > > gcc/testsuite/ChangeLog: > > 2024-05-14 Tom de Vries > > * gcc.dg/pr115066.c: Don't match comment marker. > --- > gcc/testsuite/gcc.dg/pr115066.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/testsuite/gcc.dg/pr115066.c b/gcc/testsuite/gcc.dg/pr115066.c > index 645757df209..a7e98500160 100644 > --- a/gcc/testsuite/gcc.dg/pr115066.c > +++ b/gcc/testsuite/gcc.dg/pr115066.c > @@ -2,7 +2,7 @@ > /* { dg-skip-if "split DWARF unsupported" { hppa*-*-hpux* powerpc*-ibm-aix* > *-*-darwin* } } */ > /* { dg-options "-gsplit-dwarf -g3 -dA -gdwarf-4" } */ > /* { dg-final { scan-assembler-times {\.section\t"?\.debug_macro} 1 } } */ > -/* { dg-final { scan-assembler-not {\.byte\t0x5\t# Define macro strp} } } */ > -/* { dg-final { scan-assembler {\.byte\t0xb\t# Define macro strx} } } */ > +/* { dg-final { scan-assembler-not {\.byte\t0x5\t.* Define macro strp} } } */ > +/* { dg-final { scan-assembler {\.byte\t0xb\t.* Define macro strx} } } */ Actually, perhaps better use [^\n\r]* instead of .* You don't want to match the comment on a different line. Jakub
Re: [PATCH] [testsuite] Fix gcc.dg/pr115066.c fail on aarch64
On Tue, May 14, 2024 at 03:47:46PM +0200, Tom de Vries wrote: > On aarch64, I get this failure: > ... > FAIL: gcc.dg/pr115066.c scan-assembler \\.byte\\t0xb\\t# Define macro strx > ... > > This happens because we expect to match: > ... > .byte 0xb # Define macro strx > ... > but instead we get: > ... > .byte 0xb // Define macro strx > ... > > Fix this by not explicitly matching the comment marker. > > Tested on aarch64 and x86_64. > > gcc/testsuite/ChangeLog: > > 2024-05-14 Tom de Vries > > * gcc.dg/pr115066.c: Don't match comment marker. > --- > gcc/testsuite/gcc.dg/pr115066.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Ok. Jakub
Re: [PATCH] [debug] Fix dwarf v4 .debug_macro.dwo
On Tue, May 14, 2024 at 01:35:30PM +0200, Tom de Vries wrote: > Consider a hello world, compiled with -gsplit-dwarf and dwarf version 4, and > -g3: > ... > $ gcc -gdwarf-4 -gsplit-dwarf /data/vries/hello.c -g3 -save-temps -dA > ... > > In section .debug_macro.dwo, we have: > ... > .Ldebug_macro0: > .value 0x4 # DWARF macro version number > .byte 0x2 # Flags: 32-bit, lineptr present > .long .Lskeleton_debug_line0 > .byte 0x3 # Start new file > .uleb128 0 # Included from line number 0 > .uleb128 0x1# file /data/vries/hello.c > .byte 0x5 # Define macro strp > .uleb128 0 # At line number 0 > .uleb128 0x1d0 # The macro: "__STDC__ 1" > ... > > Given that we use a DW_MACRO_define_strp, we'd expect 0x1d0 to be an > offset into a .debug_str.dwo section. > > But in fact, 0x1d0 is an index into the string offset table in > .debug_str_offsets.dwo: > ... > .long 0x34f0 # indexed string 0x1d0: __STDC__ 1 > ... > > Add asserts that catch this inconsistency, and fix this by using > DW_MACRO_define_strx instead. > > Tested on x86_64. > > PR debug/115066 ChangeLog entry is missing. Otherwise LGTM. Jakub
Re: [PATCH] c++: Optimize in maybe_clone_body aliases even when not at_eof [PR113208]
On Fri, May 10, 2024 at 03:59:25PM -0400, Jason Merrill wrote: > > 2024-05-09 Jakub Jelinek > > Jason Merrill > > > > PR lto/113208 > > * cp-tree.h (maybe_optimize_cdtor): Remove. > > * decl2.cc (tentative_decl_linkage): Call maybe_make_one_only > > for implicit instantiations of maybe in charge ctors/dtors > > declared inline. > > (import_export_decl): Don't call maybe_optimize_cdtor. > > (c_parse_final_cleanups): 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. > > > --- gcc/cp/optimize.cc.jj 2024-04-25 20:33:30.771858912 +0200 > > +++ gcc/cp/optimize.cc 2024-05-09 17:10:23.920478922 +0200 > > @@ -220,10 +220,8 @@ can_alias_cdtor (tree fn) > > gcc_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (fn)); > > /* 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; > > + return (DECL_WEAK (fn) ? (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)) > > +: (DECL_INTERFACE_KNOWN (fn) && !DECL_ONE_ONLY (fn))); > > Hmm, would > > (DECL_ONE_ONLY (fn) ? HAVE_COMDAT_GROUP > : (DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn))) > > make sense instead? I don't think DECL_WEAK is necessary for COMDAT. I think it isn't indeed necessary for COMDAT, although e.g. comdat_linkage will not call make_decl_one_only if !flag_weak. But I think it is absolutely required for the alias cdtor optimization in question, because otherwise it would be an ABI change. Consider older version of GCC or some other compiler emitting _ZN6vectorI12QualityValueEC1ERKS1_ and _ZN6vectorI12QualityValueEC2ERKS1_ symbols not as aliases, each in their own comdat groups, so .text._ZN6vectorI12QualityValueEC1ERKS1_ in _ZN6vectorI12QualityValueEC1ERKS1_ comdat group and .text._ZN6vectorI12QualityValueEC2ERKS1_ in _ZN6vectorI12QualityValueEC2ERKS1_ comdat group. And then comes GCC with the above patch without the DECL_WEAK check in there, and decides to use alias, so _ZN6vectorI12QualityValueEC1ERKS1_ is an alias to _ZN6vectorI12QualityValueEC2ERKS1_ and both live in .text._ZN6vectorI12QualityValueEC2ERKS1_ section in _ZN6vectorI12QualityValueEC5ERKS1_ comdat group. If you mix TUs with this, the linker can keep one of the section sets from the _ZN6vectorI12QualityValueEC1ERKS1_ and _ZN6vectorI12QualityValueEC2ERKS1_ and _ZN6vectorI12QualityValueEC5ERKS1_ comdat groups. If there is no .weak for the symbols, this will fail to link, one can emit it either the old way or the new way but never both, it is part of an ABI. While with .weak, mixing it is possible, worst case one gets some unused code in the linked binary or shared library. Of course the desirable case is that there is no mixing and there is no unused code, but if it happens, no big deal. Without .weak it is a big deal. Jakub
Re: [pushed 00/21] Various backports to gcc 13 (analyzer, jit, diagnostics)
On Thu, May 09, 2024 at 01:42:15PM -0400, David Malcolm wrote: > I've pushed the following changes to releases/gcc-13 > as r13-8741-g89feb3557a0188 through r13-8761-gb7a2697733d19a. Unfortunately many of the commits contained git commit message wording that update_git_version can't cope with. Wording like (cherry picked from commit r14-1664-gfe9771b59f576f) is wrong, (cherry picked from commit .) is reserved solely for what one gets from git cherry-pick -x (i.e. the full commit hash without anything extra). I had to ignore the following commits in the ChangeLog generation because of this: 89feb3557a018893cfe50c2e07f91559bd3cde2b ccf8d3e3d26c6ba3d5e11fffeed8d64018e9c060 e0c52905f666e3d23881f82dbf39466a24f009f4 b38472ffc1e631bd357573b44d956ce16d94e666 a0b13d0860848dd5f2876897ada1e22e4e681e91 b8c772cae97b54386f7853edf0f9897012bfa90b 810d35a7e054bcbb5b66d2e5924428e445f5fba9 0df1ee083434ac00ecb19582b1e5b25e105981b2 2c688f6afce4cbb414f5baab1199cd525f309fca 60dcb710b6b4aa22ea96abc8df6dfe9067f3d7fe 44968a0e00f656e9bb3e504bb2fa1a8282002015 Can you please add the ChangeLog entries for these by hand (commits which only touch ChangeLog files are allowed and shouldn't contain ChangeLog style entry in the commit message)? Thanks. Jakub
[PATCH] tree-ssa-math-opts: Pattern recognize yet another .ADD_OVERFLOW pattern [PR113982]
Hi! We pattern recognize already many different patterns, and closest to the requested one also yc = (type) y; zc = (type) z; x = yc + zc; w = (typeof_y) x; if (x > max) where y/z has the same unsigned type and type is a wider unsigned type and max is maximum value of the narrower unsigned type. But apparently people are creative in writing this in diffent ways, this requests yc = (type) y; zc = (type) z; x = yc + zc; w = (typeof_y) x; if (x >> narrower_type_bits) The following patch implements that. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-13 Jakub Jelinek PR middle-end/113982 * tree-ssa-math-opts.cc (arith_overflow_check_p): Also return 1 for RSHIFT_EXPR by precision of maxval if shift result is only used in a cast or comparison against zero. (match_arith_overflow): Handle the RSHIFT_EXPR use case. * gcc.dg/pr113982.c: New test. --- gcc/tree-ssa-math-opts.cc.jj2024-04-11 09:26:36.318369218 +0200 +++ gcc/tree-ssa-math-opts.cc 2024-05-10 18:17:08.795744811 +0200 @@ -3947,6 +3947,66 @@ arith_overflow_check_p (gimple *stmt, gi else return 0; + if (maxval + && ccode == RSHIFT_EXPR + && crhs1 == lhs + && TREE_CODE (crhs2) == INTEGER_CST + && wi::to_widest (crhs2) == TYPE_PRECISION (TREE_TYPE (maxval))) +{ + tree shiftlhs = gimple_assign_lhs (use_stmt); + if (!shiftlhs) + return 0; + use_operand_p use; + if (!single_imm_use (shiftlhs, , _use_stmt)) + return 0; + if (gimple_code (cur_use_stmt) == GIMPLE_COND) + { + ccode = gimple_cond_code (cur_use_stmt); + crhs1 = gimple_cond_lhs (cur_use_stmt); + crhs2 = gimple_cond_rhs (cur_use_stmt); + } + else if (is_gimple_assign (cur_use_stmt)) + { + if (gimple_assign_rhs_class (cur_use_stmt) == GIMPLE_BINARY_RHS) + { + ccode = gimple_assign_rhs_code (cur_use_stmt); + crhs1 = gimple_assign_rhs1 (cur_use_stmt); + crhs2 = gimple_assign_rhs2 (cur_use_stmt); + } + else if (gimple_assign_rhs_code (cur_use_stmt) == COND_EXPR) + { + tree cond = gimple_assign_rhs1 (cur_use_stmt); + if (COMPARISON_CLASS_P (cond)) + { + ccode = TREE_CODE (cond); + crhs1 = TREE_OPERAND (cond, 0); + crhs2 = TREE_OPERAND (cond, 1); + } + else + return 0; + } + else + { + enum tree_code sc = gimple_assign_rhs_code (cur_use_stmt); + tree castlhs = gimple_assign_lhs (cur_use_stmt); + if (!CONVERT_EXPR_CODE_P (sc) + || !castlhs + || !INTEGRAL_TYPE_P (TREE_TYPE (castlhs)) + || (TYPE_PRECISION (TREE_TYPE (castlhs)) + > TYPE_PRECISION (TREE_TYPE (maxval + return 0; + return 1; + } + } + else + return 0; + if ((ccode != EQ_EXPR && ccode != NE_EXPR) + || crhs1 != shiftlhs + || !integer_zerop (crhs2)) + return 0; + return 1; +} + if (TREE_CODE_CLASS (ccode) != tcc_comparison) return 0; @@ -4049,6 +4109,7 @@ arith_overflow_check_p (gimple *stmt, gi _8 = IMAGPART_EXPR <_7>; if (_8) and replace (utype) x with _9. + Or with x >> popcount (max) instead of x > max. Also recognize: x = ~z; @@ -4481,10 +4542,62 @@ match_arith_overflow (gimple_stmt_iterat gcc_checking_assert (is_gimple_assign (use_stmt)); if (gimple_assign_rhs_class (use_stmt) == GIMPLE_BINARY_RHS) { - gimple_assign_set_rhs1 (use_stmt, ovf); - gimple_assign_set_rhs2 (use_stmt, build_int_cst (type, 0)); - gimple_assign_set_rhs_code (use_stmt, - ovf_use == 1 ? NE_EXPR : EQ_EXPR); + if (gimple_assign_rhs_code (use_stmt) == RSHIFT_EXPR) + { + g2 = gimple_build_assign (make_ssa_name (boolean_type_node), + ovf_use == 1 ? NE_EXPR : EQ_EXPR, + ovf, build_int_cst (type, 0)); + gimple_stmt_iterator gsiu = gsi_for_stmt (use_stmt); + gsi_insert_before (, g2, GSI_SAME_STMT); + gimple_assign_set_rhs_with_ops (, NOP_EXPR, + gimple_assign_lhs (g2)); + update_stmt (use_stmt); + use_operand_p use; + single_imm_use (gimple_assign_lhs (use_stmt), , + _stmt); + if (gimple_code (use_stmt) == GIMPLE_COND) + { + gcond *cond_stmt = as_a (u
Re: [PATCH] c++: Optimize in maybe_clone_body aliases even when not at_eof [PR113208]
On Thu, May 09, 2024 at 02:58:52PM -0400, Marek Polacek wrote: > On Thu, May 09, 2024 at 08:20:00PM +0200, Jakub Jelinek wrote: > > --- gcc/cp/decl.cc.jj 2024-05-09 10:30:54.804505130 +0200 > > +++ gcc/cp/decl.cc 2024-05-09 17:07:08.400110018 +0200 > > @@ -19280,6 +19280,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; > > This checks SUPPORTS_ONE_ONLY twice. Oops, you're right, fixed in my copy. Jakub
[committed] testsuite: Fix up pr84508* tests [PR84508]
On Thu, May 09, 2024 at 12:45:42PM +0800, Hongtao Liu wrote: > > PR target/84508 > > * gcc.target/i386/pr84508-1.c: New test. > > * gcc.target/i386/pr84508-2.c: Ditto. The tests FAIL on x86_64-linux with /usr/bin/ld: cannot find -lubsan collect2: error: ld returned 1 exit status compiler exited with status 1 FAIL: gcc.target/i386/pr84508-1.c (test for excess errors) Excess errors: /usr/bin/ld: cannot find -lubsan The problem is that only *.dg/ubsan/ubsan.exp calls ubsan_init which adds the needed search paths to libubsan library. So, link/run tests for -fsanitize=undefined need to go into gcc.dg/ubsan/ or g++.dg/ubsan/, even when they are target specific. Tested on x86_64-linux with make check-gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} i386.exp=pr84508* ubsan.exp=pr84508*' and committed to trunk as obvious. 2024-05-09 Jakub Jelinek PR target/84508 * gcc.target/i386/pr84508-1.c: Move to ... * gcc.dg/ubsan/pr84508-1.c: ... here. Restrict to i?86/x86_64 non-ia32 targets. * gcc.target/i386/pr84508-2.c: Move to ... * gcc.dg/ubsan/pr84508-2.c: ... here. Restrict to i?86/x86_64 non-ia32 targets. diff --git a/gcc/testsuite/gcc.target/i386/pr84508-1.c b/gcc/testsuite/gcc.dg/ubsan/pr84508-1.c similarity index 74% rename from gcc/testsuite/gcc.target/i386/pr84508-1.c rename to gcc/testsuite/gcc.dg/ubsan/pr84508-1.c index bb3e28d017e..d781e01 100644 --- a/gcc/testsuite/gcc.target/i386/pr84508-1.c +++ b/gcc/testsuite/gcc.dg/ubsan/pr84508-1.c @@ -1,5 +1,6 @@ -/* { dg-do run { target { ! ia32 } } } */ +/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ! ia32 } } } } */ /* { dg-options "-fsanitize=undefined" } */ + #include int main() diff --git a/gcc/testsuite/gcc.target/i386/pr84508-2.c b/gcc/testsuite/gcc.dg/ubsan/pr84508-2.c similarity index 73% rename from gcc/testsuite/gcc.target/i386/pr84508-2.c rename to gcc/testsuite/gcc.dg/ubsan/pr84508-2.c index 32a8f20a536..cf9c7db1d15 100644 --- a/gcc/testsuite/gcc.target/i386/pr84508-2.c +++ b/gcc/testsuite/gcc.dg/ubsan/pr84508-2.c @@ -1,5 +1,6 @@ -/* { dg-do run { target { ! ia32 } } } */ +/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ! ia32 } } } } */ /* { dg-options "-fsanitize=undefined" } */ + #include int main() Jakub
[PATCH] c++, mingw, v2: Fix up types of dtor hooks to __cxa_{,thread_}atexit/__cxa_throw on mingw ia32 [PR114968]
On Thu, May 09, 2024 at 01:05:59PM -0400, Jason Merrill wrote: > I think I'd rather pass ob_parm to start_cleanup_fn, where it can also > replace the flag_use_cxa_atexit check in that function. Good idea, changed in the following patch. > > @@ -9998,7 +10004,8 @@ register_dtor_fn (tree decl) > > { > > /* We must convert CLEANUP to the type that "__cxa_atexit" > > expects. */ > > - cleanup = build_nop (get_atexit_fn_ptr_type (), cleanup); > > + cleanup = build_nop (ob_parm ? get_cxa_atexit_fn_ptr_type () > > + : get_atexit_fn_ptr_type (), cleanup); > > If we're (now) using the correct type to build the cleanup fn, this > conversion should be unnecessary. This is the use_dtor case, where cleanup will have METHOD_TYPE, so I think we need to cast. But, we can cast always to get_cxa_atexit_fn_ptr_type () type, because this is in use_dtor guarded code and use_dtor is ob_parm && CLASS_TYPE_P (type), so when use_dtor is true, ob_parm is also true. Ok for trunk if it passes another bootstrap/regtest? 2024-05-09 Jakub Jelinek PR target/114968 gcc/ * target.def (use_atexit_for_cxa_atexit): Remove spurious space from comment. (adjust_cdtor_callabi_fntype): New cxx target hook. * targhooks.h (default_cxx_adjust_cdtor_callabi_fntype): Declare. * targhooks.cc (default_cxx_adjust_cdtor_callabi_fntype): New function. * doc/tm.texi.in (TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Add. * doc/tm.texi: Regenerate. * config/i386/i386.cc (ix86_cxx_adjust_cdtor_callabi_fntype): New function. (TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Redefine. gcc/cp/ * cp-tree.h (atexit_fn_ptr_type_node, cleanup_type): Adjust macro comments. (get_cxa_atexit_fn_ptr_type): Declare. * decl.cc (get_atexit_fn_ptr_type): Adjust function comment, only build type for atexit argument. (get_cxa_atexit_fn_ptr_type): New function. (get_atexit_node): Call get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type when using __cxa_atexit. (get_thread_atexit_node): Call get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type. (start_cleanup_fn): Add ob_parm argument, call get_cxa_atexit_fn_ptr_type or get_atexit_fn_ptr_type depending on it and create PARM_DECL also based on that argument. (register_dtor_fn): Adjust start_cleanup_fn caller, use get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type for use_dtor casts. * except.cc (build_throw): Use get_cxa_atexit_fn_ptr_type (). --- gcc/target.def.jj 2024-05-09 10:30:54.926503473 +0200 +++ gcc/target.def 2024-05-09 20:27:16.294780780 +0200 @@ -6498,7 +6498,7 @@ is in effect. The default is to return hook_bool_void_false) /* Returns true if target may use atexit in the same manner as - __cxa_atexit to register static destructors. */ + __cxa_atexit to register static destructors. */ DEFHOOK (use_atexit_for_cxa_atexit, "This hook returns true if the target @code{atexit} function can be used\n\ @@ -6509,6 +6509,17 @@ unloaded. The default is to return false bool, (void), hook_bool_void_false) +/* Returns modified FUNCTION_TYPE for cdtor callabi. */ +DEFHOOK +(adjust_cdtor_callabi_fntype, + "This hook returns a possibly modified @code{FUNCTION_TYPE} for arguments\n\ +to @code{__cxa_atexit}, @code{__cxa_thread_atexit} or @code{__cxa_throw}\n\ +function pointers. ABIs like mingw32 require special attributes to be added\n\ +to function types pointed to by arguments of these functions.\n\ +The default is to return the passed argument unmodified.", + tree, (tree fntype), + default_cxx_adjust_cdtor_callabi_fntype) + DEFHOOK (adjust_class_at_definition, "@var{type} is a C++ class (i.e., RECORD_TYPE or UNION_TYPE) that has just\n\ --- gcc/targhooks.h.jj 2024-05-09 10:30:54.941503269 +0200 +++ gcc/targhooks.h 2024-05-09 20:27:16.315780505 +0200 @@ -65,6 +65,7 @@ extern machine_mode default_mode_for_suf extern tree default_cxx_guard_type (void); extern tree default_cxx_get_cookie_size (tree); +extern tree default_cxx_adjust_cdtor_callabi_fntype (tree); extern bool hook_pass_by_reference_must_pass_in_stack (cumulative_args_t, const function_arg_info &); --- gcc/targhooks.cc.jj 2024-05-09 10:30:54.927503459 +0200 +++ gcc/targhooks.cc2024-05-09 20:27:16.338780204 +0200 @@ -329,6 +329,14 @@ default_cxx_get_cookie_size (tree type) return cookie_size; } +/* Returns modified FUNCTION_TYPE for cdtor callabi. */ + +tree +default_cxx_adjust_cdtor_callabi_fntype (tree fntype) +{ + return fntype; +} + /* Return true if a parameter must be passed by reference. This version of the TARGET_PASS_BY_REFERENCE hook uses just MUST_PASS_IN_STACK. */ --- gcc/d
[PATCH] c++: Optimize in maybe_clone_body aliases even when not at_eof [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? > > 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. Ok, here is an updated patch, which sets DECL_SAVED_TREE to void_node for the aliases together with reversion of the earlier committed patch. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-09 Jakub Jelinek Jason Merrill PR lto/113208 * cp-tree.h (maybe_optimize_cdtor): Remove. * decl2.cc (tentative_decl_linkage): Call maybe_make_one_only for implicit instantiations of maybe in charge ctors/dtors declared inline. (import_export_decl): Don't call maybe_optimize_cdtor. (c_parse_final_cleanups): 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. (maybe_clone_body): Don't clear DECL_SAVED_TREE, instead set it to void_node. (maybe_clone_body): Remove. * 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/comdat3.C: New test. * g++.dg/abi/comdat4.C: New test. --- gcc/cp/cp-tree.h.jj 2024-05-09 10:30:54.775505524 +0200 +++ gcc/cp/cp-tree.h2024-05-09 17:07:01.246206288 +0200 @@ -7451,7 +7451,6 @@ extern bool handle_module_option (unsign /* In optimize.cc */ extern tree clone_attrs(tree); extern bool maybe_clone_body (tree); -extern void maybe_optimize_cdtor (tree); /* In parser.cc */ extern tree cp_convert_range_for (tree, tree, tree, cp_decomp *, bool, --- gcc/cp/decl2.cc.jj 2024-05-02 09:31:17.753298180 +0200 +++ gcc/cp/decl2.cc 2024-05-09 17:11:11.676836268 +0200 @@ -3325,16 +3325,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)) @@ -3604,9 +3611,6 @@ import_export_decl (tree decl) } DECL_INTERFACE_KNOWN (decl) = 1; - - if (DECL_CLONED_FUNCTION_P (decl)) -maybe_optimize_cdtor (decl); } /* Return an expression that performs the destruction of DECL, which @@ -5331,7 +5335,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. */ @@ -5341,7 +5345,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 --- gcc/cp/optimize.cc.jj 2024-04-25 20:33:30.771858912 +0200 +++ gcc/cp/optimize.cc 2024-05-09 17:10:23.9
[PATCH] c++: Fix parsing of abstract-declarator starting with ... followed by [ or ( [PR115012]
Hi! The C++26 P2662R3 Pack indexing paper mentions that both GCC and MSVC don't handle T...[10] parameter declaration when T is a pack. While that will change meaning in C++26, in C++11 .. C++23 this ought to be valid. Also, T...(args) as well. The following patch handles those in cp_parser_direct_declarator. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-09 Jakub Jelinek PR c++/115012 * parser.cc (cp_parser_direct_declarator): Handle abstract declarator starting with ... followed by [ or (. * g++.dg/cpp0x/variadic185.C: New test. * g++.dg/cpp0x/variadic186.C: New test. --- gcc/cp/parser.cc.jj 2024-05-09 10:30:58.0 +0200 +++ gcc/cp/parser.cc2024-05-09 16:44:01.250777325 +0200 @@ -23916,7 +23916,12 @@ cp_parser_direct_declarator (cp_parser* { /* Peek at the next token. */ token = cp_lexer_peek_token (parser->lexer); - if (token->type == CPP_OPEN_PAREN) + if (token->type == CPP_OPEN_PAREN + || (first + && dcl_kind != CP_PARSER_DECLARATOR_NAMED + && token->type == CPP_ELLIPSIS + && cxx_dialect > cxx98 + && cp_lexer_nth_token_is (parser->lexer, 2, CPP_OPEN_PAREN))) { /* This is either a parameter-declaration-clause, or a parenthesized declarator. When we know we are parsing a @@ -23955,6 +23960,11 @@ cp_parser_direct_declarator (cp_parser* Thus again, we try a parameter-declaration-clause, and if that fails, we back out and return. */ + bool pack_expansion_p = token->type == CPP_ELLIPSIS; + + if (pack_expansion_p) + /* Consume the `...' */ + cp_lexer_consume_token (parser->lexer); if (!first || dcl_kind != CP_PARSER_DECLARATOR_NAMED) { @@ -24098,6 +24108,7 @@ cp_parser_direct_declarator (cp_parser* attrs, parens_loc); declarator->attributes = gnu_attrs; + declarator->parameter_pack_p |= pack_expansion_p; /* Any subsequent parameter lists are to do with return type, so are not those of the declared function. */ @@ -24121,7 +24132,7 @@ cp_parser_direct_declarator (cp_parser* /* If this is the first, we can try a parenthesized declarator. */ - if (first) + if (first && !pack_expansion_p) { bool saved_in_type_id_in_expr_p; @@ -24156,16 +24167,27 @@ cp_parser_direct_declarator (cp_parser* else break; } - else if ((!first || dcl_kind != CP_PARSER_DECLARATOR_NAMED) - && token->type == CPP_OPEN_SQUARE - && !cp_next_tokens_can_be_attribute_p (parser)) + else if (((!first || dcl_kind != CP_PARSER_DECLARATOR_NAMED) + && token->type == CPP_OPEN_SQUARE + && !cp_next_tokens_can_be_attribute_p (parser)) + || (first + && dcl_kind != CP_PARSER_DECLARATOR_NAMED + && token->type == CPP_ELLIPSIS + && cp_lexer_nth_token_is (parser->lexer, 2, CPP_OPEN_SQUARE) + && cxx_dialect > cxx98 + && !cp_nth_tokens_can_be_std_attribute_p (parser, 2))) { /* Parse an array-declarator. */ tree bounds, attrs; + bool pack_expansion_p = token->type == CPP_ELLIPSIS; if (ctor_dtor_or_conv_p) *ctor_dtor_or_conv_p = 0; + if (pack_expansion_p) + /* Consume the `...' */ + cp_lexer_consume_token (parser->lexer); + open_paren = NULL; first = false; parser->default_arg_ok_p = false; @@ -24220,6 +24242,7 @@ cp_parser_direct_declarator (cp_parser* attrs = cp_parser_std_attribute_spec_seq (parser); declarator = make_array_declarator (declarator, bounds); declarator->std_attributes = attrs; + declarator->parameter_pack_p |= pack_expansion_p; } else if (first && dcl_kind != CP_PARSER_DECLARATOR_ABSTRACT) { --- gcc/testsuite/g++.dg/cpp0x/variadic185.C.jj 2024-05-09 15:08:49.070651189 +0200 +++ gcc/testsuite/g++.dg/cpp0x/variadic185.C2024-05-09 15:07:40.045583153 +0200 @@ -0,0 +1,39 @@ +// PR c++/115012 +// { dg-do compile { target { c++11 && c++23_down } } } +// { dg-final { scan-assembler "_Z3fooILi10EJidEEvDpAT__T0_" } } +// { dg-final { scan-assembler "_Z3barILi10EiEvPT0_" } } +// { dg-final { scan-assembler "_Z3bazILi10EJidEEvDpAT__T0_" } } +// { dg-final { scan-assembler "_Z3quxILi5EJifEEvDpAT__AT_
Re: gcc/DATESTAMP wasn't updated since 20240507
On Thu, May 09, 2024 at 12:14:43PM +0200, Jakub Jelinek wrote: > On Thu, May 09, 2024 at 12:04:38PM +0200, Rainer Orth wrote: > > I just noticed that gcc/DATESTAMP wasn't updated yesterday and today, > > staying at 20240507. > > I think it is because of the r15-268 commit, we do support > This reverts commit ... > when the referenced commit contains a ChangeLog message, but here > it doesn't, as it is a revert commit. Indeed and also the r15-311 commit. Please don't Revert Revert, we don't really support that, had to fix it all by hand. Jakub
Re: gcc/DATESTAMP wasn't updated since 20240507
On Thu, May 09, 2024 at 12:04:38PM +0200, Rainer Orth wrote: > I just noticed that gcc/DATESTAMP wasn't updated yesterday and today, > staying at 20240507. I think it is because of the r15-268 commit, we do support This reverts commit ... when the referenced commit contains a ChangeLog message, but here it doesn't, as it is a revert commit. Jakub
[committed] testsuite: Fix up vector-subaccess-1.C test for ia32 [PR89224]
Hi! The test FAILs on i686-linux due to .../gcc/testsuite/g++.dg/torture/vector-subaccess-1.C:16:6: warning: SSE vector argument without SSE enabled changes the ABI [-Wpsabi] excess warnings. This fixes it by adding -Wno-psabi, like commonly done in other tests. Committed to trunk as obvious after testing it on x86_64-linux with make check-g++ RUNTESTFLAGS='--target_board=unix\{-m32/-mno-sse,-m32,-m64\} dg-torture.exp=vector-subaccess-1.C' and backported all the way to 11 branch. 2024-05-09 Jakub Jelinek PR c++/89224 * g++.dg/torture/vector-subaccess-1.C: Add -Wno-psabi as additional options. --- gcc/testsuite/g++.dg/torture/vector-subaccess-1.C.jj2024-05-08 10:16:54.045823642 +0200 +++ gcc/testsuite/g++.dg/torture/vector-subaccess-1.C 2024-05-09 11:16:46.730114871 +0200 @@ -1,4 +1,5 @@ /* PR c++/89224 */ +/* { dg-additional-options "-Wno-psabi" } */ /* The access of `vector[i]` has the same qualifiers as the original vector which was missing. */ Jakub
[PATCH] c++, mingw: Fix up types of dtor hooks to __cxa_{,thread_}atexit/__cxa_throw on mingw ia32 [PR114968]
Hi! __cxa_atexit/__cxa_thread_atexit/__cxa_throw functions accept function pointers to usually directly destructors rather than wrappers around them. Now, mingw ia32 uses implicitly __attribute__((thiscall)) calling conventions for METHOD_TYPE (where the this pointer is passed in %ecx register, the rest on the stack), so these functions use: in config/os/mingw32/os_defines.h: #if defined (__i386__) #define _GLIBCXX_CDTOR_CALLABI __thiscall #endif in libsupc++/cxxabi.h __cxa_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void*) _GLIBCXX_NOTHROW; __cxa_thread_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void *) _GLIBCXX_NOTHROW; __cxa_throw(void*, std::type_info*, void (_GLIBCXX_CDTOR_CALLABI *) (void *)) __attribute__((__noreturn__)); Now, mingw for some weird reason uses #define TARGET_CXX_USE_ATEXIT_FOR_CXA_ATEXIT hook_bool_void_true so it never actually uses __cxa_atexit, but does use __cxa_thread_atexit and __cxa_throw. Recent changes for modules result in more detailed __cxa_*atexit/__cxa_throw prototypes precreated by the compiler, and if that happens and one also includes , the compiler complains about mismatches in the prototypes. One thing is the missing thiscall attribute on the FUNCTION_TYPE, the other problem is that all of atexit/__cxa_atexit/__cxa_thread_atexit get function pointer types gets created by a single function, get_atexit_fn_ptr_type (), which creates it depending on if atexit or __cxa_atexit will be used as either void(*)(void) or void(*)(void *), but when using atexit and __cxa_thread_atexit it uses the wrong function type for __cxa_thread_atexit. The following patch adds a target hook to add the thiscall attribute to the function pointers, and splits the get_atexit_fn_ptr_type () function into get_atexit_fn_ptr_type () and get_cxa_atexit_fn_ptr_type (), the former always creates shared void(*)(void) type, the latter creates either void(*)(void*) (on most targets) or void(__attribute__((thiscall))*)(void*) (on mingw ia32). So that we don't waiste another GTY global tree for it, because cleanup_type used for the same purpose for __cxa_throw should be the same, the code changes it to use that type too. In register_dtor_fn then based on the decision whether to use atexit, __cxa_atexit or __cxa_thread_atexit it picks the right function pointer type, and also if it decides to emit a __tcf_* wrapper for the cleanup, uses that type for that wrapper so that it agrees on calling convention. Bootstrapped/regtested on x86_64-linux and i686-linux and Liu Hao tested it on mingw32, ok for trunk? 2024-05-09 Jakub Jelinek PR target/114968 gcc/ * target.def (use_atexit_for_cxa_atexit): Remove spurious space from comment. (adjust_cdtor_callabi_fntype): New cxx target hook. * targhooks.h (default_cxx_adjust_cdtor_callabi_fntype): Declare. * targhooks.cc (default_cxx_adjust_cdtor_callabi_fntype): New function. * doc/tm.texi.in (TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Add. * doc/tm.texi: Regenerate. * config/i386/i386.cc (ix86_cxx_adjust_cdtor_callabi_fntype): New function. (TARGET_CXX_ADJUST_CDTOR_CALLABI_FNTYPE): Redefine. gcc/cp/ * cp-tree.h (atexit_fn_ptr_type_node, cleanup_type): Adjust macro comments. (get_cxa_atexit_fn_ptr_type): Declare. * decl.cc (get_atexit_fn_ptr_type): Adjust function comment, only build type for atexit argument. (get_cxa_atexit_fn_ptr_type): New function. (get_atexit_node): Call get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type when using __cxa_atexit. (get_thread_atexit_node): Call get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type. (start_cleanup_fn): Add fntype argument, don't call get_atexit_fn_ptr_type for it. (register_dtor_fn): Adjust start_cleanup_fn caller, use get_cxa_atexit_fn_ptr_type rather than get_atexit_fn_ptr_type when ob_parm is true. * except.cc (build_throw): Use get_cxa_atexit_fn_ptr_type (). --- gcc/target.def.jj 2024-05-07 21:28:46.554394913 +0200 +++ gcc/target.def 2024-05-08 11:19:39.290798568 +0200 @@ -6498,7 +6498,7 @@ is in effect. The default is to return hook_bool_void_false) /* Returns true if target may use atexit in the same manner as - __cxa_atexit to register static destructors. */ + __cxa_atexit to register static destructors. */ DEFHOOK (use_atexit_for_cxa_atexit, "This hook returns true if the target @code{atexit} function can be used\n\ @@ -6509,6 +6509,17 @@ unloaded. The default is to return false bool, (void), hook_bool_void_false) +/* Returns modified FUNCTION_TYPE for cdtor callabi. */ +DEFHOOK +(adjust_cdtor_callabi_fntype, + "This hook returns a possibly modified @code{FUNCTION_TYPE} for arguments\n\ +to @code{__cxa_atexit}, @code{__cxa_thread_atexit} or @code{__cxa_throw}\n\ +function pointers. ABIs li
[PATCH] reassoc: Fix up optimize_range_tests_to_bit_test [PR114965]
Hi! The optimize_range_tests_to_bit_test optimization normally emits a range test first: if (entry_test_needed) { tem = build_range_check (loc, optype, unshare_expr (exp), false, lowi, high); if (tem == NULL_TREE || is_gimple_val (tem)) continue; } so during the bit test we already know that exp is in the [lowi, high] range, but skips it if we have range info which tells us this isn't necessary. Also, normally it emits shifts by exp - lowi counter, but has an optimization to use just exp counter if the mask isn't a more expensive constant in that case and lowi is > 0 and high is smaller than prec. The following testcase is miscompiled because the two abnormal cases are triggered. The range of exp is [43, 43][48, 48][95, 95], so we on 64-bit arch decide we don't need the entry test, because 95 - 43 < 64. And we also decide to use just exp as counter, because the range test tests just for exp == 43 || exp == 48, so high is smaller than 64 too. Because 95 is in the exp range, we can't do that, we'd either need to do a range test first, i.e. if (exp - 43U <= 48U - 43U) if ((1UL << exp) & mask1)) or need to subtract lowi from the shift counter, i.e. if ((1UL << (exp - 43)) & mask2) but can't do both unless r.upper_bound () is < prec. The following patch ensures that. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-08 Jakub Jelinek PR tree-optimization/114965 * tree-ssa-reassoc.cc (optimize_range_tests_to_bit_test): Don't try to optimize away exp - lowi subtraction from shift count unless entry test is emitted or unless r.upper_bound () is smaller than prec. * gcc.c-torture/execute/pr114965.c: New test. --- gcc/tree-ssa-reassoc.cc.jj 2024-01-12 10:07:58.384848977 +0100 +++ gcc/tree-ssa-reassoc.cc 2024-05-07 18:18:45.558814991 +0200 @@ -3418,7 +3418,8 @@ optimize_range_tests_to_bit_test (enum t We can avoid then subtraction of the minimum value, but the mask constant could be perhaps more expensive. */ if (compare_tree_int (lowi, 0) > 0 - && compare_tree_int (high, prec) < 0) + && compare_tree_int (high, prec) < 0 + && (entry_test_needed || wi::ltu_p (r.upper_bound (), prec))) { int cost_diff; HOST_WIDE_INT m = tree_to_uhwi (lowi); --- gcc/testsuite/gcc.c-torture/execute/pr114965.c.jj 2024-05-07 18:17:16.767031821 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr114965.c 2024-05-07 18:15:52.332188943 +0200 @@ -0,0 +1,30 @@ +/* PR tree-optimization/114965 */ + +static void +foo (const char *x) +{ + + char a = '0'; + while (1) +{ + switch (*x) + { + case '_': + case '+': + a = *x; + x++; + continue; + default: + break; + } + break; +} + if (a == '0' || a == '+') +__builtin_abort (); +} + +int +main () +{ + foo ("_"); +} Jakub
Re: [PATCH] expansion: Use __trunchfbf2 calls rather than __extendhfbf2 [PR114907]
On Tue, May 07, 2024 at 08:57:00PM +0200, Richard Biener wrote: > > > > Am 07.05.2024 um 18:02 schrieb Jakub Jelinek : > > > > Hi! > > > > The HF and BF modes have the same size/precision and neither is > > a subset nor superset of the other. > > So, using either __extendhfbf2 or __trunchfbf2 is weird. > > The expansion apparently emits __extendhfbf2, but on the libgcc side > > we apparently have __trunchfbf2 implemented. > > > > I think it is easier to switch to using what is available rather than > > adding new entrypoints to libgcc, even alias, because this is backportable. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > Ok - do we have any target patterns that need adjustments? I don't think so. BFmode is i386/aarch64/arm/riscv backend only from what I can see, I've done make mddump for all of them and none of the tmp-mddump.md files show any matches for hfbf (nor bfhf). Jakub
[PATCH] expansion: Use __trunchfbf2 calls rather than __extendhfbf2 [PR114907]
Hi! The HF and BF modes have the same size/precision and neither is a subset nor superset of the other. So, using either __extendhfbf2 or __trunchfbf2 is weird. The expansion apparently emits __extendhfbf2, but on the libgcc side we apparently have __trunchfbf2 implemented. I think it is easier to switch to using what is available rather than adding new entrypoints to libgcc, even alias, because this is backportable. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-07 Jakub Jelinek PR middle-end/114907 * expr.cc (convert_mode_scalar): Use trunc_optab rather than sext_optab for HF->BF conversions. * optabs-libfuncs.cc (gen_trunc_conv_libfunc): Likewise. * gcc.dg/pr114907.c: New test. --- gcc/expr.cc.jj 2024-04-09 09:29:04.0 +0200 +++ gcc/expr.cc 2024-05-06 13:21:33.933798494 +0200 @@ -355,8 +355,16 @@ convert_mode_scalar (rtx to, rtx from, i && REAL_MODE_FORMAT (from_mode) == _half_format)); if (GET_MODE_PRECISION (from_mode) == GET_MODE_PRECISION (to_mode)) - /* Conversion between decimal float and binary float, same size. */ - tab = DECIMAL_FLOAT_MODE_P (from_mode) ? trunc_optab : sext_optab; + { + if (REAL_MODE_FORMAT (to_mode) == _bfloat_half_format + && REAL_MODE_FORMAT (from_mode) == _half_format) + /* libgcc implements just __trunchfbf2, not __extendhfbf2. */ + tab = trunc_optab; + else + /* Conversion between decimal float and binary float, same + size. */ + tab = DECIMAL_FLOAT_MODE_P (from_mode) ? trunc_optab : sext_optab; + } else if (GET_MODE_PRECISION (from_mode) < GET_MODE_PRECISION (to_mode)) tab = sext_optab; else --- gcc/optabs-libfuncs.cc.jj 2024-01-03 11:51:31.739728303 +0100 +++ gcc/optabs-libfuncs.cc 2024-05-06 15:50:21.611027802 +0200 @@ -589,7 +589,9 @@ gen_trunc_conv_libfunc (convert_optab ta if (GET_MODE_CLASS (float_tmode) != GET_MODE_CLASS (float_fmode)) gen_interclass_conv_libfunc (tab, opname, float_tmode, float_fmode); - if (GET_MODE_PRECISION (float_fmode) <= GET_MODE_PRECISION (float_tmode)) + if (GET_MODE_PRECISION (float_fmode) <= GET_MODE_PRECISION (float_tmode) + && (REAL_MODE_FORMAT (float_tmode) != _bfloat_half_format + || REAL_MODE_FORMAT (float_fmode) != _half_format)) return; if (GET_MODE_CLASS (float_tmode) == GET_MODE_CLASS (float_fmode)) --- gcc/testsuite/gcc.dg/pr114907.c.jj 2024-05-06 15:59:08.734958523 +0200 +++ gcc/testsuite/gcc.dg/pr114907.c 2024-05-06 16:02:38.914139829 +0200 @@ -0,0 +1,27 @@ +/* PR middle-end/114907 */ +/* { dg-do run } */ +/* { dg-options "" } */ +/* { dg-add-options float16 } */ +/* { dg-require-effective-target float16_runtime } */ +/* { dg-add-options bfloat16 } */ +/* { dg-require-effective-target bfloat16_runtime } */ + +__attribute__((noipa)) _Float16 +foo (__bf16 x) +{ + return (_Float16) x; +} + +__attribute__((noipa)) __bf16 +bar (_Float16 x) +{ + return (__bf16) x; +} + +int +main () +{ + if (foo (11.125bf16) != 11.125f16 + || bar (11.125f16) != 11.125bf16) +__builtin_abort (); +} Jakub
[PATCH] tree-inline: Remove .ASAN_MARK calls when inlining functions into no_sanitize callers [PR114956]
Hi! In r9-5742 we've started allowing to inline always_inline functions into functions which have disabled e.g. address sanitization even when the always_inline function is implicitly from command line options sanitized. This mostly works fine because most of the asan instrumentation is done only late after ipa, but as the following testcase the .ASAN_MARK ifn calls gimplifier adds can result in ICEs. Fixed by dropping those during inlining, similarly to how we drop .TSAN_FUNC_EXIT calls. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-07 Jakub Jelinek PR sanitizer/114956 * tree-inline.cc: Include asan.h. (copy_bb): Remove also .ASAN_MARK calls if id->dst_fn has asan/hwasan sanitization disabled. * gcc.dg/asan/pr114956.c: New test. --- gcc/tree-inline.cc.jj 2024-05-03 09:44:21.199055899 +0200 +++ gcc/tree-inline.cc 2024-05-06 10:45:37.231349328 +0200 @@ -65,6 +65,7 @@ along with GCC; see the file COPYING3. #include "symbol-summary.h" #include "symtab-thunks.h" #include "symtab-clones.h" +#include "asan.h" /* I'm not real happy about this, but we need to handle gimple and non-gimple trees. */ @@ -2226,13 +2227,26 @@ copy_bb (copy_body_data *id, basic_block } else if (call_stmt && id->call_stmt - && gimple_call_internal_p (stmt) - && gimple_call_internal_fn (stmt) == IFN_TSAN_FUNC_EXIT) - { - /* Drop TSAN_FUNC_EXIT () internal calls during inlining. */ - gsi_remove (_gsi, false); - continue; - } + && gimple_call_internal_p (stmt)) + switch (gimple_call_internal_fn (stmt)) + { + case IFN_TSAN_FUNC_EXIT: + /* Drop .TSAN_FUNC_EXIT () internal calls during inlining. */ + gsi_remove (_gsi, false); + continue; + case IFN_ASAN_MARK: + /* Drop .ASAN_MARK internal calls during inlining into + no_sanitize functions. */ + if (!sanitize_flags_p (SANITIZE_ADDRESS, id->dst_fn) + && !sanitize_flags_p (SANITIZE_HWADDRESS, id->dst_fn)) + { + gsi_remove (_gsi, false); + continue; + } + break; + default: + break; + } /* Statements produced by inlining can be unfolded, especially when we constant propagated some operands. We can't fold --- gcc/testsuite/gcc.dg/asan/pr114956.c.jj 2024-05-06 10:54:52.601892840 +0200 +++ gcc/testsuite/gcc.dg/asan/pr114956.c2024-05-06 10:54:33.920143734 +0200 @@ -0,0 +1,26 @@ +/* PR sanitizer/114956 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fsanitize=address,null" } */ + +int **a; +void qux (int *); + +__attribute__((always_inline)) static inline int * +foo (void) +{ + int b[1]; + qux (b); + return a[1]; +} + +__attribute__((no_sanitize_address)) void +bar (void) +{ + *a = foo (); +} + +void +baz (void) +{ + bar (); +} Jakub
Re: [wwwdocs] Specify AArch64 BitInt support for little-endian only
On Tue, May 07, 2024 at 02:12:07PM +0100, Andre Vieira (lists) wrote: > Hey Jakub, > > This what ya had in mind? Yes. > diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html > index > ca5174de991bb088f653468f77485c15a61526e6..924e045a15a78b5702a0d6997953f35c6b47efd1 > 100644 > --- a/htdocs/gcc-14/changes.html > +++ b/htdocs/gcc-14/changes.html > @@ -325,7 +325,7 @@ You may also want to check out our >Bit-precise integer types (_BitInt (N) >and unsigned _BitInt (N)): integer types with >a specified number of bits. These are only supported on > - IA-32, x86-64 and AArch64 at present. > + IA-32, x86-64 and AArch64 (little-endian) at present. >Structure, union and enumeration types may be defined more >than once in the same scope with the same contents and the same >tag; if such types are defined with the same contents and the Jakub
Re: [PATCH] middle-end/114931 - type_hash_canon and structual equality types
On Fri, May 03, 2024 at 09:11:20PM +0200, Martin Uecker wrote: > > TYPE_CANONICAL as used by the middle-end cannot express this but > > Hm. so how does it work now for arrays? Do you have a testcase which doesn't work correctly with the arrays? E.g. same_type_for_tbaa has type1 = TYPE_MAIN_VARIANT (type1); type2 = TYPE_MAIN_VARIANT (type2); /* Handle the most common case first. */ if (type1 == type2) return 1; /* If we would have to do structural comparison bail out. */ if (TYPE_STRUCTURAL_EQUALITY_P (type1) || TYPE_STRUCTURAL_EQUALITY_P (type2)) return -1; /* Compare the canonical types. */ if (TYPE_CANONICAL (type1) == TYPE_CANONICAL (type2)) return 1; /* ??? Array types are not properly unified in all cases as we have spurious changes in the index types for example. Removing this causes all sorts of problems with the Fortran frontend. */ if (TREE_CODE (type1) == ARRAY_TYPE && TREE_CODE (type2) == ARRAY_TYPE) return -1; ... and later compares alias sets and the like. So, even if int[] and int[0] have different TYPE_CANONICAL, they will be considered maybe the same. Also, guess get_alias_set has some ARRAY_TYPE handling... Anyway, I think we should just go with Richi's patch. Jakub
Re: [PATCH] middle-end/114931 - type_hash_canon and structual equality types
On Fri, May 03, 2024 at 08:04:18PM +0200, Martin Uecker wrote: > A change that is not optimal but would avoid a lot of trouble is to > only use the tag of the struct for computing a TYPE_CANONICAL, which > could then be set already for incomplete types and never needs to > change again. We would not differentiate between different struct > types with the same tag for aliasing analysis, but in most cases > I would expect different structs to have a different tag. Having incompatible types have the same TYPE_CANONICAL would lead to wrong code IMHO, while for aliasing purposes that might be conservative (though not sure, the alias set computation is based on what types the element have etc., so if the alias set is computed for say struct S { int s; }; and then the same alias set used for struct S { long long a; double b; union { short c; float d; } c; };, I think nothing good will come out of that), but middle-end also uses TYPE_CANONICAL to see if types are the same, say e.g. useless_type_conversion_p says that conversions from one RECORD_TYPE to a different RECORD_TYPE are useless if they have the same TYPE_CANONICAL. /* For aggregates we rely on TYPE_CANONICAL exclusively and require explicit conversions for types involving to be structurally compared types. */ else if (AGGREGATE_TYPE_P (inner_type) && TREE_CODE (inner_type) == TREE_CODE (outer_type)) return TYPE_CANONICAL (inner_type) && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type); So, if you have struct S { int s; } and struct S { short a, b; }; and VIEW_CONVERT_EXPR between them, that VIEW_CONVERT_EXPR will be removed as useless, etc. BTW, the idea of lazily updating TYPE_CANONICAL is basically what I've described as the option to update all the derived types where it would pretty much do that for all TYPE_STRUCTURAL_EQUALITY_P types in the hash table (see if they are derived from the type in question and recompute the TYPE_CANONICAL after recomputing all the TYPE_CANONICAL of its base types), except perhaps even more costly (if the trigger would be some build_array_type/build_function_type/... function is called and found a cached TYPE_STRUCTURAL_EQUALITY_P type). Note also that TYPE_STRUCTURAL_EQUALITY_P isn't the case just for the C23 types which are marked that way when incomplete and later completed, but by various other cases for types which will be permanently like that, so doing expensive checks each time some build*_type* is called that refers to those would be expensive. Jakub
Re: [PATCH] middle-end/114931 - type_hash_canon and structual equality types
On Fri, May 03, 2024 at 05:32:12PM +0200, Martin Uecker wrote: > Am Freitag, dem 03.05.2024 um 14:13 +0200 schrieb Richard Biener: > > TYPE_STRUCTURAL_EQUALITY_P is part of our type system so we have > > to make sure to include that into the type unification done via > > type_hash_canon. This requires the flag to be set before querying > > the hash which is the biggest part of the patch. > > I assume this does not affect structs / unions because they > do not make this mechanism of type unification (each tagged type > is a unique type), but only derived types that end up having > TYPE_STRUCTURAL_EQUALITY_P because they are constructed from > incomplete structs / unions before TYPE_CANONICAL is set. > > I do not yet understand why this change is needed. Type > identity should not be affected by setting TYPE_CANONICAL, so > why do we need to keep such types separate? I understand that we > created some inconsistencies, but I do not see why this change > is needed to fix it. But I also haven't understood how we ended > up with a TYPE_CANONICAL having TYPE_STRUCTURAL_EQUALITY_P in > PR 114931 ... So, the C23 situation before the r14-10045 change (not counting the r14-9763 change that was later reverted) was that sometimes TYPE_CANONICAL of a RECORD_TYPE/UNION_TYPE could change from self to a different RECORD_TYPE/UNION_TYPE and we didn't bother to adjust derived types. That was really dangerous, I think e.g. type alias set wasn't recomputed. r14-10045 changed it to the non-ideal, but perhaps less wrong model, where we start with TYPE_STRUCTURAL_EQUALITY_P on incomplete types in C23 and perhaps later on change them to !TYPE_STRUCTURAL_EQUALITY_P when the type is completed, and adjust TYPE_CANONICAL of some easily discoverable derived types but certainly not all. Still, that change introduces something novel to the type system, namely that TYPE_CANONICAL can change on a type, even when it is just limited to the TYPE_STRUCTURAL_EQUALITY_P -> !TYPE_STRUCTURAL_EQUALITY_P kind of change and we never change one non-NULL TYPE_CANONICAL to a different one (ok, not counting the short lived TYPE_CANONICAL being initially set to self during make_node and then quickly adjusted in the callers). One question is, could we for C23 somehow limit this for the most common case where people just forward declare some aggregate type and then soon after define it? But guess the problematic counterexample there is struct S; // forward declare struct S *p; // create some derived types from it void foo (void) { struct S { int s; }; // define the type in a different scope // (perhaps with a forward declaration as well) struct S s; use (); // create derived types } struct S { int s; };// define the type in the global scope to something // that matches previously defined struct S in // another scope So e.g. noting in the hash table that a particular type has been forward declared so far and using TYPE_STRUCTURAL_EQUALITY_P only if it has been forward declared in some other scope wouldn't work. Another question is whether c_update_type_canonical can or should try to update TYPE_ALIAS_SET if TYPE_ALIAS_SET_KNOWN_P. Or do we never cache alias sets for TYPE_STRUCTURAL_EQUALITY_P types? Anyway, the ICE on the testcase is because alias.cc asserts that a !TYPE_STRUCTURAL_EQUALITY_P (type) has !TYPE_STRUCTURAL_EQUALITY_P (TYPE_CANONICAL (type)). The possibilities to resolve that are either at c_update_type_canonical time try to find all the derived types rather than just some and recompute their TYPE_CANONICAL. Guess we could e.g. just traverse the whole type_hash_table hash table and for each type see if it is in any way related to the type that is being changed and then recompute them. Though, especially FUNCTION_TYPEs make that really ugly and furthermore it needs to be recomputed in the right order, basically in the derivation order. Without doing that, we'll have some TYPE_STRUCTURAL_EQUALITY_P derived types in the type_hash_table hash table; that is conservatively correct, but can result in worse code generation because of using alias set 0. Another possibility is what Richi posted, essentially stop reusing derived types created from the time when the base type was incomplete when asking for a new derived type. We'll get the TYPE_STRUCTURAL_EQUALITY_P derived types if they were created before the base type was completed when used directly (e.g. when it is a TREE_TYPE of some decl etc.), but when we ask for a new type we'd disregard the old type and create a new one. I'm not sure the patch is complete for that, because it doesn't adjust check_base_type, build_pointer_type_for_mode, build_reference_type_for_mode which don't use type_hash_canon but walk TYPE_NEXT_VARIANT list or TYPE_POINTER_TO or TYPE_REFERENCE_TO chains. Though, maybe it is ok as is when c_update_type_canonical adjusts the pointer types and variant types, those
[PATCH] c++: Implement C++26 P2893R3 - Variadic friends [PR114459]
Hi! The following patch imeplements the C++26 P2893R3 - Variadic friends paper. The paper allows for the friend type declarations to specify more than one friend type specifier and allows to specify ... at the end of each. The patch doesn't introduce tentative parsing of friend-type-declaration non-terminal, but rather just extends existing parsing where it is a friend declaration which ends with ; after the declaration specifiers to the cases where it ends with ...; or , or ..., In that case it pedwarns for cxx_dialect < cxx26, handles the ... and if there is , continues in a loop to parse the further friend type specifiers. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-03 Jakub Jelinek PR c++/114459 gcc/c-family/ * c-cppbuiltin.cc (c_cpp_builtins): Predefine __cpp_variadic_friend=202403L for C++26. gcc/cp/ * parser.cc (cp_parser_member_declaration): Implement C++26 P2893R3 - Variadic friends. Parse friend type declarations with ... or with more than one friend type specifier. * friend.cc (make_friend_class): Allow TYPE_PACK_EXPANSION. * pt.cc (instantiate_class_template): Handle PACK_EXPANSION_P in friend classes. gcc/testsuite/ * g++.dg/cpp26/feat-cxx26.C (__cpp_variadic_friend): Add test. * g++.dg/cpp26/variadic-friend1.C: New test. --- gcc/c-family/c-cppbuiltin.cc.jj 2024-05-02 09:31:17.746298275 +0200 +++ gcc/c-family/c-cppbuiltin.cc2024-05-03 14:50:08.008242950 +0200 @@ -1093,6 +1093,7 @@ c_cpp_builtins (cpp_reader *pfile) cpp_define (pfile, "__cpp_placeholder_variables=202306L"); cpp_define (pfile, "__cpp_structured_bindings=202403L"); cpp_define (pfile, "__cpp_deleted_function=202403L"); + cpp_define (pfile, "__cpp_variadic_friend=202403L"); } if (flag_concepts) { --- gcc/cp/parser.cc.jj 2024-05-03 09:43:47.781511477 +0200 +++ gcc/cp/parser.cc2024-05-03 13:26:38.208088017 +0200 @@ -28102,7 +28102,14 @@ cp_parser_member_declaration (cp_parser* goto out; /* If there is no declarator, then the decl-specifier-seq should specify a type. */ - if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)) + if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON) + || (cp_parser_friend_p (_specifiers) + && cxx_dialect >= cxx11 + && (cp_lexer_next_token_is (parser->lexer, CPP_COMMA) + || (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS) + && (cp_lexer_nth_token_is (parser->lexer, 2, CPP_SEMICOLON) + || cp_lexer_nth_token_is (parser->lexer, 2, + CPP_COMMA)) { /* If there was no decl-specifier-seq, and the next token is a `;', then we have something like: @@ -28137,44 +28144,81 @@ cp_parser_member_declaration (cp_parser* { /* If the `friend' keyword was present, the friend must be introduced with a class-key. */ - if (!declares_class_or_enum && cxx_dialect < cxx11) -pedwarn (decl_spec_token_start->location, OPT_Wpedantic, - "in C++03 a class-key must be used " - "when declaring a friend"); - /* In this case: + if (!declares_class_or_enum && cxx_dialect < cxx11) + pedwarn (decl_spec_token_start->location, OPT_Wpedantic, +"in C++03 a class-key must be used " +"when declaring a friend"); + if (!cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON) + && cxx_dialect < cxx26) + pedwarn (cp_lexer_peek_token (parser->lexer)->location, +OPT_Wc__26_extensions, +"variadic friends or friend type declarations with " +"multiple types only available with " +"%<-std=c++2c%> or %<-std=gnu++2c%>"); + location_t friend_loc = decl_specifiers.locations[ds_friend]; + do + { + /* In this case: - template struct A { - friend struct A::B; - }; +template struct A { + friend struct A::B; +}; - A::B will be represented by a TYPENAME_TYPE, and - therefore not recognized by check_tag_decl. */ - if (!type) -{ - type = decl_specifiers.type; - if (type && TREE_CODE (type) == TYPE_DECL) -type = TREE_TYPE (type); -
Re: [PATCH v2] gcc-14: Mention that some warnings are now errors
On Fri, May 03, 2024 at 04:06:28PM +0100, Jonathan Wakely wrote: > I agree it should be mentioned, but I would put it in the caveats > section at the top, not as the last item of the C section. > > How about this? OK for wwwdocs? LGTM. > commit fe5fd75ea5a7a08eee0831cadbdd05689e9408db > Author: Jonathan Wakely > Date: Fri May 3 16:04:49 2024 +0100 > > Add caveat to GCC 14 release notes about C warnings-as-errors change > > diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html > index 46a0266d..82906de1 100644 > --- a/htdocs/gcc-14/changes.html > +++ b/htdocs/gcc-14/changes.html > @@ -40,6 +40,11 @@ a work-in-progress. > href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end;>-Wflex-array-member-not-at-end > to >identify all such cases in the source code and modify them. > > + C: > + Certain warnings about are now errors, see > + Porting to GCC 14 > + for details. > + > href="https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html;>-fcf-protection=[full|branch|return|none|check] >is refactored, to override -fcf-protection, >-fcf-protection=none needs to be added and then Jakub
Re: [PATCH] testsuite: fix analyzer C++ failures on Solaris [PR111475]
On Fri, May 03, 2024 at 09:31:08AM -0400, David Malcolm wrote: > Jakub, Richi, Rainer: this is a non-trivial change that cleans up > analyzer C++ testsuite results on Solaris, but has a slight risk of > affecting analyzer behavior on other targets. As such, I was thinking > to hold off on backporting it to GCC 14 until after 14.1 is released. > Is that a good plan? Agreed 14.2 is better target than 14.1 for this, especially if committed shortly after 14.1 goes out. Jakub
Re: [PATCH] Avoid changing type in the type_hash_canon hash
On Fri, May 03, 2024 at 12:58:55PM +0200, Richard Biener wrote: > When building a type and type_hash_canon returns an existing type > avoid changing it, in particular its TYPE_CANONICAL. > > Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages. > > OK for trunk? > > Thanks, > Richard. > > PR middle-end/114931 > * tree.cc (build_array_type_1): Return early when type_hash_canon > returned an older existing type. > (build_function_type): Likewise. > (build_method_type_directly): Likewise. > (build_offset_type): Likewise. LGTM, thanks. Jakub
[PATCH] tree-inline: Add __builtin_stack_{save,restore} pair about inline calls with calls to alloca [PR113596]
Hi! The following patch adds save_NNN = __builtin_stack_save (); ... __builtin_stack_restore (save_NNN); pair around inline calls which call alloca (alloca calls because of VLA vars are ignored in that decision). The patch doesn't change anything on whether we try to inline such calls or not, it just fixes the behavior when we inline them despite those checks. The stack save/restore restores the behavior that alloca acquired regions are freed at the end of the containing call. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-03 Jakub Jelinek PR middle-end/113596 * tree-inline.cc (expand_call_inline): Emit __builtin_stack_save and __builtin_stack_restore calls around inlined functions which call alloca. * gcc.dg/pr113596.c: New test. * gcc.dg/tree-ssa/pr113596.c: New test. --- gcc/tree-inline.cc.jj 2024-04-11 11:09:07.274670922 +0200 +++ gcc/tree-inline.cc 2024-05-02 19:05:06.963750322 +0200 @@ -4794,6 +4794,7 @@ expand_call_inline (basic_block bb, gimp use_operand_p use; gimple *simtenter_stmt = NULL; vec *simtvars_save; + tree save_stack = NULL_TREE; /* The gimplifier uses input_location in too many places, such as internal_get_tmp_var (). */ @@ -5042,6 +5043,28 @@ expand_call_inline (basic_block bb, gimp GSI_NEW_STMT); } + /* If function to be inlined calls alloca, wrap the inlined function + in between save_stack = __builtin_stack_save (); and + __builtin_stack_restore (save_stack); calls. */ + if (id->src_cfun->calls_alloca && !gimple_call_noreturn_p (stmt)) +/* Don't do this for VLA allocations though, just for user alloca + calls. */ +for (struct cgraph_edge *e = id->src_node->callees; e; e = e->next_callee) + if (gimple_maybe_alloca_call_p (e->call_stmt) + && !gimple_call_alloca_for_var_p (e->call_stmt)) + { + tree fn = builtin_decl_implicit (BUILT_IN_STACK_SAVE); + gcall *call = gimple_build_call (fn, 0); + save_stack = make_ssa_name (ptr_type_node); + gimple_call_set_lhs (call, save_stack); + gimple_stmt_iterator si = gsi_last_bb (bb); + gsi_insert_after (, call, GSI_NEW_STMT); + struct cgraph_node *dest = cgraph_node::get_create (fn); + id->dst_node->create_edge (dest, call, bb->count)->inline_failed + = CIF_BODY_NOT_AVAILABLE; + break; + } + if (DECL_INITIAL (fn)) { if (gimple_block (stmt)) @@ -5165,6 +5188,17 @@ expand_call_inline (basic_block bb, gimp } } + if (save_stack) +{ + tree fn = builtin_decl_implicit (BUILT_IN_STACK_RESTORE); + gcall *call = gimple_build_call (fn, 1, save_stack); + gsi_insert_before (_gsi, call, GSI_SAME_STMT); + struct cgraph_node *dest = cgraph_node::get_create (fn); + id->dst_node->create_edge (dest, call, +return_block->count)->inline_failed + = CIF_BODY_NOT_AVAILABLE; +} + /* Reset the escaped solution. */ if (cfun->gimple_df) { --- gcc/testsuite/gcc.dg/pr113596.c.jj 2024-05-02 15:05:25.048642302 +0200 +++ gcc/testsuite/gcc.dg/pr113596.c 2024-05-02 15:05:25.048642302 +0200 @@ -0,0 +1,24 @@ +/* PR middle-end/113596 */ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +__attribute__((noipa)) void +bar (char *p, int n) +{ + p[0] = 1; + p[n - 1] = 2; +} + +static inline __attribute__((always_inline)) void +foo (int n) +{ + char *p = __builtin_alloca (n); + bar (p, n); +} + +int +main () +{ + for (int i = 2; i < 8192; ++i) +foo (i); +} --- gcc/testsuite/gcc.dg/tree-ssa/pr113596.c.jj 2024-05-02 19:10:29.218455257 +0200 +++ gcc/testsuite/gcc.dg/tree-ssa/pr113596.c2024-05-02 19:11:11.211895559 +0200 @@ -0,0 +1,37 @@ +/* PR middle-end/113596 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-einline" } */ +/* { dg-final { scan-tree-dump-times "__builtin_stack_save \\\(" 3 "einline" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_stack_restore \\\(" 3 "einline" } } */ + +void baz (char *p, int n); +volatile int v; + +static inline __attribute__((always_inline)) void +foo (int n) +{ + ++v; + { +char *p = __builtin_alloca (n); +baz (p, n); + } + ++v; +} + +static inline __attribute__((always_inline)) void +bar (int n) +{ + ++v; + { +char p[n]; +baz (p, n); + } + ++v; +} + +void +qux (int n) +{ + foo (n); + bar (n); +} Jakub
Re: Trait built-in naming convention
On Thu, May 02, 2024 at 12:52:59PM -0700, Ken Matsui wrote: > > This seems to be the prevailing sentiment, so let's continue that way. > > Thanks for the input. > > I actually found that we have two built-in type traits prefixed with > __builtin: __builtin_is_corresponding_member and That is a FE builtin, not a trait, and is very much different from the __is_* traits, is varargs with extra processing, I don't think any of the normal traits accepts pointer to members. > __builtin_is_pointer_interconvertible_with_class. Do we want to > update these to use __ instead for consistency? No, I think we want to keep them as is. Jakub
Re: [pushed] Objective-C, NeXT, v2: Correct a regression in code-gen.
On Thu, May 02, 2024 at 01:53:21PM +0100, Iain Sandoe wrote: > My testing of the GCC-14 release branch revealed an Objective-C > regression in code-gen, the fix has been tested on x86_64, i686 > and powerpc darwin, pushed to trunk. > > I will shortly apply this to the open branches, since they are > affected too. Given that this is completely local to Darwin and > Objective-C (and pretty trivial too) - would it be acceptable for > GCC-14.1? Can't this just wait for GCC 14.2? The code has been like that for years, no, and it is ObjC, not a release critical language. Jakub
Re: [PATCH] fix single argument static_assert
On Thu, May 02, 2024 at 12:28:29PM +0200, Marc Poulhiès wrote: > Single argument static_assert is C++17 only. > > gcc/ChangeLog: > > * value-range.h: fix static_assert to use 2 arguments. > --- > > Ok for master? Yes. Jakub
[committed] libgomp: Add gfx90c, 1036 and 1103 declare variant tests
Hi! Recently -march=gfx{90c,1036,1103} support has been added, but corresponding changes weren't done in the testsuite. The following patch adds that. Tested on x86_64-linux (with fiji and gfx1103 devices; had to use OMP_DEFAULT_DEVICE=1 there, fiji doesn't really work due to LLVM dropping support, but we still list those as offloading devices). Committed to trunk. 2024-05-02 Jakub Jelinek * testsuite/libgomp.c/declare-variant-4.h (gfx90c, gfx1036, gfx1103): New functions. (f): Add #pragma omp declare variant directives for those. * testsuite/libgomp.c/declare-variant-4-gfx90c.c: New test. * testsuite/libgomp.c/declare-variant-4-gfx1036.c: New test. * testsuite/libgomp.c/declare-variant-4-gfx1103.c: New test. --- libgomp/testsuite/libgomp.c/declare-variant-4.h.jj 2024-01-29 12:11:57.917149306 +0100 +++ libgomp/testsuite/libgomp.c/declare-variant-4.h 2024-05-02 11:41:42.579379273 +0200 @@ -37,6 +37,13 @@ gfx90a (void) __attribute__ ((noipa)) int +gfx90c (void) +{ + return 0x90c; +} + +__attribute__ ((noipa)) +int gfx1030 (void) { return 0x1030; @@ -44,11 +51,25 @@ gfx1030 (void) __attribute__ ((noipa)) int +gfx1036 (void) +{ + return 0x1036; +} + +__attribute__ ((noipa)) +int gfx1100 (void) { return 0x1100; } +__attribute__ ((noipa)) +int +gfx1103 (void) +{ + return 0x1103; +} + #ifdef USE_FIJI_FOR_GFX803 #pragma omp declare variant(gfx803) match(device = {isa("fiji")}) #else @@ -58,8 +79,11 @@ gfx1100 (void) #pragma omp declare variant(gfx906) match(device = {isa("gfx906")}) #pragma omp declare variant(gfx908) match(device = {isa("gfx908")}) #pragma omp declare variant(gfx90a) match(device = {isa("gfx90a")}) +#pragma omp declare variant(gfx90c) match(device = {isa("gfx90c")}) #pragma omp declare variant(gfx1030) match(device = {isa("gfx1030")}) +#pragma omp declare variant(gfx1036) match(device = {isa("gfx1036")}) #pragma omp declare variant(gfx1100) match(device = {isa("gfx1100")}) +#pragma omp declare variant(gfx1103) match(device = {isa("gfx1103")}) __attribute__ ((noipa)) int f (void) --- libgomp/testsuite/libgomp.c/declare-variant-4-gfx90c.c.jj 2024-05-02 11:38:57.272597106 +0200 +++ libgomp/testsuite/libgomp.c/declare-variant-4-gfx90c.c 2024-05-02 11:39:11.169410657 +0200 @@ -0,0 +1,8 @@ +/* { dg-do link { target { offload_target_amdgcn } } } */ +/* { dg-additional-options -foffload=amdgcn-amdhsa } */ +/* { dg-additional-options -foffload=-march=gfx90c } */ +/* { dg-additional-options "-foffload=-fdump-tree-optimized" } */ + +#include "declare-variant-4.h" + +/* { dg-final { only_for_offload_target amdgcn-amdhsa scan-offload-tree-dump "= gfx90c \\(\\);" "optimized" } } */ --- libgomp/testsuite/libgomp.c/declare-variant-4-gfx1036.c.jj 2024-05-02 11:39:29.393166162 +0200 +++ libgomp/testsuite/libgomp.c/declare-variant-4-gfx1036.c 2024-05-02 11:39:51.834865074 +0200 @@ -0,0 +1,8 @@ +/* { dg-do link { target { offload_target_amdgcn } } } */ +/* { dg-additional-options -foffload=amdgcn-amdhsa } */ +/* { dg-additional-options -foffload=-march=gfx1036 } */ +/* { dg-additional-options "-foffload=-fdump-tree-optimized" } */ + +#include "declare-variant-4.h" + +/* { dg-final { only_for_offload_target amdgcn-amdhsa scan-offload-tree-dump "= gfx1036 \\(\\);" "optimized" } } */ --- libgomp/testsuite/libgomp.c/declare-variant-4-gfx1103.c.jj 2024-05-02 11:39:43.155981513 +0200 +++ libgomp/testsuite/libgomp.c/declare-variant-4-gfx1103.c 2024-05-02 11:40:02.801717936 +0200 @@ -0,0 +1,8 @@ +/* { dg-do link { target { offload_target_amdgcn } } } */ +/* { dg-additional-options -foffload=amdgcn-amdhsa } */ +/* { dg-additional-options -foffload=-march=gfx1103 } */ +/* { dg-additional-options "-foffload=-fdump-tree-optimized" } */ + +#include "declare-variant-4.h" + +/* { dg-final { only_for_offload_target amdgcn-amdhsa scan-offload-tree-dump "= gfx1103 \\(\\);" "optimized" } } */ Jakub
Re: [PATCH] libgcc: Rename __trunchfbf2 to __extendhfbf2
On Wed, May 01, 2024 at 12:55:25PM -0700, H.J. Lu wrote: > Since bfloat16 has the same range as float32, _Float16 to bfloat16 > conversion is an extension, not a truncation. Rename trunchfbf2.c > to extendhfbf2.c to provide __extendhfbf2, instead of __trunchfbf2. > > Since _Float16 to bfloat16 conversion never worked from the day one, > the same libgcc version of __trunchfbf2 is used with __extendhfbf2 so > that this can be backported to release branches all the way where > __trunchfbf2 was added. This is wrong. First of all, it is ABI incompatible change, we can't do that. And second, neither _Float16 is a subset of __bf16 nor the other way, so both extend and trunc names are equally wrong. Jakub
[PATCH] c++: Implement C++26 P2573R2 - = delete("should have a reason"); [PR114458]
Hi! The following patch implements the C++26 P2573R2 = delete("should have a reason"); paper. I've tried to avoid increasing compile time memory for it when it isn't used (e.g. by adding a new lang_decl tree member), so the reason is represented as STRING_CST in DECL_INITIAL (which normally is for DECL_DELETED_FN error_mark_node) and to differentiate this delete("reason") initializer from some bogus attempt to initialize a function with "reason" using the RID_DELETE identifier as TREE_TYPE of the STRING_CST, as nothing needs to care about the type of the reason. If preferred it could be say TREE_LIST with the reason STRING_CST and RID_DELETE identifier or something similar instead, but that would need more compile time memory when it is used. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-05-01 Jakub Jelinek PR c++/114458 gcc/c-family/ * c-cppbuiltin.cc (c_cpp_builtins): Predefine __cpp_deleted_function=202403L for C++26. gcc/cp/ChangeLog * parser.cc (cp_parser_pure_specifier): Implement C++26 P2573R2 - = delete("should have a reason");. Parse deleted-function-body. * decl.cc (duplicate_decls): Copy DECL_INITIAL from DECL_DELETED_FN olddecl to newdecl if it is a STRING_CST. (cp_finish_decl): Handle deleted init with a reason. * decl2.cc: Include "escaped_string.h". (grokfield): Handle deleted init with a reason. (mark_used): Emit DECL_DELETED_FN reason in the message if any. gcc/testsuite/ * g++.dg/cpp26/feat-cxx26.C (__cpp_deleted_function): Add test. * g++.dg/cpp26/delete-reason1.C: New test. * g++.dg/cpp26/delete-reason2.C: New test. * g++.dg/parse/error65.C (f1): Adjust expected diagnostics. --- gcc/c-family/c-cppbuiltin.cc.jj 2024-04-30 08:57:07.359039013 +0200 +++ gcc/c-family/c-cppbuiltin.cc2024-04-30 19:16:45.069542205 +0200 @@ -1092,6 +1092,7 @@ c_cpp_builtins (cpp_reader *pfile) cpp_define (pfile, "__cpp_static_assert=202306L"); cpp_define (pfile, "__cpp_placeholder_variables=202306L"); cpp_define (pfile, "__cpp_structured_bindings=202403L"); + cpp_define (pfile, "__cpp_deleted_function=202403L"); } if (flag_concepts) { --- gcc/cp/parser.cc.jj 2024-04-30 08:57:07.349039147 +0200 +++ gcc/cp/parser.cc2024-04-30 16:47:01.427952875 +0200 @@ -28573,6 +28573,27 @@ cp_parser_pure_specifier (cp_parser* par || token->keyword == RID_DELETE) { maybe_warn_cpp0x (CPP0X_DEFAULTED_DELETED); + if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN)) + { + if (cxx_dialect >= cxx11 && cxx_dialect < cxx26) + pedwarn (cp_lexer_peek_token (parser->lexer)->location, +OPT_Wc__26_extensions, +"% reason only available with " +"%<-std=c++2c%> or %<-std=gnu++2c%>"); + + /* Consume the `('. */ + matching_parens parens; + parens.consume_open (parser); + tree reason = cp_parser_unevaluated_string_literal (parser); + /* Consume the `)'. */ + parens.require_close (parser); + if (TREE_CODE (reason) == STRING_CST) + { + TREE_TYPE (reason) = token->u.value; + return reason; + } + } + return token->u.value; } --- gcc/cp/decl.cc.jj 2024-04-30 08:55:26.172389593 +0200 +++ gcc/cp/decl.cc 2024-04-30 19:01:32.316543498 +0200 @@ -2410,6 +2410,10 @@ duplicate_decls (tree newdecl, tree oldd "previous declaration of %qD", olddecl); } DECL_DELETED_FN (newdecl) |= DECL_DELETED_FN (olddecl); + if (DECL_DELETED_FN (olddecl) + && DECL_INITIAL (olddecl) + && TREE_CODE (DECL_INITIAL (olddecl)) == STRING_CST) + DECL_INITIAL (newdecl) = DECL_INITIAL (olddecl); } } @@ -8587,17 +8591,20 @@ cp_finish_decl (tree decl, tree init, bo if (init && TREE_CODE (decl) == FUNCTION_DECL) { tree clone; - if (init == ridpointers[(int)RID_DELETE]) + if (init == ridpointers[(int)RID_DELETE] + || (TREE_CODE (init) == STRING_CST + && TREE_TYPE (init) == ridpointers[(int)RID_DELETE])) { /* FIXME check this is 1st decl. */ DECL_DELETED_FN (decl) = 1; DECL_DECLARED_INLINE_P (decl) = 1; - DECL_INITIAL (decl) = error_mark_node; + DECL_INITIAL (decl) + = TREE_CODE (init) == STRING_CST ? init : error_mark_node; FOR_EACH_CLONE (clone, decl) { DECL_DELETED_FN (clone) = 1; DECL_DECLARED_INLINE_P (clone) = 1; - DECL_INITIAL (clone) = error_mark
Re: [wwwdocs] Porting-to-14: Mention new pragma GCC Target behavior
On Tue, Apr 30, 2024 at 11:12:30PM +0200, Martin Jambor wrote: > Would the following then perhaps describe the situation accurately? > Note that I have moved the whole thing to C++ section because it seems > porting issues in C because of this are quite unlikely. > > Michal, I assume that the file where this issue happened was written in > C++, right? > > Martin > > > > diff --git a/htdocs/gcc-14/porting_to.html b/htdocs/gcc-14/porting_to.html > index c825a68e..1e67b0b3 100644 > --- a/htdocs/gcc-14/porting_to.html > +++ b/htdocs/gcc-14/porting_to.html > @@ -514,6 +514,51 @@ be included explicitly when compiling with GCC 14: > > > > +Pragma GCC Target now affects preprocessor > symbols I'd use lowercase Target here > + > + > +The behavior of pragma GCC Target and specifically how it affects ISA And here as well, perhaps even #pragma GCC target. Otherwise LGTM. > +macros has changed in GCC 14. In GCC 13 and older, the GCC > +target pragma defined and undefined corresponding ISA macros in > +C when using integrated preprocessor during compilation but not when > +preprocessor was invoked as a separate step or when using -save-temps. > +In C++ the ISA macro definitions were performed in a way which did not > +have any actual effect. > + > +In GCC 14 C++ behaves like C with integrated preprocessing in earlier > +versions. Moreover, in both languages ISA macros are defined and > +undefined as expected when preprocessing separately from compilation. > + > + > +This can lead to different behavior, especially in C++. For example, > +functions the C++ snippet below will be (silently) compiled for an > +incorrect instruction set by GCC 14. > + > + > + #if ! __AVX2__ > + #pragma GCC push_options > + #pragma GCC target("avx2") > + #endif > + > + /* Code to be compiled for AVX2. */ > + > + /* With GCC 14, __AVX2__ here will always be defined and pop_options > + never called. */ > + #if ! __AVX2__ > + #pragma GCC pop_options > + #endif > + > + /* With GCC 14, all following functions will be compiled for AVX2 > + which was not intended. */ > + > + > + > +The fix in this case would be to remember > +whether pop_options needs to be performed in a new > +user-defined macro. > + > + > + > > > Jakub
Re: [PATCH] Don't assert for IFN_COND_{MIN, MAX} in vect_transform_reduction
On Tue, Apr 30, 2024 at 09:30:00AM +0200, Richard Biener wrote: > On Mon, Apr 29, 2024 at 5:30 PM H.J. Lu wrote: > > > > On Mon, Apr 29, 2024 at 6:47 AM liuhongt wrote: > > > > > > The Fortran standard does not specify what the result of the MAX > > > and MIN intrinsics are if one of the arguments is a NaN. So it > > > should be ok to tranform reduction for IFN_COND_MIN with vectorized > > > COND_MIN and REDUC_MIN. > > > > The commit subject isn't very clear. This patch isn't about "Don't assert > > for IFN_COND_{MIN,MAX}". It allows IFN_COND_{MIN,MAX} in > > vect_transform_reduction. > > Well, we allow it elsewhere, we just fail to enumerate all COND_* we allow > here correctly. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}. > > > Ok for trunk and backport to GCC14? > > OK for trunk and branch. Oops, I've just sent the same patch, just with a different testcase (reduced and which tests both the min and max). I think the reduced testcase is better. > > > gcc/ChangeLog: > > > > > > PR 114883 Missing tree-optimization/ > > > * tree-vect-loop.cc (vect_transform_reduction): Don't assert > > > for IFN_COND_{MIN, MAX}. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gfortran.dg/pr114883.f90: New test. Jakub
[PATCH] vect: Adjust vect_transform_reduction assertion [PR114883]
Hi! The assertion doesn't allow IFN_COND_MIN/IFN_COND_MAX, which are commutative conditional binary operations like ADD/MUL/AND/IOR/XOR, and can be handled just fine. In particular, we emit vminpd %zmm3, %zmm5, %zmm0{%k2} vminpd %zmm0, %zmm3, %zmm5{%k1} and vmaxpd %zmm3, %zmm5, %zmm0{%k2} vmaxpd %zmm0, %zmm3, %zmm5{%k1} in the vectorized loops of the first and second subroutine. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and 14.1? 2024-04-30 Jakub Jelinek Hongtao Liu PR tree-optimization/114883 * tree-vect-loop.cc (vect_transform_reduction): Allow IFN_COND_MIN and IFN_COND_MAX in the assert. * gfortran.dg/pr114883.f90: New test. --- gcc/tree-vect-loop.cc.jj2024-04-17 11:34:02.465185397 +0200 +++ gcc/tree-vect-loop.cc 2024-04-29 20:41:04.973723992 +0200 @@ -8505,7 +8505,8 @@ vect_transform_reduction (loop_vec_info { gcc_assert (code == IFN_COND_ADD || code == IFN_COND_SUB || code == IFN_COND_MUL || code == IFN_COND_AND - || code == IFN_COND_IOR || code == IFN_COND_XOR); + || code == IFN_COND_IOR || code == IFN_COND_XOR + || code == IFN_COND_MIN || code == IFN_COND_MAX); gcc_assert (op.num_ops == 4 && (op.ops[reduc_index] == op.ops[internal_fn_else_index ((internal_fn) code)])); --- gcc/testsuite/gfortran.dg/pr114883.f90.jj 2024-04-29 20:39:39.000871849 +0200 +++ gcc/testsuite/gfortran.dg/pr114883.f90 2024-04-29 20:39:27.757021972 +0200 @@ -0,0 +1,53 @@ +! PR tree-optimization/114883 +! { dg-do compile } +! { dg-options "-O2 -fvect-cost-model=cheap" } +! { dg-additional-options "-march=x86-64-v4" { target i?86-*-* x86_64-*-* } } + +subroutine pr114883_1(a, b, c, d, e, f, g, h, o) + real(8) :: c(1011), d(1011), e(0:1011) + real(8) :: p, q, f, r, g(1011), h(1011), b, bar + integer :: o(100), a, t, u + p = 0.0_8 + r = bar() + u = 1 + do i = 1,a +do k = 1,1011 + km1 = max0(k-1,1) + h(k) = c(k) * e(k-1) * d(km1) + f = g(k) + h(k) + if(f.gt.1.e-6)then +p = min(p,r) + endif +end do +q = 0.9_8 * p +t = integer(b/q + 1) +if(t>100)then + u = t +endif +o(u) = o(u) + 1 + end do +end subroutine pr114883_1 +subroutine pr114883_2(a, b, c, d, e, f, g, h, o) + real(8) :: c(1011), d(1011), e(0:1011) + real(8) :: p, q, f, r, g(1011), h(1011), b, bar + integer :: o(100), a, t, u + p = 0.0_8 + r = bar() + u = 1 + do i = 1,a +do k = 1,1011 + km1 = max0(k-1,1) + h(k) = c(k) * e(k-1) * d(km1) + f = g(k) + h(k) + if(f.gt.1.e-6)then +p = max(p,r) + endif +end do +q = 0.9_8 * p +t = integer(b/q + 1) +if(t>100)then + u = t +endif +o(u) = o(u) + 1 + end do +end subroutine pr114883_2 Jakub
[PATCH] gimple-ssa-sprintf: Use [0, 1] range for %lc with (wint_t) 0 argument [PR114876]
Hi! Seems when Martin S. implemented this, he coded there strict reading of the standard, which said that %lc with (wint_t) 0 argument is handled as wchar_t[2] temp = { arg, 0 }; %ls with temp arg and so shouldn't print any values. But, most of the libc implementations actually handled that case like %c with '\0' argument, adding a single NUL character, the only known exception is musl. Recently, C23 changed this in response to GB-141 and POSIX in https://austingroupbugs.net/view.php?id=1647 so that it should have the same behavior as %c with '\0'. Because there is implementation divergence, the following patch uses a range rather than hardcoding it to all 1s (i.e. the %c behavior), though the likely case is still 1 (forward looking plus most of implementations). The res.knownrange = true; assignment removed is redundant due to the same assignment done unconditionally before the if statement, rest is formatting fixes. I don't think the min >= 0 && min < 128 case is right either, I'd think it should be min >= 0 && max < 128, otherwise it is just some possible inputs are (maybe) ASCII and there can be others, but this code is a total mess anyway, with the min, max, likely (somewhere in [min, max]?) and then unlikely possibly larger than max, dunno, perhaps for at least some chars in the ASCII range the likely case could be for the ascii case; so perhaps just the one_2_one_ascii shouldn't set max to 1 and mayfail should be true for max >= 128. Anyway, didn't feel I should touch that right now. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Shall it go to 14.1, or wait for 14.2? 2024-04-30 Jakub Jelinek PR tree-optimization/114876 * gimple-ssa-sprintf.cc (format_character): For min == 0 && max == 0, set max, likely and unlikely members to 1 rather than 0. Remove useless res.knownrange = true;. Formatting fixes. * gcc.dg/pr114876.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust expected diagnostics. --- gcc/gimple-ssa-sprintf.cc.jj2024-01-03 11:51:22.225860346 +0100 +++ gcc/gimple-ssa-sprintf.cc 2024-04-29 12:52:59.760668894 +0200 @@ -2177,8 +2177,7 @@ format_character (const directive , res.knownrange = true; - if (dir.specifier == 'C' - || dir.modifier == FMT_LEN_l) + if (dir.specifier == 'C' || dir.modifier == FMT_LEN_l) { /* A wide character can result in as few as zero bytes. */ res.range.min = 0; @@ -2189,10 +2188,13 @@ format_character (const directive , { if (min == 0 && max == 0) { - /* The NUL wide character results in no bytes. */ - res.range.max = 0; - res.range.likely = 0; - res.range.unlikely = 0; + /* In strict reading of older ISO C or POSIX, this required +no characters to be emitted. ISO C23 changes that, so +does POSIX, to match what has been implemented in most of the +implementations, namely emitting a single NUL character. +Let's use 0 for minimum and 1 for all the other values. */ + res.range.max = 1; + res.range.likely = res.range.unlikely = 1; } else if (min >= 0 && min < 128) { @@ -2200,11 +2202,12 @@ format_character (const directive , is not a 1-to-1 mapping to the source character set or if the source set is not ASCII. */ bool one_2_one_ascii - = (target_to_host_charmap[0] == 1 && target_to_host ('a') == 97); + = (target_to_host_charmap[0] == 1 + && target_to_host ('a') == 97); /* A wide character in the ASCII range most likely results in a single byte, and only unlikely in up to MB_LEN_MAX. */ - res.range.max = one_2_one_ascii ? 1 : target_mb_len_max ();; + res.range.max = one_2_one_ascii ? 1 : target_mb_len_max (); res.range.likely = 1; res.range.unlikely = target_mb_len_max (); res.mayfail = !one_2_one_ascii; @@ -2235,7 +2238,6 @@ format_character (const directive , /* A plain '%c' directive. Its output is exactly 1. */ res.range.min = res.range.max = 1; res.range.likely = res.range.unlikely = 1; - res.knownrange = true; } /* Bump up the byte counters if WIDTH is greater. */ --- gcc/testsuite/gcc.dg/pr114876.c.jj 2024-04-29 12:26:45.774965158 +0200 +++ gcc/testsuite/gcc.dg/pr114876.c 2024-04-29 12:51:37.863777055 +0200 @@ -0,0 +1,34 @@ +/* PR tree-optimization/114876 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump-not "return \[01\];" "optimized" } } */ +/* { dg-final { scan-tree-dump "return 3;" &
[PATCH] libgcc: Do use weakrefs for glibc 2.34 on GNU Hurd
On Mon, Apr 29, 2024 at 01:44:24PM +, Joseph Myers wrote: > > glibc 2.34 and later doesn't have separate libpthread (libpthread.so.0 is a > > dummy shared library with just some symbol versions for compatibility, but > > all the pthread_* APIs are in libc.so.6). > > I suspect this has caused link failures in the glibc testsuite for Hurd, > which still has separate libpthread. > > https://sourceware.org/pipermail/libc-testresults/2024q2/012556.html So like this then? I can't really test it on Hurd, but will certainly test on x86_64-linux/i686-linux. 2024-04-29 Jakub Jelinek * gthr.h (GTHREAD_USE_WEAK): Don't redefine to 0 for glibc 2.34+ on GNU Hurd. --- libgcc/gthr.h.jj2024-04-25 20:43:10.555694952 +0200 +++ libgcc/gthr.h 2024-04-29 16:57:40.734062691 +0200 @@ -142,7 +142,7 @@ see the files COPYING3 and COPYING.RUNTI #endif #ifdef __GLIBC_PREREQ -#if __GLIBC_PREREQ(2, 34) +#if __GLIBC_PREREQ(2, 34) && !defined(__gnu_hurd__) /* glibc 2.34 and later has all pthread_* APIs inside of libc, no need to link separately with -lpthread. */ #undef GTHREAD_USE_WEAK Jakub
Re: [PATCH] arm: [MVE intrinsics] Fix support for predicate constants [PR target/114801]
On Fri, Apr 26, 2024 at 11:10:12PM +, Christophe Lyon wrote: > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/mve/pr114801.c > @@ -0,0 +1,36 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */ > +/* { dg-add-options arm_v8_1m_mve } */ > +/* { dg-final { check-function-bodies "**" "" "" } } */ > + > +#include > + > +/* > +** test_32: > +**... > +** mov r[0-9]+, #65535 @ movhi > +**... > +*/ > +uint32x4_t test_32() { > + return vdupq_m_n_u32(vdupq_n_u32(0), 0, 0x); Just a testcase nit. I think testing 0x isn't that useful, it tests the same 4 bits 4 times. Might be more interesting to test 4 different 4 bit elements, one of them 0 (to verify it doesn't turn that into all ones), one all 1s (that is the other valid case) and then 2 random other values in between. > +} > + > +/* > +** test_16: > +**... > +** mov r[0-9]+, #52428 @ movhi > +**... > +*/ > +uint16x8_t test_16() { > + return vdupq_m_n_u16(vdupq_n_u16(0), 0, 0x); And for these it can actually test all 4 possible 2 bit elements, so say 0x3021 > +} > + > +/* > +** test_8: > +**... > +** mov r[0-9]+, #52428 @ movhi > +**... > +*/ > +uint8x16_t test_8() { > + return vdupq_m_n_u8(vdupq_n_u8(0), 0, 0x); and here use some random pattern. BTW, the patch is ok for 14.1 if it is approved and committed today (so that it can be cherry-picked tomorrow morning at latest to the branch). Jakub
Re: [Patch, fortran] PR114859 - [14/15 Regression] Seeing new segmentation fault in same_type_as since r14-9752
On Sun, Apr 28, 2024 at 10:37:06PM +0100, Paul Richard Thomas wrote: > Could this be looked at quickly? The timing of this regression is more than > a little embarrassing on the eve of the 14.1 release. The testcase and the > comment in gfc_trans_class_init_assign explain what this problem is all > about and how the patch fixes it. > > OK for 15-branch and backporting to 14-branch (hopefully to the RC as well)? The patch is ok for 14.1 if cherry-picked today. Jakub
Re: [PATCH] libstdc++: Update Solaris baselines for GCC 14.0
On Mon, Apr 29, 2024 at 10:07:42AM +0200, Rainer Orth wrote: > This patch updates the Solaris baselines for the GLIBCXX_3.4.33 version > added in GCC 14.0. > > Tested on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (32 and 64-bit > each), together with the GLIBCXX_3.4.32 update, on both gcc-14 branch > and trunk. > > Ok for and gcc-14 branch and trunk? Ok for both. > 2024-04-28 Rainer Orth > > libstdc++-v3: > * config/abi/post/i386-solaris/baseline_symbols.txt: Regenerate. > * config/abi/post/i386-solaris/amd64/baseline_symbols.txt: > Likewise. > * config/abi/post/sparc-solaris/baseline_symbols.txt: Likewise. > * config/abi/post/sparc-solaris/sparcv9/baseline_symbols.txt: > Likewise. Jakub
Re: [PATCH] libstdc++: Update Solaris baselines for GCC 13.2
On Mon, Apr 29, 2024 at 10:02:56AM +0200, Rainer Orth wrote: > This patch updates the Solaris baselines for the GLIBCXX_3.4.32 version > added in GCC 13.2. > > Tested on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (32 and 64-bit > each) on the gcc-13 branch and (together with the GLIBCXX_3.4.33 update) > on both gcc-14 branch and trunk. > > Ok for all of gcc-13 and gcc-14 branches and trunk? I think Solaris shouldn't have the _ZNSt8ios_base4InitC1Ev @@GLIBCXX_3.4.32 export, so this LGTM, for all 3 branches. > 2024-04-28 Rainer Orth > > libstdc++-v3: > * config/abi/post/i386-solaris/baseline_symbols.txt: Regenerate. > * config/abi/post/i386-solaris/amd64/baseline_symbols.txt: > Likewise. > * config/abi/post/sparc-solaris/baseline_symbols.txt: Likewise. > * config/abi/post/sparc-solaris/sparcv9/baseline_symbols.txt: > Likewise. Jakub
Re: [PATCH v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885]
On Mon, Apr 29, 2024 at 03:47:05PM +0800, Kito Cheng wrote: > Hi Jakub: > > Is this OK for GCC 14 branch? it's fix ICE on valid code, thanks :) Ok. Jakub
[PATCH] c++: Implement C++26 P0609R3 - Attributes for Structured Bindings [PR114456]
Hi! The following patch implements the P0609R3 paper; we build the VAR_DECLs for the structured binding identifiers early, so all we need IMHO is just to parse the attributed identifier list and pass the attributes to the VAR_DECL creation. The paper mentions maybe_unused and gnu::nonstring attributes as examples where they can be useful. Not sure about either of them. For maybe_unused, the thing is that both GCC and clang already don't diagnose maybe unused for the structured binding identifiers, because it would be a false positive too often; and there is no easy way to find out if a structured binding has been written with the P0609R3 paper in mind or not (maybe we could turn it on if in the structured binding is any attribute, even if just [[]] and record that as a flag on the whole underlying decl, so that we'd diagnose auto [a, b, c[[]]] = d; // use a, c but not b but not auto [e, f, g] = d; // use a, c but not b ). For gnu::nonstring, the issue is that we currently don't allow the attribute on references to char * or references to char[], just on char */char[]. I've filed a PR for that. The first testcase in the patch tests it on [[]] and [[maybe_unused]], just whether it is parsed properly, second on gnu::deprecated, which works. Haven't used deprecated attribute because the paper said that attribute is for further investigation. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-29 Jakub Jelinek PR c++/114456 gcc/c-family/ * c-cppbuiltin.cc (c_cpp_builtins): Predefine __cpp_structured_bindings for C++26 to 202403L rather than 201606L. gcc/cp/ * parser.cc (cp_parser_decomposition_declaration): Implement C++26 P0609R3 - Attributes for Structured Bindings. Parse attributed identifier lists for structured binding declarations, pass the attributes to start_decl. gcc/testsuite/ * g++.dg/cpp26/decomp1.C: New test. * g++.dg/cpp26/decomp2.C: New test. * g++.dg/cpp26/feat-cxx26.C (__cpp_structured_bindings): Expect 202403 rather than 201606. --- gcc/cp/parser.cc.jj 2024-04-26 11:42:24.653016208 +0200 +++ gcc/cp/parser.cc2024-04-26 13:59:17.791482874 +0200 @@ -16075,13 +16075,37 @@ cp_parser_decomposition_declaration (cp_ /* Parse the identifier-list. */ auto_vec v; + bool attr_diagnosed = false; + int first_attr = -1; + unsigned int cnt = 0; if (!cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_SQUARE)) while (true) { cp_expr e = cp_parser_identifier (parser); if (e.get_value () == error_mark_node) break; + tree attr = NULL_TREE; + if (cp_next_tokens_can_be_std_attribute_p (parser)) + { + if (cxx_dialect >= cxx17 && cxx_dialect < cxx26 && !attr_diagnosed) + { + pedwarn (cp_lexer_peek_token (parser->lexer)->location, +OPT_Wc__26_extensions, +"structured bindings with attributed identifiers " +"only available with %<-std=c++2c%> or " +"%<-std=gnu++2c%>"); + attr_diagnosed = true; + } + attr = cp_parser_std_attribute_spec_seq (parser); + if (attr == error_mark_node) + attr = NULL_TREE; + if (attr && first_attr == -1) + first_attr = v.length (); + } v.safe_push (e); + ++cnt; + if (first_attr != -1) + v.safe_push (attr); if (!cp_lexer_next_token_is (parser->lexer, CPP_COMMA)) break; cp_lexer_consume_token (parser->lexer); @@ -16139,8 +16163,11 @@ cp_parser_decomposition_declaration (cp_ declarator->id_loc = e.get_location (); } tree elt_pushed_scope; + tree attr = NULL_TREE; + if (first_attr != -1 && i >= (unsigned) first_attr) + attr = v[++i].get_value (); tree decl2 = start_decl (declarator, _specs, SD_DECOMPOSITION, - NULL_TREE, NULL_TREE, _pushed_scope); + NULL_TREE, attr, _pushed_scope); if (decl2 == error_mark_node) decl = error_mark_node; else if (decl != error_mark_node && DECL_CHAIN (decl2) != prev) @@ -16183,7 +16210,7 @@ cp_parser_decomposition_declaration (cp_ if (decl != error_mark_node) { - cp_decomp decomp = { prev, v.length () }; + cp_decomp decomp = { prev, cnt }; cp_finish_decl (decl, initializer, non_constant_p, NULL_TREE, (is_direct_init ? LOOKUP_NORMAL : LOOKUP_IMPLICIT), ); @@ -16193,7 +16220,7 @@ cp_parser_decomposition_declaration (cp_ else if (decl != error_mark_node) { *maybe_range_for_decl = prev; - cp_decomp decomp = { prev, v.length
[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);
[PATCH] libgcc: Don't use weakrefs for glibc 2.34
Hi! glibc 2.34 and later doesn't have separate libpthread (libpthread.so.0 is a dummy shared library with just some symbol versions for compatibility, but all the pthread_* APIs are in libc.so.6). So, we don't need to do the .weakref dances to check whether a program has been linked with -lpthread or not, in dynamically linked apps those will be always true anyway. In -static linking, this fixes various issues people had when only linking some parts of libpthread.a and getting weird crashes. A hack for that was what e.g. some Fedora glibcs used, where libpthread.a was a library containing just one giant *.o file which had all the normal libpthread.a *.o files linked with -r together. libstdc++-v3 actually does something like this already since r10-10928, the following patch is meant to fix it even for libgfortran, libobjc and whatever else uses gthr.h. Bootstrapped/regtested on x86_64-linux and i686-linux (with glibc 2.35), ok for trunk? 2024-04-25 Jakub Jelinek * gthr.h (GTHREAD_USE_WEAK): Redefine to 0 for GLIBC 2.34 or later. --- libgcc/gthr.h.jj2024-01-03 12:07:28.623363560 +0100 +++ libgcc/gthr.h 2024-04-25 12:09:39.708622613 +0200 @@ -141,6 +141,15 @@ see the files COPYING3 and COPYING.RUNTI #define GTHREAD_USE_WEAK 0 #endif +#ifdef __GLIBC_PREREQ +#if __GLIBC_PREREQ(2, 34) +/* glibc 2.34 and later has all pthread_* APIs inside of libc, + no need to link separately with -lpthread. */ +#undef GTHREAD_USE_WEAK +#define GTHREAD_USE_WEAK 0 +#endif +#endif + #ifndef GTHREAD_USE_WEAK #define GTHREAD_USE_WEAK 1 #endif Jakub
[committed] openmp: Copy DECL_LANG_SPECIFIC and DECL_LANG_FLAG_? to tree-nested decl copy [PR114825]
Hi! tree-nested.cc creates in 2 spots artificial VAR_DECLs, one of them is used both for debug info and OpenMP/OpenACC lowering purposes, the other solely for OpenMP/OpenACC lowering purposes. When the decls are used in OpenMP/OpenACC lowering, the OMP langhooks (mostly Fortran, C just a little and C++ doesn't have nested functions) then inspect the flags on the vars and based on that decide how to lower the corresponding clauses. Unfortunately we weren't copying DECL_LANG_SPECIFIC and DECL_LANG_FLAG_?, so the langhooks made decisions on the default flags on those instead. As the original decl isn't necessarily a VAR_DECL, could be e.g. PARM_DECL, using copy_node wouldn't work properly, so this patch just copies those flags in addition to other flags it was copying already. And I've removed code duplication by introducing a helper function which does copying common to both uses. Bootstrapped/regtested on x86_64-linux and i686-linux, committed so far to trunk. 2024-04-25 Jakub Jelinek PR fortran/114825 * tree-nested.cc (get_debug_decl): New function. (get_nonlocal_debug_decl): Use it. (get_local_debug_decl): Likewise. * gfortran.dg/gomp/pr114825.f90: New test. --- gcc/tree-nested.cc.jj 2024-01-29 09:41:19.804391621 +0100 +++ gcc/tree-nested.cc 2024-04-24 18:02:55.103841888 +0200 @@ -1047,6 +1047,37 @@ get_frame_field (struct nesting_info *in static void note_nonlocal_vla_type (struct nesting_info *info, tree type); +/* Helper for get_nonlocal_debug_decl and get_local_debug_decl. */ + +static tree +get_debug_decl (tree decl) +{ + tree new_decl += build_decl (DECL_SOURCE_LOCATION (decl), + VAR_DECL, DECL_NAME (decl), TREE_TYPE (decl)); + DECL_ARTIFICIAL (new_decl) = DECL_ARTIFICIAL (decl); + DECL_IGNORED_P (new_decl) = DECL_IGNORED_P (decl); + TREE_THIS_VOLATILE (new_decl) = TREE_THIS_VOLATILE (decl); + TREE_SIDE_EFFECTS (new_decl) = TREE_SIDE_EFFECTS (decl); + TREE_READONLY (new_decl) = TREE_READONLY (decl); + TREE_ADDRESSABLE (new_decl) = TREE_ADDRESSABLE (decl); + DECL_SEEN_IN_BIND_EXPR_P (new_decl) = 1; + if ((TREE_CODE (decl) == PARM_DECL + || TREE_CODE (decl) == RESULT_DECL + || VAR_P (decl)) + && DECL_BY_REFERENCE (decl)) +DECL_BY_REFERENCE (new_decl) = 1; + /* Copy DECL_LANG_SPECIFIC and DECL_LANG_FLAG_* for OpenMP langhook + purposes. */ + DECL_LANG_SPECIFIC (new_decl) = DECL_LANG_SPECIFIC (decl); +#define COPY_DLF(n) DECL_LANG_FLAG_##n (new_decl) = DECL_LANG_FLAG_##n (decl) + COPY_DLF (0); COPY_DLF (1); COPY_DLF (2); COPY_DLF (3); + COPY_DLF (4); COPY_DLF (5); COPY_DLF (6); COPY_DLF (7); + COPY_DLF (8); +#undef COPY_DLF + return new_decl; +} + /* A subroutine of convert_nonlocal_reference_op. Create a local variable in the nested function with DECL_VALUE_EXPR set to reference the true variable in the parent function. This is used both for debug info @@ -1094,21 +1125,8 @@ get_nonlocal_debug_decl (struct nesting_ x = build_simple_mem_ref_notrap (x); /* ??? We should be remapping types as well, surely. */ - new_decl = build_decl (DECL_SOURCE_LOCATION (decl), -VAR_DECL, DECL_NAME (decl), TREE_TYPE (decl)); + new_decl = get_debug_decl (decl); DECL_CONTEXT (new_decl) = info->context; - DECL_ARTIFICIAL (new_decl) = DECL_ARTIFICIAL (decl); - DECL_IGNORED_P (new_decl) = DECL_IGNORED_P (decl); - TREE_THIS_VOLATILE (new_decl) = TREE_THIS_VOLATILE (decl); - TREE_SIDE_EFFECTS (new_decl) = TREE_SIDE_EFFECTS (decl); - TREE_READONLY (new_decl) = TREE_READONLY (decl); - TREE_ADDRESSABLE (new_decl) = TREE_ADDRESSABLE (decl); - DECL_SEEN_IN_BIND_EXPR_P (new_decl) = 1; - if ((TREE_CODE (decl) == PARM_DECL - || TREE_CODE (decl) == RESULT_DECL - || VAR_P (decl)) - && DECL_BY_REFERENCE (decl)) -DECL_BY_REFERENCE (new_decl) = 1; SET_DECL_VALUE_EXPR (new_decl, x); DECL_HAS_VALUE_EXPR_P (new_decl) = 1; @@ -1892,21 +1910,8 @@ get_local_debug_decl (struct nesting_inf x = info->frame_decl; x = build3 (COMPONENT_REF, TREE_TYPE (field), x, field, NULL_TREE); - new_decl = build_decl (DECL_SOURCE_LOCATION (decl), -VAR_DECL, DECL_NAME (decl), TREE_TYPE (decl)); + new_decl = get_debug_decl (decl); DECL_CONTEXT (new_decl) = info->context; - DECL_ARTIFICIAL (new_decl) = DECL_ARTIFICIAL (decl); - DECL_IGNORED_P (new_decl) = DECL_IGNORED_P (decl); - TREE_THIS_VOLATILE (new_decl) = TREE_THIS_VOLATILE (decl); - TREE_SIDE_EFFECTS (new_decl) = TREE_SIDE_EFFECTS (decl); - TREE_READONLY (new_decl) = TREE_READONLY (decl); - TREE_ADDRESSABLE (new_decl) = TREE_ADDRESSABLE (decl); - DECL_SEEN_IN_BIND_EXPR_P (new_decl) = 1; - if ((TREE_CODE (decl) == PARM_DECL - || TREE_CODE (decl) == RESULT_DECL - || VAR_P (decl)) - && DECL_BY_REFERENCE (decl)) -DECL_BY_REFERENCE (new_decl) = 1; SET_DECL_VALUE_EXPR (new_decl, x); DECL_HAS_VAL
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
Re: [wwwdocs] Porting-to-14: Mention new pragma GCC Target behavior
On Thu, Apr 25, 2024 at 02:34:22PM +0200, Martin Jambor wrote: > when looking at a package build issue with GCC 14, Michal Jireš noted a > different behavior of pragma GCC Target. This snippet tries to describe > the gist of the problem. I have left it in the C section even though it > is not really C specific, but could not think of a good name for a new > section for it. Ideas (and any other suggestions for improvements) > welcome, of course. The change was more subtle. We used to define/undefine the ISA macros in C in GCC 13 and older as well, but only when using integrated preprocessor during compilation, so it didn't work that way with -save-temps or separate -E and -S/-c steps. While in C++ it behaved as if the define/undefines aren't done at all (they were done, but after preprocessing/lexing everything, so didn't affect anything). In GCC 14, it behaves in C++ the same as in C in older versions, and additionally they are defined/undefined also when using separate preprocessing, in both C and C++. 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
[PATCH] libcpp: Adjust __STDC_VERSION__ for C23
Hi! While the C23 standard isn't officially release yet, in 2011 we've changed __STDC_VERSION__ value for C11 already in the month in which the new __STDC_VERSION__ value has been finalized, so we want to change this now or wait until we implement all the C23 features? Note, seems Clang up to 17 also used 202000L for -std=c2x but Clang 18+ uses 202311L as specified in the latest C23 drafts. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-25 Jakub Jelinek * init.cc (cpp_init_builtins): Change __STDC_VERSION__ for C23 from 202000L to 202311L. * doc/cpp.texi (__STDC_VERSION__): Document 202311L value for -std=c23/-std=gnu23. --- libcpp/init.cc.jj 2024-01-03 12:07:27.552378565 +0100 +++ libcpp/init.cc 2024-04-24 23:54:32.613204591 +0200 @@ -594,7 +594,7 @@ cpp_init_builtins (cpp_reader *pfile, in _cpp_define_builtin (pfile, "__STDC_VERSION__ 199409L"); else if (CPP_OPTION (pfile, lang) == CLK_STDC23 || CPP_OPTION (pfile, lang) == CLK_GNUC23) -_cpp_define_builtin (pfile, "__STDC_VERSION__ 202000L"); +_cpp_define_builtin (pfile, "__STDC_VERSION__ 202311L"); else if (CPP_OPTION (pfile, lang) == CLK_STDC17 || CPP_OPTION (pfile, lang) == CLK_GNUC17) _cpp_define_builtin (pfile, "__STDC_VERSION__ 201710L"); --- gcc/doc/cpp.texi.jj 2024-01-03 11:38:23.583695795 +0100 +++ gcc/doc/cpp.texi2024-04-25 00:28:08.144182645 +0200 @@ -1886,8 +1886,8 @@ the 1999 revision of the C standard; the signifies the 2011 revision of the C standard; the value @code{201710L} signifies the 2017 revision of the C standard (which is otherwise identical to the 2011 version apart from correction of -defects). An unspecified value larger than @code{201710L} is used for -the experimental @option{-std=c23} and @option{-std=gnu23} modes. +defects). The value @code{202311L} is used for the experimental +@option{-std=c23} and @option{-std=gnu23} modes. This macro is not defined if the @option{-traditional-cpp} option is used, nor when compiling C++ or Objective-C@. Jakub
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
[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)) +
[PATCH] v2: DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
On Tue, Apr 23, 2024 at 07:07:17PM -0400, David Malcolm wrote: > That sounds like a better approach; thanks. Attached below. Tested by checking ../configure --disable-bootstrap --enable-languages=c --disable-multilib built trunk on void foo (int x) { __builtin_printf ("%ld\n", x); } testcase and looking for the URL in there, then repeating that after changing gcc/BASE-VER to 14.1.0 and again after changing it to 14.1.1 > > Still, what do you think we should do on the release branches > > (recommend to > > developers and check with the post-commit CI)? > > My hope is that the URL suffixes don't change: we shouldn't be adding > new command-line options on the release branches, and I'd hope that > texinfo doesn't change the generated anchors from run to run. Unfortunately that is not the case. *.opt files change all the time on release branches. Sure, not all the changes in there would cause *.urls changes, but many of them would. E.g. looking at 13 branch, r13-7022 r13-7130 r13-7169 r13-7276 r13-7336 r13-7415 r13-7518 r13-7528 r13-7650 r13-7651 r13-7728 r13-7794 r13-8039 r13-8040 r13-8223 r13-8350 r13-8351 r13-8357 r13-8419 r13-8545 commits changed the *.opt files. > > No regeneration of *.urls except before doing a new release > > candidate, > > or a different make goal that would grab html files from the web and > > regenerate against that? > > That sounds overcomplicated. > > If the anchors do change, it's fairly trivial to run "make regenerate- > opt-urls" locally, isn't it? I think the primary question is, do we want the *.urls checking CI on the release branches as well or only on the trunk? Given the xz backdoor, I think checking release branches for the regeneration hiccups (primarily for configure, Makefile etc.) is desirable. And with the patch posted here (or with what I've committed a week ago) on the release branches the default root will be initially identical but after some commits starts diverging. If we can live with some URLs being stale or misplaced until next release in the default case (if people provide their own root it will be always accurate), we don't need to do anything further (except perhaps enable the autoregen CI on 14 branch). 2024-04-24 Jakub Jelinek PR other/114738 * opts.cc (get_option_url): Revert 2024-04-17 changes. * gcc-urlifier.cc: Don't include diagnostic-core.h. (gcc_urlifier::make_doc_url): Revert 2024-04-17 changes. * configure.ac (documentation-root-url): On release branches append gcc-MAJOR.MINOR.0/ to the default DOCUMENTATION_ROOT_URL. * doc/install.texi (--with-documentation-root-url=): Document the change of the default. * configure: Regenerate. --- gcc/opts.cc.jj 2024-04-17 16:17:19.537749951 +0200 +++ gcc/opts.cc 2024-04-24 09:53:01.300399491 +0200 @@ -3761,19 +3761,7 @@ get_option_url (const diagnostic_context { label_text url_suffix = get_option_url_suffix (option_index, lang_mask); if (url_suffix.get ()) - { - char infix[32]; - /* On release branches, append to DOCUMENTATION_ROOT_URL the -subdirectory with documentation of the latest release made -from the branch. */ - if (BUILDING_GCC_MINOR != 0 && BUILDING_GCC_PATCHLEVEL <= 1U) - sprintf (infix, "gcc-%u.%u.0/", -BUILDING_GCC_MAJOR, BUILDING_GCC_MINOR); - else - infix[0] = '\0'; - return concat (DOCUMENTATION_ROOT_URL, infix, url_suffix.get (), -nullptr); - } + return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (), nullptr); } return nullptr; --- gcc/gcc-urlifier.cc.jj 2024-04-17 16:17:19.538749937 +0200 +++ gcc/gcc-urlifier.cc 2024-04-24 09:53:01.299399505 +0200 @@ -26,7 +26,6 @@ along with GCC; see the file COPYING3. #include "gcc-urlifier.h" #include "opts.h" #include "options.h" -#include "diagnostic-core.h" #include "selftest.h" namespace { @@ -209,16 +208,7 @@ gcc_urlifier::make_doc_url (const char * if (!doc_url_suffix) return nullptr; - char infix[32]; - /* On release branches, append to DOCUMENTATION_ROOT_URL the - subdirectory with documentation of the latest release made - from the branch. */ - if (BUILDING_GCC_MINOR != 0 && BUILDING_GCC_PATCHLEVEL <= 1U) -sprintf (infix, "gcc-%u.%u.0/", -BUILDING_GCC_MAJOR, BUILDING_GCC_MINOR); - else -infix[0] = '\0'; - return concat (DOCUMENTATION_ROOT_URL, infix, doc_url_suffix, nullptr); + return concat (DOCUMENTATION_ROOT_URL, doc_url_suffix, nullptr); } } // anonymous namespace --- gcc/configure.ac.jj 2024-04-17 16:09:49.697031449 +0200 +++ gcc/configure.ac2024-04-24 10:41:01.189687856 +0200 @@ -1088,9 +1088,16 @@ AC_ARG_WITH(docum
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 +struc
[PATCH] c++, v2: Fix constexpr evaluation of parameters passed by invisible reference [PR111284]
On Mon, Apr 15, 2024 at 02:19:36PM +0200, Jakub Jelinek wrote: > They weren't the same, one was trying to evaluate the convert_from_reference > with vc_glvalue, the other evaluates it without that and with vc_prvalue. > Now, I guess the > + /* Undo convert_for_arg_passing work here. */ > + if (TYPE_REF_P (TREE_TYPE (x)) > + && !same_type_p (type, TREE_TYPE (TREE_TYPE (x > + x = cp_fold_convert (build_reference_type (type), x); > part could be thrown away, given the other !same_type_p check (that one is > needed because adjust_temp_type can't deal with that), at least > when I remove that > GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 make check-g++ > RUNTESTFLAGS="dg.exp='constexpr-dtor* pr114426.C constexpr-111284.C > constexpr-lifetime7.C'" > still passes. I've now tested that version and it passed bootstrap/regtest on x86_64-linux and i686-linux. But as I said earlier, trying to tweak the patch further doesn't work on the constexpr-dtor{5,6}.C tests. 2024-04-23 Jakub Jelinek PR c++/111284 * constexpr.cc (cxx_bind_parameters_in_call): For PARM_DECLs with TREE_ADDRESSABLE types use vc_glvalue rather than vc_prvalue for cxx_eval_constant_expression and if it doesn't have the same type as it should, cast the reference type to reference to type before convert_from_reference and instead of adjust_temp_type take address of the arg, cast to reference to type and then convert_from_reference. (cxx_eval_constant_expression) : For lval case on parameters with TREE_ADDRESSABLE types lookup result in ctx->globals if possible. Otherwise if lookup in ctx->globals was successful for parameter with TREE_ADDRESSABLE type, recurse with vc_prvalue on the returned value. * g++.dg/cpp1z/constexpr-111284.C: New test. * g++.dg/cpp1y/constexpr-lifetime7.C: Expect one error on a different line. --- gcc/cp/constexpr.cc.jj 2024-02-13 10:29:57.979155641 +0100 +++ gcc/cp/constexpr.cc 2024-03-07 19:35:01.032412221 +0100 @@ -1877,13 +1877,18 @@ cxx_bind_parameters_in_call (const const x = build_address (x); } if (TREE_ADDRESSABLE (type)) - /* Undo convert_for_arg_passing work here. */ - x = convert_from_reference (x); - /* Normally we would strip a TARGET_EXPR in an initialization context -such as this, but here we do the elision differently: we keep the -TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm. */ - arg = cxx_eval_constant_expression (ctx, x, vc_prvalue, - non_constant_p, overflow_p); + { + /* Undo convert_for_arg_passing work here. */ + x = convert_from_reference (x); + arg = cxx_eval_constant_expression (ctx, x, vc_glvalue, + non_constant_p, overflow_p); + } + else + /* Normally we would strip a TARGET_EXPR in an initialization context + such as this, but here we do the elision differently: we keep the + TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm. */ + arg = cxx_eval_constant_expression (ctx, x, vc_prvalue, + non_constant_p, overflow_p); /* Check we aren't dereferencing a null pointer when calling a non-static member function, which is undefined behaviour. */ if (i == 0 && DECL_OBJECT_MEMBER_FUNCTION_P (fun) @@ -1909,7 +1914,16 @@ cxx_bind_parameters_in_call (const const { /* Make sure the binding has the same type as the parm. But only for constant args. */ - if (!TYPE_REF_P (type)) + if (TREE_ADDRESSABLE (type)) + { + if (!same_type_p (type, TREE_TYPE (arg))) + { + arg = build_fold_addr_expr (arg); + arg = cp_fold_convert (build_reference_type (type), arg); + arg = convert_from_reference (arg); + } + } + else if (!TYPE_REF_P (type)) arg = adjust_temp_type (type, arg); if (!TREE_CONSTANT (arg)) *non_constant_args = true; @@ -7499,9 +7513,19 @@ cxx_eval_constant_expression (const cons case PARM_DECL: if (lval && !TYPE_REF_P (TREE_TYPE (t))) - /* glvalue use. */; + { + /* glvalue use. */ + if (TREE_ADDRESSABLE (TREE_TYPE (t))) + if (tree v = ctx->global->get_value (t)) + r = v; + } else if (tree v = ctx->global->get_value (t)) - r = v; + { + r = v; + if (TREE_ADDRESSABLE (TREE_TYPE (t))) + r = cxx_eval_constant_expression (ctx, r, vc_prvalue, + non_constant_p, overflow_p); + } else if (l
[PATCH] i386: Avoid =,r,r andn double-word alternative for ia32 [PR114810]
Hi! As discussed in the PR, on ia32 with its 8 GPRs, where 1 is always fixed and other 2 often are as well having an alternative which needs 3 double-word registers is just too much for RA. The following patch splits that alternative into two, one with o is used even on ia32, but one with the 3x r is used just for -m64/-mx32. Tried to reduce the testcase further, but it wasn't easily possible. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-23 Jakub Jelinek PR target/114810 * config/i386/i386.md (*andn3_doubleword_bmi): Split the =,r,ro alternative into =,r,r enabled only for x64 and =,r,o. * g++.target/i386/pr114810.C: New test. --- gcc/config/i386/i386.md.jj 2024-04-15 14:25:58.203322878 +0200 +++ gcc/config/i386/i386.md 2024-04-23 12:15:47.171956091 +0200 @@ -12482,10 +12482,10 @@ (define_split }) (define_insn_and_split "*andn3_doubleword_bmi" - [(set (match_operand: 0 "register_operand" "=,r,r") + [(set (match_operand: 0 "register_operand" "=,,r,r") (and: - (not: (match_operand: 1 "register_operand" "r,0,r")) - (match_operand: 2 "nonimmediate_operand" "ro,ro,0"))) + (not: (match_operand: 1 "register_operand" "r,r,0,r")) + (match_operand: 2 "nonimmediate_operand" "r,o,ro,0"))) (clobber (reg:CC FLAGS_REG))] "TARGET_BMI" "#" @@ -12496,7 +12496,8 @@ (define_insn_and_split "*andn3_doub (parallel [(set (match_dup 3) (and:DWIH (not:DWIH (match_dup 4)) (match_dup 5))) (clobber (reg:CC FLAGS_REG))])] - "split_double_mode (mode, [0], 3, [0], [3]);") + "split_double_mode (mode, [0], 3, [0], [3]);" + [(set_attr "isa" "x64,*,*,*")]) (define_insn_and_split "*andn3_doubleword" [(set (match_operand:DWI 0 "register_operand") --- gcc/testsuite/g++.target/i386/pr114810.C.jj 2024-04-23 14:21:19.202613799 +0200 +++ gcc/testsuite/g++.target/i386/pr114810.C2024-04-23 14:24:22.813116589 +0200 @@ -0,0 +1,861 @@ +// PR target/114810 +// { dg-do compile { target { { { *-*-linux* } && ia32 } && c++17 } } } +// { dg-options "-mstackrealign -O2 -mbmi -fno-exceptions -fno-plt -march=x86-64 -w" } +// { dg-additional-options "-fpie" { target pie } } + +enum E1 { a, dp, b, jm, c, dq, d, mj, e, dr, f, jn, h, dt, j, nt, l, du, m, jo, n, dv, o, mk, p, dw, q, jp, s, dx, t, ol, u, dy, v, jq, w }; +enum dz { x, ml, y }; +struct ea { short g; } z, jr; +long long aa; +struct eb { ea ab; ea dp[]; }; +enum ac { }; +typedef enum { } nu; +struct ad { ac k; }; +unsigned ec (long); +struct ae; +int js (ae); +unsigned af (); +struct ed; +template < int ag > struct ee { using ah = ed[ag]; }; +template < int ag > struct array { typename ee < ag >::ah ai; ed & operator[] (int aj) { return ai[aj]; } }; +struct { void dp (...); } ak; +void ef (int); +template < typename al > struct jt { al & operator[] (short); }; +struct am { void operator= (bool); }; +struct an { am operator[] (unsigned); }; +template < typename, unsigned, unsigned >using eg = an; +struct ao; +struct ae { ae (ao *); }; +struct mm { mm (); mm (int); }; +enum ap { }; +enum eh { }; +bool aq, ju, ar, ei, nv, as, ej, at; +struct jv +{ + jv (eh au):dp (au) {} + jv (); + operator eh (); + unsigned av () + { +aq = dp & 7; +return dp * (aq ? : 4); + } + unsigned ek () + { +int aw; +bool mn = dp & 7; +aw = dp * (mn ? : 4); +return aw + 3 >> 2; + } + eh dp; +} ax, el, ay, jw, az, em, ba, om; +struct ed +{ + ed ():bb (), dp () {} + int bc () { return bb; } + jv en () { return (eh) dp; } + unsigned ek () + { +jv bd; +bd = (eh) dp; +return bd.ek (); + } + ap jx (); + unsigned bb:24; + int dp:8; +}; +struct be { short dp = 0; } bf, eo; +struct bg +{ + bg (); + bg (ed r) + { +dp.bh = r; +if (r.bc ()) + mo = true; +else + bi = true; + } + static bg ep (int); + bg (be); + struct { ed bh; } dp; + union { char mo:1; char bi:1; short bj = 0; }; +} jy, bk, eq, bl, mp, bm, er; +struct bn +{ + explicit bn (ed bo):bh (bo) {} + ed dp (); + ed bh; + be es; + char bj = 0; +}; +struct bp +{ + eg < int, 6, 4 > dp; +}; +jt < bg > bq; +jt < bn > definitions; +struct ao +{ + bp & br (); +}; +enum jz:short; +template < typename > using bs = ae; +ao *et (); +short bt, nw; +struct bu +{ + int dp; +}; +dz bv; +unsigned eu; +struct bw +{ + ac k; + unsigned dp; +} *bx; +bool ka (); +struct by +{ + bool dp; +}; +typedef enum +{ bz, ev } ca; +typedef enum +{ + mq, cb, ew, cc, kb, cd, ex, ce +} on; +typedef struct cf +{ + on jx; + char dp; + char cg; +} kc; +struct ch +{ + kc *dp; +}; +typedef enum +{ + ci, ey, cj, mr,
Re: [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
On Tue, Apr 23, 2024 at 11:40:55AM -0400, David Malcolm wrote: > > So, I think at least for the MAJOR.MINOR.0 releases we want to use > > URLs like above rather than the trunk ones and we can use the same > > process > > of updating *.opt.urls as well for that. > > Would it make sense to instead update the default value in > gcc/configure.ac for DOCUMENTATION_ROOT_URL when branching or > releasing, from https://gcc.gnu.org/onlinedocs/ to > https://gcc.gnu.org/onlinedocs/gcc-MAJOR-MINOR.0/ > > ? > > Before this patch the DOCUMENTATION_ROOT_URL expresses the location of > a built texinfo html tree of docs, and the url suffixes express the > path within that tree. > > As the patch is written, if a distributor overrides --with- > documentation-root-url= at configure time, then they need to mirror the > structure of our website on their website, which seems like a burden. Sure, that is doable (of course, it shouldn't be done by updating gcc/configure.ac but by adjusting the default in there based on gcc_version, I'll post a patch tomorrow). Still, what do you think we should do on the release branches (recommend to developers and check with the post-commit CI)? No regeneration of *.urls except before doing a new release candidate, or a different make goal that would grab html files from the web and regenerate against that? Jakub
[committed] testsuite: Adjust testsuite expectations for diagnostic spelling fixes
Hi! The nullability-00.m* tests unfortunately check the exact spelling of the diagnostics I've changed earlier today. Tested on x86_64-linux and i686-linux, committed to trunk as obvious. 2024-04-23 Jakub Jelinek * objc.dg/attributes/nullability-00.m: Adjust expected diagnostic spelling: recognised -> recognized. * obj-c++.dg/attributes/nullability-00.mm: Likewise. --- gcc/testsuite/objc.dg/attributes/nullability-00.m.jj2020-11-13 13:50:23.365551538 +0100 +++ gcc/testsuite/objc.dg/attributes/nullability-00.m 2024-04-23 17:37:18.978721522 +0200 @@ -8,7 +8,7 @@ __attribute__((objc_nullability("unspeci __attribute__((objc_nullability("nullable"))) id c; __attribute__((objc_nullability("nonnull"))) id d; __attribute__((objc_nullability("resettable"))) id e; -__attribute__((objc_nullability("nonsense"))) id e_3; /* { dg-error {'objc_nullability' attribute argument '"nonsense"' is not recognised} } */ +__attribute__((objc_nullability("nonsense"))) id e_3; /* { dg-error {'objc_nullability' attribute argument '"nonsense"' is not recognized} } */ __attribute__((objc_nullability(noGoingToWork))) id e_4; /* { dg-error {'noGoingToWork' undeclared here} } */ @interface MyRoot --- gcc/testsuite/obj-c++.dg/attributes/nullability-00.mm.jj2020-11-13 13:50:23.361551584 +0100 +++ gcc/testsuite/obj-c++.dg/attributes/nullability-00.mm 2024-04-23 17:37:35.717500341 +0200 @@ -8,7 +8,7 @@ __attribute__((objc_nullability("unspeci __attribute__((objc_nullability("nullable"))) id c; __attribute__((objc_nullability("nonnull"))) id d; __attribute__((objc_nullability("resettable"))) id e; -__attribute__((objc_nullability("nonsense"))) id e_3; /* { dg-error {'objc_nullability' attribute argument '"nonsense"' is not recognised} } */ +__attribute__((objc_nullability("nonsense"))) id e_3; /* { dg-error {'objc_nullability' attribute argument '"nonsense"' is not recognized} } */ __attribute__((objc_nullability(noGoingToWork))) id e_4; /* { dg-error {'noGoingToWork' was not declared in this scope} } */ @interface MyRoot Jakub
Re: [PATCH] libbacktrace: Avoid GNU ld --compress-debug-sections=zlib-gabi
On Tue, Apr 23, 2024 at 04:18:49PM +0200, Jakub Jelinek wrote: > Then you have two tests (ctestg and ctesta) doing exactly the same thing, > that can't be right. > I guess it might be fine to use zlib when it is an alias to zlib-gabi, > but zlib-gnu shouldn't be replaced. > > I must say I don't really understand the patch though, because configury > checks > AC_CACHE_CHECK([whether --compress-debug-sections is supported], > [libgo_cv_ld_compress], > [LDFLAGS_hold=$LDFLAGS > LDFLAGS="$LDFLAGS -Wl,--compress-debug-sections=zlib-gnu" > AC_LINK_IFELSE([AC_LANG_PROGRAM(,)], > [libgo_cv_ld_compress=yes], > [libgo_cv_ld_compress=no]) > LDFLAGS=$LDFLAGS_hold]) > AM_CONDITIONAL(HAVE_COMPRESSED_DEBUG, test "$libgo_cv_ld_compress" = yes) > > So, if Solaris doesn't support --compress-debug-sections=zlib-gnu, it > shouldn't be tested. Or does it support zlib-gnu and zlib? What we could do is drop the HAVE_COMPRESSED_DEBUG stuff altogether, and instead similarly how we have HAVE_COMPRESSED_DEBUG_ZSTD have HAVE_COMPRESSED_DEBUG_{ZLIB,ZLIB_GABI,ZLIB_GNU} and for each of those if linker supports them test with that corresponding flag. Jakub
Re: [PATCH] libbacktrace: Avoid GNU ld --compress-debug-sections=zlib-gabi
On Tue, Apr 23, 2024 at 04:05:07PM +0200, Rainer Orth wrote: > I noticed that libbacktrace make check FAILs on Solaris with the native > ld already when building the tests: > > libtool: link: /var/gcc/regression/master/11.4-gcc/build/./gcc/xgcc > -B/var/gcc/r > egression/master/11.4-gcc/build/./gcc/ -B/vol/gcc/sparc-sun-solaris2.11/bin/ > -B/ > vol/gcc/sparc-sun-solaris2.11/lib/ -isystem > /vol/gcc/sparc-sun-solaris2.11/inclu > de -isystem /vol/gcc/sparc-sun-solaris2.11/sys-include -fchecking=1 > -funwind-tab > les -frandom-seed=ctesta_alloc -W -Wall -Wwrite-strings -Wstrict-prototypes > -Wmi > ssing-prototypes -Wold-style-definition -Wmissing-format-attribute > -Wcast-qual - > Werror -g -g -O2 -Wl,--compress-debug-sections=zlib-gabi -o ctesta_alloc > ctesta_ > alloc-btest.o ctesta_alloc-testlib.o ./.libs/libbacktrace_alloc.a > ld: fatal: unrecognized '--compress-debug-sections' cmp-type: zlib-gabi > collect2: error: ld returned 1 exit status > make[1]: *** [Makefile:1379: ctesta_alloc] Error 1 > > Solaris ld only supports --compress-debug-sections=zlib, while GNU ld > allows zlib-gabi as an alias for zlib. gold is the same, it seems, > while lld doesn't support zlib-gabi at all. > > Therefore the patch uses zlib instead. Then you have two tests (ctestg and ctesta) doing exactly the same thing, that can't be right. I guess it might be fine to use zlib when it is an alias to zlib-gabi, but zlib-gnu shouldn't be replaced. I must say I don't really understand the patch though, because configury checks AC_CACHE_CHECK([whether --compress-debug-sections is supported], [libgo_cv_ld_compress], [LDFLAGS_hold=$LDFLAGS LDFLAGS="$LDFLAGS -Wl,--compress-debug-sections=zlib-gnu" AC_LINK_IFELSE([AC_LANG_PROGRAM(,)], [libgo_cv_ld_compress=yes], [libgo_cv_ld_compress=no]) LDFLAGS=$LDFLAGS_hold]) AM_CONDITIONAL(HAVE_COMPRESSED_DEBUG, test "$libgo_cv_ld_compress" = yes) So, if Solaris doesn't support --compress-debug-sections=zlib-gnu, it shouldn't be tested. Or does it support zlib-gnu and zlib? Jakub
[committed] Further spelling fixes in translatable strings
On Tue, Apr 23, 2024 at 11:32:08AM +0100, Jonathan Wakely wrote: > On Mon, 22 Apr 2024 at 22:30, Jakub Jelinek wrote: > Yup: > https://gcc.gnu.org/codingconventions.html#Spelling > > That spelling is explicitly mentioned at the link above, so they > should be "ize" really. Ok. I've committed that patch plus the following as obvious too. I see various similar cases in m2 and rust FEs where they don't make it into gcc/po/gcc.pot, guess those would be nice to get fixed too, end best even find out how to make those messages translatable. Talking about e.g. m2/gm2-compiler/M2Quads.mod:'%a unrecognised builtin constant', Id) | m2/gm2-compiler/M2Quads.mod: InternalError ('unrecognised value') ... rust/parse/rust-parse-impl.h: "unrecognised token %qs for item in trait", rust/parse/rust-parse-impl.h:"unrecognised token %qs for item in inherent impl", but none of that making it into gcc.pot. 2024-04-23 Jakub Jelinek * config/darwin.opt (init): Spelling fix: initialiser -> initializer. gcc/c-family/ * c-attribs.cc (handle_objc_nullability_attribute): Spelling fix: recognised -> recognized. gcc/m2/ * lang.opt (fdef=, fmod=): Spelling fix: recognise -> recognize. --- gcc/config/darwin.opt.jj2024-01-03 11:51:43.137570111 +0100 +++ gcc/config/darwin.opt 2024-04-23 10:34:56.406196449 +0200 @@ -211,7 +211,7 @@ Driver RejectNegative Separate init Driver RejectNegative Separate --init The symbol will be used as the first initialiser for a dylib. +-init The symbol will be used as the first initializer for a dylib. install_name Driver RejectNegative Separate --- gcc/c-family/c-attribs.cc.jj2024-01-09 15:35:43.626688356 +0100 +++ gcc/c-family/c-attribs.cc 2024-04-23 10:30:23.458043442 +0200 @@ -6244,7 +6244,7 @@ handle_objc_nullability_attribute (tree || strcmp (TREE_STRING_POINTER (val), "resettable") == 0)) *no_add_attrs = false; /* OK */ else if (val != error_mark_node) -error ("%qE attribute argument %qE is not recognised", name, val); +error ("%qE attribute argument %qE is not recognized", name, val); return NULL_TREE; } --- gcc/m2/lang.opt.jj 2024-04-23 08:30:59.312249288 +0200 +++ gcc/m2/lang.opt 2024-04-23 10:34:13.118806549 +0200 @@ -96,7 +96,7 @@ turn on tracing of procedure line number fdef= Modula-2 Joined -recognise the specified suffix as a definition module filename +recognize the specified suffix as a definition module filename fdump-system-exports Modula-2 @@ -172,7 +172,7 @@ compile all implementation modules and p fmod= Modula-2 Joined -recognise the specified suffix as implementation and module filenames +recognize the specified suffix as implementation and module filenames fnil Modula-2 @@ -278,7 +278,7 @@ static-libgm2 Driver Link the standard Modula-2 libraries statically in the compilation. -; Here are C options that we also recognise, either within the compiler +; Here are C options that we also recognize, either within the compiler ; or to build the preprocessor command lines. Wall Jakub
[PATCH] Spelling fixes for translatable strings
Hi! I've run aspell on gcc.pot (just quickly skimming, so pressing I key hundreds of times and just stopping when I catch something that looks like a misspelling). I plan to commit this tomorrow as obvious unless somebody finds some issues in it, you know, I'm not a native English speaker. Yes, I know favour is valid UK spelling, but we spell the US way I think. I've left some *ise* -> *ize* cases (recognise, initialise), those had too many hits, though in translatable strings just 4, so maybe worth changing too: msgid "recognise the specified suffix as a definition module filename" msgid "recognise the specified suffix as implementation and module filenames" "initialiser for a dylib." msgid "%qE attribute argument %qE is not recognised" 2024-04-22 Jakub Jelinek * config/epiphany/epiphany.opt (may-round-for-trunc): Spelling fix: floatig -> floating. * config/riscv/riscv.opt (mcsr-check): Spelling fix: CRS -> CSR. * params.opt (-param=ipa-cp-profile-count-base=): Spelling fix: frequncy -> frequency. gcc/c-family/ * c.opt (Wstrict-flex-arrays): Spelling fix: inproper -> improper. gcc/cp/ * parser.cc (cp_parser_using_declaration): Spelling fix: favour -> favor. gcc/m2/ * lang.opt (fuse-list=): Spelling fix: finalializations -> finalizations. --- gcc/config/epiphany/epiphany.opt.jj 2024-01-03 11:51:47.489509710 +0100 +++ gcc/config/epiphany/epiphany.opt2024-04-22 22:53:56.581549745 +0200 @@ -105,7 +105,7 @@ Enum(attr_fp_mode) String(int) Value(FP_ may-round-for-trunc Target Mask(MAY_ROUND_FOR_TRUNC) -A floatig point to integer truncation may be replaced with rounding to save mode switching. +A floating point to integer truncation may be replaced with rounding to save mode switching. mvect-double Target Mask(VECT_DOUBLE) --- gcc/config/riscv/riscv.opt.jj 2024-04-09 08:12:29.379449739 +0200 +++ gcc/config/riscv/riscv.opt 2024-04-22 22:52:06.780080712 +0200 @@ -152,7 +152,7 @@ required to materialize symbol addresses mcsr-check Target Var(riscv_mcsr_check) Init(0) -Enable the CSR checking for the ISA-dependent CRS and the read-only CSR. +Enable the CSR checking for the ISA-dependent CSR and the read-only CSR. The ISA-dependent CSR are only valid when the specific ISA is set. The read-only CSR can not be written by the CSR instructions. --- gcc/cp/parser.cc.jj 2024-04-22 18:12:35.326282135 +0200 +++ gcc/cp/parser.cc2024-04-22 23:14:11.928605442 +0200 @@ -22431,7 +22431,7 @@ cp_parser_using_declaration (cp_parser* if (access_declaration_p && errorcount == oldcount) warning_at (diag_token->location, OPT_Wdeprecated, "access declarations are deprecated " - "in favour of using-declarations; " + "in favor of using-declarations; " "suggestion: add the % keyword"); return true; --- gcc/m2/lang.opt.jj 2024-04-03 09:58:33.538770735 +0200 +++ gcc/m2/lang.opt 2024-04-22 22:47:59.120533842 +0200 @@ -260,7 +260,7 @@ optimize non var unbounded parameters by fuse-list= Modula-2 Joined -orders the initialization/finalializations for scaffold-static or force linking of modules if scaffold-dynamic +orders the initialization/finalizations for scaffold-static or force linking of modules if scaffold-dynamic fversion Modula-2 --- gcc/c-family/c.opt.jj 2024-04-15 10:16:58.571245875 +0200 +++ gcc/c-family/c.opt 2024-04-22 22:41:48.188705755 +0200 @@ -1320,7 +1320,7 @@ C ObjC C++ ObjC++ LangEnabledBy(C ObjC C Wstrict-flex-arrays C C++ Var(warn_strict_flex_arrays) Warning -Warn about inproper usages of flexible array members +Warn about improper usages of flexible array members according to the level of -fstrict-flex-arrays. Wstrict-null-sentinel --- gcc/params.opt.jj 2024-01-03 11:51:22.563855655 +0100 +++ gcc/params.opt 2024-04-22 23:06:47.466804309 +0200 @@ -263,7 +263,7 @@ Maximum size of a list of values associa -param=ipa-cp-profile-count-base= Common Joined UInteger Var(param_ipa_cp_profile_count_base) Init(10) IntegerRange(0, 100) Param Optimization -When using profile feedback, use the edge at this percentage position in frequncy histogram as the bases for IPA-CP heuristics. +When using profile feedback, use the edge at this percentage position in frequency histogram as the bases for IPA-CP heuristics. -param=ipa-jump-function-lookups= Common Joined UInteger Var(param_ipa_jump_function_lookups) Init(8) Param Optimization Jakub
Re: [PATCH] testsuite: prune -freport-bug output
On Fri, Apr 19, 2024 at 05:54:11PM -0400, Marek Polacek wrote: > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > I can defer to 15 if needed, of course. > > -- >8 -- > When the compiler defaults to -freport-bug, a few dg-ice tests fail > with: > > Excess errors: > Preprocessed source stored into /tmp/cc6hldZ0.out file, please attach this to > your bugreport. > > We could add -fno-report-bug to those tests. But it seems to me that a > better fix would be to prune the "Preprocessed source stored..." message > in prune_gcc_output. > > gcc/testsuite/ChangeLog: > > * lib/prune.exp (prune_gcc_output): Also prune -freport-bug output. LGTM. > --- a/gcc/testsuite/lib/prune.exp > +++ b/gcc/testsuite/lib/prune.exp > @@ -51,6 +51,7 @@ proc prune_gcc_output { text } { > regsub -all "(^|\n)\[^\n\]*: re(compiling|linking)\[^\n\]*" $text "" text > regsub -all "(^|\n)Please submit.*instructions\[^\n\]*" $text "" text > regsub -all "(^|\n)\[0-9\]\[0-9\]* errors\." $text "" text > +regsub -all "(^|\n)Preprocessed.*bugreport\[^\n\]*" $text "" text > > # Diagnostic inclusion stack > regsub -all "(^|\n)(In file)?\[ \]+included from \[^\n\]*" $text "" text > > base-commit: d86472a6f041ccf3d1be0cf6bb15d1e0ad8f6dbe > -- > 2.44.0 Jakub
[PATCH] c++: Copy over DECL_DISREGARD_INLINE_LIMITS flag to inheriting ctors [PR114784]
Hi! The following testcase is rejected with error: inlining failed in call to 'always_inline' '...': call is unlikely and code size would grow errors. The problem is that starting with the r14-2149 change we try to copy most of the attributes from the inherited to inheriting ctor, but don't copy associated flags that decl_attributes sets. Now, the other clone_attrs user, cp/optimize.cc (maybe_clone_body) copies over DECL_COMDAT (clone) = DECL_COMDAT (fn); DECL_WEAK (clone) = DECL_WEAK (fn); if (DECL_ONE_ONLY (fn)) cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone)); DECL_USE_TEMPLATE (clone) = DECL_USE_TEMPLATE (fn); DECL_EXTERNAL (clone) = DECL_EXTERNAL (fn); DECL_INTERFACE_KNOWN (clone) = DECL_INTERFACE_KNOWN (fn); DECL_NOT_REALLY_EXTERN (clone) = DECL_NOT_REALLY_EXTERN (fn); DECL_VISIBILITY (clone) = DECL_VISIBILITY (fn); DECL_VISIBILITY_SPECIFIED (clone) = DECL_VISIBILITY_SPECIFIED (fn); DECL_DLLIMPORT_P (clone) = DECL_DLLIMPORT_P (fn); DECL_DISREGARD_INLINE_LIMITS (clone) = DECL_DISREGARD_INLINE_LIMITS (fn); The following patch just copies DECL_DISREGARD_INLINE_LIMITS to fix this exact bug, not really sure which other flags should be copied and which shouldn't. Plus there are tons of other flags, some of which might need to be copied too, some of which might not, perhaps in both places, like: DECL_UNINLINABLE, maybe DECL_PRESERVE_P, TREE_USED, maybe DECL_USER_ALIGN/DECL_ALIGN, maybe DECL_WEAK, maybe DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT, DECL_NO_LIMIT_STACK. TREE_READONLY, DECL_PURE_P, TREE_THIS_VOLATILE (for const, pure and noreturn attributes) probably makes no sense, DECL_IS_RETURNS_TWICE neither (returns_twice ctor?). What about TREE_NOTHROW? DECL_FUNCTION_SPECIFIC_OPTIMIZATION, DECL_FUNCTION_SPECIFIC_TARGET... Anyway, another problem is that if inherited_ctor is a TEMPLATE_DECL, as also can be seen in the using D::D; case in the testcase, then DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor)); attempts to copy the attributes from the TEMPLATE_DECL which doesn't have them. The following patch copies them from STRIP_TEMPLATE (inherited_ctor) which does. E.g. DECL_DECLARED_CONSTEXPR_P works fine as the macro itself uses STRIP_TEMPLATE too, but not 100% sure about other macros used on inherited_ctor earlier. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Do you want to copy other flags (and which ones and in which places), is that ok to be deferred till stage 1? 2024-04-22 Jakub Jelinek PR c++/114784 * method.cc (implicitly_declare_fn): Call clone_attrs on DECL_ATTRIBUTES on STRIP_TEMPLATE (inherited_ctor) rather than inherited_ctor. Also copy DECL_DISREGARD_INLINE_LIMITS flag from it. * g++.dg/cpp0x/inh-ctor39.C: New test. --- gcc/cp/method.cc.jj 2024-04-20 00:02:56.702387748 +0200 +++ gcc/cp/method.cc2024-04-22 12:51:36.395722484 +0200 @@ -3307,8 +3307,11 @@ implicitly_declare_fn (special_function_ /* Copy constexpr from the inherited constructor even if the inheriting constructor doesn't satisfy the requirements. */ constexpr_p = DECL_DECLARED_CONSTEXPR_P (inherited_ctor); + tree inherited_ctor_fn = STRIP_TEMPLATE (inherited_ctor); /* Also copy any attributes. */ - DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor)); + DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor_fn)); + DECL_DISREGARD_INLINE_LIMITS (fn) + = DECL_DISREGARD_INLINE_LIMITS (inherited_ctor_fn); } /* Add the "this" parameter. */ --- gcc/testsuite/g++.dg/cpp0x/inh-ctor39.C.jj 2024-04-22 13:02:10.490836357 +0200 +++ gcc/testsuite/g++.dg/cpp0x/inh-ctor39.C 2024-04-22 13:01:50.088122255 +0200 @@ -0,0 +1,55 @@ +// PR c++/114784 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-O2" } + +template +struct A { + [[gnu::always_inline]] A (int t) { foo ().bar (t, {}); } + [[gnu::always_inline]] A (long long t) { foo ().bar (t, {}); } + T foo (); +}; + +struct B : A { + using A::A; + [[gnu::always_inline]] B (long long v) : A (v) {} + template + void bar (T &&, int); + char b; +}; + +struct C { + C (int v) : a(v) { } + C (long long v) : a(v) { } + B a; +}; + +static C +baz () +{ + C x(0); + C y(0LL); + return 0; +} + +[[gnu::cold]] int +qux () +{ + baz (); + return 0; +} + +template +struct D { + template + [[gnu::always_inline]] D (T) { d = sizeof (T); } + D(); + int d; +}; +template +struct E : D { + using D::D; +}; + +E c = {}; +E d = 1; +E e = 1.0; Jakub
[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; + r
[PATCH] libstdc++: Workaround kernel-headers on s390x-linux
Hi! We see FAIL: 17_intro/headers/c++1998/all_attributes.cc (test for excess errors) FAIL: 17_intro/headers/c++2011/all_attributes.cc (test for excess errors) FAIL: 17_intro/headers/c++2014/all_attributes.cc (test for excess errors) FAIL: 17_intro/headers/c++2017/all_attributes.cc (test for excess errors) FAIL: 17_intro/headers/c++2020/all_attributes.cc (test for excess errors) FAIL: 17_intro/names.cc -std=gnu++17 (test for excess errors) on s390x-linux. The first 5 are due to kernel-headers not using uglified attribute names, where contains __attribute__((packed, aligned(4))) I've filed a downstream bugreport for this in https://bugzilla.redhat.com/show_bug.cgi?id=2276084 (not really sure where to report kernel-headers issues upstream), while the last one is due to from glibc containing: #ifdef __USE_MISC # define __ctx(fld) fld #else # define __ctx(fld) __ ## fld #endif ... typedef union { double __ctx(d); float __ctx(f); } fpreg_t; and g++ predefining -D_GNU_SOURCE which implies define __USE_MISC. The following patch adds a workaround for this on the libstdc++ testsuite side, ok for trunk? 2024-04-22 Jakub Jelinek * testsuite/17_intro/names.cc (d, f): Undefine on s390*-linux*. * testsuite/17_intro/headers/c++1998/all_attributes.cc (packed): Don't define on s390. * testsuite/17_intro/headers/c++2011/all_attributes.cc (packed): Likewise. * testsuite/17_intro/headers/c++2014/all_attributes.cc (packed): Likewise. * testsuite/17_intro/headers/c++2017/all_attributes.cc (packed): Likewise. * testsuite/17_intro/headers/c++2020/all_attributes.cc (packed): Likewise. --- libstdc++-v3/testsuite/17_intro/names.cc.jj 2024-02-02 22:13:29.575359165 +0100 +++ libstdc++-v3/testsuite/17_intro/names.cc2024-04-22 16:54:35.696710752 +0200 @@ -270,6 +270,12 @@ #undef u #endif +#if defined (__linux__) && defined (__s390__) +// defines fpreg_t::d and fpreg_t::f +#undef d +#undef f +#endif + #if defined (__linux__) && defined (__sparc__) #undef y #endif --- libstdc++-v3/testsuite/17_intro/headers/c++1998/all_attributes.cc.jj 2024-01-03 12:08:09.603789437 +0100 +++ libstdc++-v3/testsuite/17_intro/headers/c++1998/all_attributes.cc 2024-04-22 17:11:04.734990469 +0200 @@ -29,7 +29,11 @@ # define noreturn 1 # define visibility 1 #endif +#ifndef __s390__ +// kernel-headers uses __attribute__((packed,aligned(4))) on +// S390. #define packed 1 +#endif #define pure 1 // glibc's sysdeps/unix/sysv/linux/arm/sys/ucontext.h uses this on ARM. #ifndef __arm__ --- libstdc++-v3/testsuite/17_intro/headers/c++2011/all_attributes.cc.jj 2024-04-22 17:11:20.028805205 +0200 +++ libstdc++-v3/testsuite/17_intro/headers/c++2011/all_attributes.cc 2024-04-22 17:12:01.531302456 +0200 @@ -29,7 +29,11 @@ # define visibility 1 #endif #define no_unique_address 1 +#ifndef __s390__ +// kernel-headers uses __attribute__((packed,aligned(4))) on +// S390. #define packed 1 +#endif #define pure 1 // glibc's sysdeps/unix/sysv/linux/arm/sys/ucontext.h uses this on ARM. #ifndef __arm__ --- libstdc++-v3/testsuite/17_intro/headers/c++2014/all_attributes.cc.jj 2024-01-03 12:08:09.556790095 +0100 +++ libstdc++-v3/testsuite/17_intro/headers/c++2014/all_attributes.cc 2024-04-22 17:12:20.630071099 +0200 @@ -29,7 +29,11 @@ # define visibility 1 #endif #define no_unique_address 1 +#ifndef __s390__ +// kernel-headers uses __attribute__((packed,aligned(4))) on +// S390. #define packed 1 +#endif #define pure 1 // glibc's sysdeps/unix/sysv/linux/arm/sys/ucontext.h uses this on ARM. #ifndef __arm__ --- libstdc++-v3/testsuite/17_intro/headers/c++2017/all_attributes.cc.jj 2024-01-03 12:08:09.506790795 +0100 +++ libstdc++-v3/testsuite/17_intro/headers/c++2017/all_attributes.cc 2024-04-22 17:12:38.751851577 +0200 @@ -28,7 +28,11 @@ # define visibility 1 #endif #define no_unique_address 1 +#ifndef __s390__ +// kernel-headers uses __attribute__((packed,aligned(4))) on +// S390. #define packed 1 +#endif #define pure 1 // glibc's sysdeps/unix/sysv/linux/arm/sys/ucontext.h uses this on ARM. #ifndef __arm__ --- libstdc++-v3/testsuite/17_intro/headers/c++2020/all_attributes.cc.jj 2024-01-03 12:08:09.502790851 +0100 +++ libstdc++-v3/testsuite/17_intro/headers/c++2020/all_attributes.cc 2024-04-22 17:12:56.043642110 +0200 @@ -27,7 +27,11 @@ # define cold 1 # define visibility 1 #endif +#ifndef __s390__ +// kernel-headers uses __attribute__((packed,aligned(4))) on +// S390. #define packed 1 +#endif #define pure 1 // glibc's sysdeps/unix/sysv/linux/arm/sys/ucontext.h uses this on ARM. #ifndef __arm__ Jakub
[committed] i386: Fix up *avx2_eq3 constraints [PR114783]
Hi! The r14-4456 change (part of APX EGPR support) seems to have mistakenly changed in the @@ -16831,7 +16831,7 @@ (define_insn "*avx2_eq3" [(set (match_operand:VI_256 0 "register_operand" "=x") (eq:VI_256 (match_operand:VI_256 1 "nonimmediate_operand" "%x") - (match_operand:VI_256 2 "nonimmediate_operand" "xm")))] + (match_operand:VI_256 2 "nonimmediate_operand" "jm")))] "TARGET_AVX2 && !(MEM_P (operands[1]) && MEM_P (operands[2]))" "vpcmpeq\t{%2, %1, %0|%0, %1, %2}" [(set_attr "type" "ssecmp") hunk the xm constraint to jm, while in many other spots it changed correctly xm to xjm. The instruction doesn't require the last operand to be in memory, it can handle 3 256-bit registers just fine, just it is a VEX only encoded instruction and so can't allow APX EGPR regs in the memory operand. The following patch fixes it, so that we don't force one of the == operands into memory all the time. Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk as obvious. 2024-04-20 Jakub Jelinek PR target/114783 * config/i386/sse.md (*avx2_eq3): Change last operand's constraint from "jm" to "xjm". * gcc.target/i386/avx2-pr114783.c: New test. --- gcc/config/i386/sse.md.jj 2024-04-10 09:55:05.849877708 +0200 +++ gcc/config/i386/sse.md 2024-04-19 19:37:34.110320387 +0200 @@ -17029,7 +17029,7 @@ (define_insn "*avx2_eq3" [(set (match_operand:VI_256 0 "register_operand" "=x") (eq:VI_256 (match_operand:VI_256 1 "nonimmediate_operand" "%x") - (match_operand:VI_256 2 "nonimmediate_operand" "jm")))] + (match_operand:VI_256 2 "nonimmediate_operand" "xjm")))] "TARGET_AVX2 && !(MEM_P (operands[1]) && MEM_P (operands[2]))" "vpcmpeq\t{%2, %1, %0|%0, %1, %2}" [(set_attr "type" "ssecmp") --- gcc/testsuite/gcc.target/i386/avx2-pr114783.c.jj2024-04-19 19:28:50.216644029 +0200 +++ gcc/testsuite/gcc.target/i386/avx2-pr114783.c 2024-04-19 19:45:02.943054262 +0200 @@ -0,0 +1,12 @@ +/* PR target/114783 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -mavx2 -mno-avx512f -masm=att" } */ +/* { dg-final { scan-assembler "vpcmpeqd\[ \\t\]+%ymm\[01\], %ymm\[01\], %ymm0" } } */ + +typedef int V __attribute__((vector_size (32))); + +V +foo (V x, V y) +{ + return x == y; +} Jakub
[committed] internal-fn: Fix up expand_arith_overflow [PR114753]
Hi! During backporting I've noticed I've missed one return spot for the restoration of the original flag_trapv flag value. Tested on x86_64-linux, committed to trunk as obvious. Sorry. 2024-04-19 Jakub Jelinek PR middle-end/114753 * internal-fn.cc (expand_arith_overflow): Add one missing restore of flag_trapv before return. --- gcc/internal-fn.cc.jj 2024-04-18 09:45:08.079695875 +0200 +++ gcc/internal-fn.cc 2024-04-19 18:11:51.202204402 +0200 @@ -2793,6 +2793,7 @@ expand_arith_overflow (enum tree_code co case PLUS_EXPR: expand_addsub_overflow (loc, code, lhs, arg0, arg1, unsr_p, unsr_p, unsr_p, false, NULL); + flag_trapv = save_flag_trapv; return; case MULT_EXPR: expand_mul_overflow (loc, lhs, arg0, arg1, unsr_p, Jakub
[PATCH] c-family: Allow arguments with NULLPTR_TYPE as sentinels [PR114780]
Hi! While in C++ the ellipsis argument conversions include "An argument that has type cv std::nullptr_t is converted to type void*" in C23 a nullptr_t argument is not promoted in any way, but va_arg description says: "the type of the next argument is nullptr_t and type is a pointer type that has the same representation and alignment requirements as a pointer to a character type." So, while in C++ check_function_sentinel will never see NULLPTR_TYPE, for C23 it can see that and currently we incorrectly warn about those. The only question is whether we should warn on any argument with nullptr_t type or just about nullptr (nullptr_t argument with integer_zerop value). Through undefined behavior guess one could pass non-NULL pointer that way, say by union { void *p; nullptr_t q; } u; u.p = and pass u.q to ..., but valid code should always pass something that will read as (char *) 0 when read using va_arg (ap, char *), so I think it is better not to warn rather than warn in those cases. Note, clang seems to pass (void *)0 rather than expression of nullptr_t type to ellipsis in C23 mode as if it did the C++ ellipsis argument conversions, in that case guess not warning about that would be even safer, but what GCC does I think follows the spec more closely, even when in a valid program one shouldn't be able to observe the difference. Ok for trunk and later 13.3 if it passes bootstrap/regtest (so far just checked on the sentinel related C/C++ tests)? 2024-04-19 Jakub Jelinek PR c/114780 * c-common.cc (check_function_sentinel): Allow as sentinel any argument of NULLPTR_TYPE. * gcc.dg/format/sentinel-2.c: New test. --- gcc/c-family/c-common.cc.jj 2024-03-27 19:39:17.968676626 +0100 +++ gcc/c-family/c-common.cc2024-04-19 12:58:01.577985800 +0200 @@ -5783,6 +5783,7 @@ check_function_sentinel (const_tree fnty sentinel = fold_for_warn (argarray[nargs - 1 - pos]); if ((!POINTER_TYPE_P (TREE_TYPE (sentinel)) || !integer_zerop (sentinel)) + && TREE_CODE (TREE_TYPE (sentinel)) != NULLPTR_TYPE /* Although __null (in C++) is only an integer we allow it nevertheless, as we are guaranteed that it's exactly as wide as a pointer, and we don't want to force --- gcc/testsuite/gcc.dg/format/sentinel-2.c.jj 2024-04-19 12:57:57.431043948 +0200 +++ gcc/testsuite/gcc.dg/format/sentinel-2.c2024-04-19 12:58:39.020460785 +0200 @@ -0,0 +1,21 @@ +/* PR c/114780 */ +/* { dg-do compile } */ +/* { dg-options "-std=c23 -Wformat" } */ + +#include + +[[gnu::sentinel]] void foo (int, ...); +[[gnu::sentinel]] void bar (...); + +void +baz (nullptr_t p) +{ + foo (1, 2, nullptr); + foo (3, 4, 5, p); + bar (nullptr); + bar (p); + foo (6, 7, 0); // { dg-warning "missing sentinel in function call" } + bar (0); // { dg-warning "missing sentinel in function call" } + foo (8, 9, NULL); + bar (NULL); +} Jakub
Re: [PATCH] rtlanal: Fix set_noop_p for volatile loads or stores [PR114768]
On Fri, Apr 19, 2024 at 12:23:03PM +0200, Thomas Schwinge wrote: > On 2024-04-19T08:24:03+0200, Jakub Jelinek wrote: > > --- gcc/testsuite/gcc.dg/pr114768.c.jj 2024-04-18 15:37:49.139433678 > > +0200 > > +++ gcc/testsuite/gcc.dg/pr114768.c 2024-04-18 15:43:30.389730365 +0200 > > @@ -0,0 +1,10 @@ > > +/* PR rtl-optimization/114768 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-rtl-final" } */ > > +/* { dg-final { scan-rtl-dump "\\\(mem/v:" "final" { target { ! { > > nvptx*-*-* } } } } } */ > > + > > +void > > +foo (int *p) > > +{ > > + *p = *(volatile int *) p; > > +} > > Why exclude nvptx target here? As far as I can see, it does behave in > the exactly same way as expected; see 'diff' of before vs. after the > 'gcc/rtlanal.cc' code changes: I wasn't sure if the non-RA targets (for which we don't have an effective target) even have final dump. If they do as you show, then guess the target guard can go. Jakub
[PATCH] rtlanal: Fix set_noop_p for volatile loads or stores [PR114768]
Hi! On the following testcase, combine propagates the mem/v load into mem store with the same address and then removes it, because noop_move_p says it is a no-op move. If it was the other way around, i.e. mem/v store and mem load, or both would be mem/v, it would be kept. The problem is that rtx_equal_p never checks any kind of flags on the rtxes (and I think it would be quite dangerous to change it at this point), and set_noop_p checks side_effects_p on just one of the operands, not both. In the MEM <- MEM set, it only checks it on the destination, in store to ZERO_EXTRACT only checks it on the source. The following patch adds the missing side_effects_p checks. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-19 Jakub Jelinek PR rtl-optimization/114768 * rtlanal.cc (set_noop_p): Don't return true for MEM <- MEM sets if src has side-effects or for stores into ZERO_EXTRACT if ZERO_EXTRACT operand has side-effects. * gcc.dg/pr114768.c: New test. --- gcc/rtlanal.cc.jj 2024-02-24 12:45:28.674249100 +0100 +++ gcc/rtlanal.cc 2024-04-18 15:09:55.199499083 +0200 @@ -1637,12 +1637,15 @@ set_noop_p (const_rtx set) return true; if (MEM_P (dst) && MEM_P (src)) -return rtx_equal_p (dst, src) && !side_effects_p (dst); +return (rtx_equal_p (dst, src) + && !side_effects_p (dst) + && !side_effects_p (src)); if (GET_CODE (dst) == ZERO_EXTRACT) -return rtx_equal_p (XEXP (dst, 0), src) - && !BITS_BIG_ENDIAN && XEXP (dst, 2) == const0_rtx - && !side_effects_p (src); +return (rtx_equal_p (XEXP (dst, 0), src) + && !BITS_BIG_ENDIAN && XEXP (dst, 2) == const0_rtx + && !side_effects_p (src) + && !side_effects_p (XEXP (dst, 0))); if (GET_CODE (dst) == STRICT_LOW_PART) dst = XEXP (dst, 0); --- gcc/testsuite/gcc.dg/pr114768.c.jj 2024-04-18 15:37:49.139433678 +0200 +++ gcc/testsuite/gcc.dg/pr114768.c 2024-04-18 15:43:30.389730365 +0200 @@ -0,0 +1,10 @@ +/* PR rtl-optimization/114768 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-rtl-final" } */ +/* { dg-final { scan-rtl-dump "\\\(mem/v:" "final" { target { ! { nvptx*-*-* } } } } } */ + +void +foo (int *p) +{ + *p = *(volatile int *) p; +} Jakub
[PATCH] libgcc: Another __divmodbitint4 bug fix [PR114762]
Hi! The following testcase is miscompiled because the code to decrement vn on negative value with all ones in most significant limb (even partial) and 0 in most significant bit of the second most significant limb doesn't take into account the case where all bits below the most significant limb are zero. This has been a problem both in the version before yesterday's commit where it has been done only if un was one shorter than vn before this decrement, and is now problem even more often when it is done earlier. When we decrement vn in such case and negate it, we end up with all 0s in the v2 value, so have both the problems with UB on __builtin_clz* and the expectations of the algorithm that the divisor has most significant bit set after shifting, plus when the decremented vn is 1 it can SIGFPE on division by zero even when it is not division by zero etc. Other values shouldn't get 0 in the new most significant limb after negation, because the bitint_reduce_prec canonicalization should reduce prec if the second most significant limb is all ones and if that limb is all zeros, if at least one limb below it is non-zero, carry in will make it non-zero. The following patch fixes it by checking if at least one bit below the most significant limb is non-zero, in that case it decrements, otherwise it will do nothing (but e.g. for the un < vn case that also means the divisor is large enough that the result should be q 0 r u). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-19 Jakub Jelinek PR libgcc/114762 * libgcc2.c (__divmodbitint4): Don't decrement vn if all bits below the most significant limb are zero. * gcc.dg/torture/bitint-70.c: New test. --- libgcc/libgcc2.c.jj 2024-04-18 09:48:55.172538667 +0200 +++ libgcc/libgcc2.c2024-04-18 12:17:28.893616007 +0200 @@ -1715,11 +1715,18 @@ __divmodbitint4 (UBILtype *q, SItype qpr && vn > 1 && (Wtype) v[BITINT_END (1, vn - 2)] >= 0) { - vp = 0; - --vn; + /* Unless all bits below the most significant limb are zero. */ + SItype vn2; + for (vn2 = vn - 2; vn2 >= 0; --vn2) + if (v[BITINT_END (vn - 1 - vn2, vn2)]) + { + vp = 0; + --vn; #if __LIBGCC_BITINT_ORDER__ == __ORDER_BIG_ENDIAN__ - ++v; + ++v; #endif + break; + } } if (__builtin_expect (un < vn, 0)) { --- gcc/testsuite/gcc.dg/torture/bitint-70.c.jj 2024-04-18 12:26:09.406383158 +0200 +++ gcc/testsuite/gcc.dg/torture/bitint-70.c2024-04-18 12:26:57.253718287 +0200 @@ -0,0 +1,22 @@ +/* PR libgcc/114762 */ +/* { dg-do run { target bitint } } */ +/* { dg-options "-std=c23" } */ +/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */ +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */ + +#if __BITINT_MAXWIDTH__ >= 255 +__attribute__((__noipa__)) signed _BitInt(255) +foo (signed _BitInt(255) a, signed _BitInt(65) b) +{ + return a / b; +} +#endif + +int +main () +{ +#if __BITINT_MAXWIDTH__ >= 255 + if (foo (1, -0xwb - 1wb)) +__builtin_abort (); +#endif +} Jakub
Re: [PATCH] libgcc: Fix up __divmodbitint4 [PR114755]
On Thu, Apr 18, 2024 at 11:25:43AM +0200, Christophe Lyon wrote: > On Thu, 18 Apr 2024 at 09:37, Jakub Jelinek wrote: > > The following testcase aborts on aarch64-linux but does not on x86_64-linux. > > In both cases there is UB in the __divmodbitint4 implemenetation. > > When the divisor is negative with most significant limb (even when partial) > > all ones, has at least 2 limbs and the second most significant limb has the > > most significant bit clear, when this number is negated, it will have 0 > > in the most significant limb. > > Already in the PR114397 r14-9592 fix I was dealing with such divisors, but > > thought the problem is only if because of that un < vn doesn't imply the > > quotient is 0 and remainder u. > > But as this testcase shows, the problem is with such divisors always. > > What happens is that we use __builtin_clz* on the most significant limb, > > and assume it will not be 0 because that is UB for the builtins. > > Normally the most significant limb of the divisor shouldn't be 0, as > > guaranteed by the bitint_reduce_prec e.g. for the positive numbers, unless > > the divisor is just 0 (but for vn == 1 we have special cases). > > Just curious: could this have been caught by ubsan? (I don't know if > it knows about clz) ubsan does instrument clz, I don't remember right now if even libgcc is built with -fsanitize=undefined during bootstrap-ubsan, if it is, it probably should (but we didn't have this test in the testsuite). Jakub
[PATCH] libgcc: Fix up __divmodbitint4 [PR114755]
Hi! The following testcase aborts on aarch64-linux but does not on x86_64-linux. In both cases there is UB in the __divmodbitint4 implemenetation. When the divisor is negative with most significant limb (even when partial) all ones, has at least 2 limbs and the second most significant limb has the most significant bit clear, when this number is negated, it will have 0 in the most significant limb. Already in the PR114397 r14-9592 fix I was dealing with such divisors, but thought the problem is only if because of that un < vn doesn't imply the quotient is 0 and remainder u. But as this testcase shows, the problem is with such divisors always. What happens is that we use __builtin_clz* on the most significant limb, and assume it will not be 0 because that is UB for the builtins. Normally the most significant limb of the divisor shouldn't be 0, as guaranteed by the bitint_reduce_prec e.g. for the positive numbers, unless the divisor is just 0 (but for vn == 1 we have special cases). The following patch moves the handling of this corner case a few lines earlier before the un < vn check, because adjusting the vn later is harder. Bootstrapped/regtested on x86_64-linux and i686-linux, plus tested with make check-gcc -j32 -k GCC_TEST_RUN_EXPENSIVE=1 RUNTESTFLAGS="GCC_TEST_RUN_EXPENSIVE=1 dg.exp='*bitint* pr112673.c builtin-stdc-bit-*.c pr112566-2.c pr112511.c' dg-torture.exp=*bitint* dfp.exp=*bitint*" on aarch64-linux, ok for trunk? 2024-04-18 Jakub Jelinek PR libgcc/114755 * libgcc2.c (__divmodbitint4): Perform the decrement on negative v with most significant limb all ones and the second least significant limb with most significant bit clear always, regardless of un < vn. * gcc.dg/torture/bitint-69.c: New test. --- libgcc/libgcc2.c.jj 2024-03-21 13:07:43.629886730 +0100 +++ libgcc/libgcc2.c2024-04-17 19:00:55.453691368 +0200 @@ -1705,69 +1705,62 @@ __divmodbitint4 (UBILtype *q, SItype qpr USItype rn = ((USItype) rprec + W_TYPE_SIZE - 1) / W_TYPE_SIZE; USItype up = auprec % W_TYPE_SIZE; USItype vp = avprec % W_TYPE_SIZE; + /* If vprec < 0 and the top limb of v is all ones and the second most + significant limb has most significant bit clear, then just decrease + vn/avprec/vp, because after negation otherwise v2 would have most + significant limb clear. */ + if (vprec < 0 + && ((v[BITINT_END (0, vn - 1)] | (vp ? ((UWtype) -1 << vp) : 0)) + == (UWtype) -1) + && vn > 1 + && (Wtype) v[BITINT_END (1, vn - 2)] >= 0) +{ + vp = 0; + --vn; +#if __LIBGCC_BITINT_ORDER__ == __ORDER_BIG_ENDIAN__ + ++v; +#endif +} if (__builtin_expect (un < vn, 0)) { - /* If abs(v) > abs(u), then q is 0 and r is u. -Unfortunately un < vn doesn't always mean abs(v) > abs(u). -If uprec > 0 and vprec < 0 and vn == un + 1, if the -top limb of v is all ones and the second most significant -limb has most significant bit clear, then just decrease -vn/avprec/vp and continue, after negation both numbers -will have the same number of limbs. */ - if (un + 1 == vn - && uprec >= 0 - && vprec < 0 - && ((v[BITINT_END (0, vn - 1)] | (vp ? ((UWtype) -1 << vp) : 0)) - == (UWtype) -1) - && (Wtype) v[BITINT_END (1, vn - 2)] >= 0) - { - vp = 0; - --vn; + /* q is 0 and r is u. */ + if (q) + __builtin_memset (q, 0, qn * sizeof (UWtype)); + if (r == NULL) + return; #if __LIBGCC_BITINT_ORDER__ == __ORDER_BIG_ENDIAN__ - ++v; + r += rn - 1; + u += un - 1; #endif + if (up) + --un; + if (rn < un) + un = rn; + for (rn -= un; un; --un) + { + *r = *u; + r += BITINT_INC; + u += BITINT_INC; } - else + if (!rn) + return; + if (up) { - /* q is 0 and r is u. */ - if (q) - __builtin_memset (q, 0, qn * sizeof (UWtype)); - if (r == NULL) + if (uprec > 0) + *r = *u & (((UWtype) 1 << up) - 1); + else + *r = *u | ((UWtype) -1 << up); + r += BITINT_INC; + if (!--rn) return; -#if __LIBGCC_BITINT_ORDER__ == __ORDER_BIG_ENDIAN__ - r += rn - 1; - u += un - 1; -#endif - if (up) - --un; - if (rn < un) - un = rn; - for (rn -= un; un; --un) - { - *r = *u; - r += BITINT_INC; - u += BITINT_INC; - } - if (!rn) - return; - if (up) - { - if (uprec > 0) - *r = *u & (((UWtype) 1 << up) - 1); - else - *r = *u | ((UWtype) -1 << up); -
[PATCH] internal-fn: Temporarily disable flag_trapv during .{ADD,SUB,MUL}_OVERFLOW etc. expansion [PR114753]
Hi! __builtin_{add,sub,mul}_overflow{,_p} builtins are well defined for all inputs even for -ftrapv, and the -fsanitize=signed-integer-overflow ifns shouldn't abort in libgcc but emit the desired ubsan diagnostics or abort depending on -fsanitize* setting regardless of -ftrapv. The expansion of these internal functions uses expand_expr* in various places (e.g. MULT_EXPR at least in 2 spots), so temporarily disabling flag_trapv in all those spots would be hard. The following patch disables it around the bodies of 3 functions which can do the expand_expr calls. If it was in the C++ FE, I'd use some RAII sentinel, but I don't think we have one in the middle-end. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-18 Jakub Jelinek PR middle-end/114753 * internal-fn.cc (expand_mul_overflow): Save flag_trapv and temporarily clear it for the duration of the function, then restore previous value. (expand_vector_ubsan_overflow): Likewise. (expand_arith_overflow): Likewise. * gcc.dg/pr114753.c: New test. --- gcc/internal-fn.cc.jj 2024-03-23 08:22:50.490607002 +0100 +++ gcc/internal-fn.cc 2024-04-17 13:44:21.673594413 +0200 @@ -1631,7 +1631,11 @@ expand_mul_overflow (location_t loc, tre rtx target = NULL_RTX; signop sign; enum insn_code icode; + int save_flag_trapv = flag_trapv; + /* We don't want any __mulv?i3 etc. calls from the expansion of + these internal functions, so disable -ftrapv temporarily. */ + flag_trapv = 0; done_label = gen_label_rtx (); do_error = gen_label_rtx (); @@ -2479,6 +2483,7 @@ expand_mul_overflow (location_t loc, tre else expand_arith_overflow_result_store (lhs, target, mode, res); } + flag_trapv = save_flag_trapv; } /* Expand UBSAN_CHECK_* internal function if it has vector operands. */ @@ -2499,7 +2504,11 @@ expand_vector_ubsan_overflow (location_t rtx resvr = NULL_RTX; unsigned HOST_WIDE_INT const_cnt = 0; bool use_loop_p = (!cnt.is_constant (_cnt) || const_cnt > 4); + int save_flag_trapv = flag_trapv; + /* We don't want any __mulv?i3 etc. calls from the expansion of + these internal functions, so disable -ftrapv temporarily. */ + flag_trapv = 0; if (lhs) { optab op; @@ -2629,6 +2638,7 @@ expand_vector_ubsan_overflow (location_t } else if (resvr) emit_move_insn (lhsr, resvr); + flag_trapv = save_flag_trapv; } /* Expand UBSAN_CHECK_ADD call STMT. */ @@ -2707,7 +2717,11 @@ expand_arith_overflow (enum tree_code co prec0 = MIN (prec0, pr); pr = get_min_precision (arg1, uns1_p ? UNSIGNED : SIGNED); prec1 = MIN (prec1, pr); + int save_flag_trapv = flag_trapv; + /* We don't want any __mulv?i3 etc. calls from the expansion of + these internal functions, so disable -ftrapv temporarily. */ + flag_trapv = 0; /* If uns0_p && uns1_p, precop is minimum needed precision of unsigned type to hold the exact result, otherwise precop is minimum needed precision of signed type to @@ -2748,6 +2762,7 @@ expand_arith_overflow (enum tree_code co ops.location = loc; rtx tem = expand_expr_real_2 (, NULL_RTX, mode, EXPAND_NORMAL); expand_arith_overflow_result_store (lhs, target, mode, tem); + flag_trapv = save_flag_trapv; return; } @@ -2771,6 +2786,7 @@ expand_arith_overflow (enum tree_code co if (integer_zerop (arg0) && !unsr_p) { expand_neg_overflow (loc, lhs, arg1, false, NULL); + flag_trapv = save_flag_trapv; return; } /* FALLTHRU */ @@ -2781,6 +2797,7 @@ expand_arith_overflow (enum tree_code co case MULT_EXPR: expand_mul_overflow (loc, lhs, arg0, arg1, unsr_p, unsr_p, unsr_p, false, NULL); + flag_trapv = save_flag_trapv; return; default: gcc_unreachable (); @@ -2826,6 +2843,7 @@ expand_arith_overflow (enum tree_code co else expand_mul_overflow (loc, lhs, arg0, arg1, unsr_p, uns0_p, uns1_p, false, NULL); + flag_trapv = save_flag_trapv; return; } --- gcc/testsuite/gcc.dg/pr114753.c.jj 2024-04-17 13:55:16.246482369 +0200 +++ gcc/testsuite/gcc.dg/pr114753.c 2024-04-17 13:54:14.035352376 +0200 @@ -0,0 +1,14 @@ +/* PR middle-end/114753 */ +/* { dg-do run } */ +/* { dg-options "-O2 -ftrapv" } */ + +int +main () +{ + volatile long long i = __LONG_LONG_MAX__; + volatile long long j = 2; + long long k; + if (!__builtin_mul_overflow (i, j, ) || k != -2LL) +__builtin_abort (); + return 0; +} Jakub
Re: [PATCH] libcpp: Regenerate aclocal.m4 and configure [PR 114748]
On Wed, Apr 17, 2024 at 01:16:43PM -0400, Eric Gallager wrote: > > --- a/libcpp/configure > > +++ b/libcpp/configure > > @@ -2670,6 +2670,9 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu > > > > > > > > + > > + > > + > > So, this is kind of a minor style nitpick, but personally, it kind of > bothers me when autotools goes and inserts a bunch of unnecessary > blank newlines in the generated output scripts. Does anyone else think > it'd be worth it to scatter around some of m4's "dnl" comments in the > configure.ac scripts, to get autoconf to eat the unnecessary newlines? In stage1 maybe, but certainly not in stage4. Jakub
Re: [PATCH] c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 17, 2024 at 04:34:07PM +0200, Jan Hubicka wrote: > I think for most scenarios this is OK, but I am not sure about > incremental linking (both LTO and non-LTO). What seems wrong is that we > produce C5 comdat that is not exporting what it should and thus breaking > the invariant that in valid code all comdats of same name are > semantically equivalent. Yeah, exactly. That is what I'm worried about too. > Perhaps it makes no difference since this > scenario is pretty special and we know that the functions are > semantically equivalent and their addresses are never compared for > equality (at least I failed to produce some useful testcase). Yes, I think one can't take address of a constructor/destructor and compare that for equality; I guess the destructor address can be stored in vtables, but code manually reading stuff from vtables and assuming pointer equality is almost certainly not valid. Jakub
Re: [PATCH] c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 17, 2024 at 03:26:53PM +0200, Jan Hubicka wrote: > > > > I've tried to see what actually happens during linking without LTO, so > > compiled > > pr113208_0.C with -O1 -fkeep-inline-functions -std=c++20 with vanilla trunk > > (so it has those 2 separate comdats, one for C2 and one for C1), though I've > > changed the > > void m(k); > > line to > > __attribute__((noipa)) void m(k) {} > > in the testcase, then compiled > > pr113208_1.C with -O2 -fkeep-inline-functions -std=c++20 > > -fno-omit-frame-pointer > > so that one can clearly differentiate from where the implementation was > > picked and finally added > > template struct _Vector_base { > > int g() const; > > _Vector_base(int, int); > > }; > > > > struct QualityValue; > > template <> > > _Vector_base::_Vector_base(int, int) {} > > template <> > > int _Vector_base::g() const { return 0; } > > int main () {} > > If I link this, I see _ZN6vectorI12QualityValueEC2ERKS1_ and > > _ZN6vectorI12QualityValueEC1ERKS1_ as separate functions with the > > omitted frame pointer bodies, so clearly the pr113208_0.C versions prevailed > > in both cases. It is unclear why that isn't the case for LTO. > > I think it is because of -fkeep-inline-functions which makes the first > object file to define both symbols, while with LTO we optimize out one > of them. > > So to reproduce same behaviour with non-LTO we would probably need use > -O1 and arrange the contructor to be unilinable instead of using > -fkeep-inline-functions. Ah, you're right. If I compile (the one line modified) pr113208_0.C with -O -fno-early-inlining -fdisable-ipa-inline -std=c++20 it does have just _ZN6vectorI12QualityValueEC2ERKS1_ in _ZN6vectorI12QualityValueEC2ERKS1_ comdat and no _ZN6vectorI12QualityValueEC1ERKS1_ and pr113208_1.C with -O -fno-early-inlining -fdisable-ipa-inline -std=c++20 -fno-omit-frame-pointer and link that together with the above mentioned third *.C file, I see 0040112a <_ZN6vectorI12QualityValueEC2ERKS1_>: 40112a: 53 push %rbx 40112b: 48 89 fbmov%rdi,%rbx 40112e: 48 89 f7mov%rsi,%rdi 401131: e8 9c 00 00 00 call 4011d2 <_ZNK12_Vector_baseI12QualityValueE1gEv> 401136: 89 c2 mov%eax,%edx 401138: be 01 00 00 00 mov$0x1,%esi 40113d: 48 89 dfmov%rbx,%rdi 401140: e8 7b 00 00 00 call 4011c0 <_ZN12_Vector_baseI12QualityValueEC1Eii> 401145: 5b pop%rbx 401146: c3 ret i.e. the C2 prevailing from pr113208_0.s where it is the only symbol, and 00401196 <_ZN6vectorI12QualityValueEC1ERKS1_>: 401196: 55 push %rbp 401197: 48 89 e5mov%rsp,%rbp 40119a: 53 push %rbx 40119b: 48 83 ec 08 sub$0x8,%rsp 40119f: 48 89 fbmov%rdi,%rbx 4011a2: 48 89 f7mov%rsi,%rdi 4011a5: e8 28 00 00 00 call 4011d2 <_ZNK12_Vector_baseI12QualityValueE1gEv> 4011aa: 89 c2 mov%eax,%edx 4011ac: be 01 00 00 00 mov$0x1,%esi 4011b1: 48 89 dfmov%rbx,%rdi 4011b4: e8 07 00 00 00 call 4011c0 <_ZN12_Vector_baseI12QualityValueEC1Eii> 4011b9: 48 8b 5d f8 mov-0x8(%rbp),%rbx 4011bd: c9 leave 4011be: c3 ret which is the C1 alias originally aliased to C2 in C5 comdat. So, that would match linker behavior where it sees C1 -> C2 alias prevails, but a different version of C2 prevails, so let's either make C1 a non-alias or alias to a non-exported symbol or something like that. Though, I admit I have no idea what we do with comdat's during LTO, perhaps doing what I said above could break stuff if linker after seeing the LTO resulting objects decides on prevailing symbols differently. Jakub
Re: [PATCH] libcpp: Regenerate aclocal.m4 and configure [PR 114748]
On Wed, Apr 17, 2024 at 02:01:55PM +, Christophe Lyon wrote: > As discussed in the PR, aclocal.m4 and configure were incorrectly > regenerated at some point. > > 2024-04-17 Christophe Lyon > > PR preprocessor/114748 > libcpp/ > * aclocal.m4: Regenerate. > * configure: Regenerate. Ok, thanks. Jakub
Re: [PATCH] c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
On Wed, Apr 17, 2024 at 11:04:26AM +0200, Jan Hubicka wrote: > > 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. > > Thanks for working this out! Creating cgraph node with no body is > harmless as it will be removed later. That is actually for functions with bodies, maybe_clone_body creates the bodies for those, but still when it happens early, the cdtors have tentative_decl_linkage linkage, which in many cases means DECL_EXTERNAL, DECL_NOT_REALLY_EXTERN (C++ FE special thing), DECL_DEFER_OUTPUT etc. > > + tree comdat_group = cdtor_comdat_group (fns[1], fns[0]); > > + n1 = cgraph_node::get (fns[1]); > > + n0->set_comdat_group (comdat_group); > > + if (n1->same_comdat_group) > > +n1->remove_from_same_comdat_group (); > > + n1->add_to_same_comdat_group (n0); > > + if (fns[2]) > > +n2->add_to_same_comdat_group (n0); > > + import_export_decl (fns[1]); > > So this is handling the case where symbol was already inserted into one > comdat group and later we need to move it into the C5 group? As long as > we move everythingf rom old comdat group, this should be fine. The above is pretty much an adjusted copy of what maybe_clone_body does, except it doesn't call cgraph_node::get{,_create} all the time and uses import_export_decl rather than expand_or_defer_fn{,_1}. > > + /* Remove the body now that it is an alias. */ > > + DECL_SAVED_TREE (fns[1]) = NULL_TREE; > Maybe using release_function_body since it also knows how to remove > DECL_STRUCT_FUNCTION that exists at this stage? Guess I could try that, clearing of DECL_SAVED_TREE was what was done in maybe_clone_body too. > I was thinking how to solve the problem on cgrpah side. We definitely > have long lasting bug where aliases are handled incorrectly for which I > made WIP patch (but probably would like to hold it after release branch is > created). When foo has alias bar and foo is praviled then the alias > target is prevailed too. This is what causes the ICE about cross comdat > section alias. However fixing this is not enough as I do not think we > can handle incremental linking correctly (as discussed briefly on IRC > technically we should keep both sections but that would require two > symbols of same name in single .o file). > > With the aliasing fixed we turn the other symbol into static but keep > alias, so we end up with one comdat group having the non-aliased > constructor and second comdat group (C5) exporting only the alias, which > is not quite reight either. I've tried to see what actually happens during linking without LTO, so compiled pr113208_0.C with -O1 -fkeep-inline-functions -std=c++20 with vanilla trunk (so it has those 2 separate comdats, one for C2 and one for C1), though I've changed the void m(k); line to __attribute__((noipa)) void m(k) {} in the testcase, then compiled pr113208_1.C with -O2 -fkeep-inline-functions -std=c++20 -fno-omit-frame-pointer so that one can clearly differentiate from where the implementation was picked and finally added template struct _Vector_base { int g() const; _Vector_base(int, int); }; struct QualityValue; template <> _Vector_base::_Vector_base(int, int) {} template <> int _Vector_base::g() const { return 0; } int main () {} If I link this, I see _ZN6vectorI12QualityValueEC2ERKS1_ and _ZN6vectorI12QualityValueEC1ERKS1_ as separate functions with the omitted frame pointer bodies, so clearly the pr113208_0.C versions prevailed in both cases. It is unclear why that isn't the case for LTO. Jakub
[PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
Hi! Starting with GCC 14 we have the nice URLification of the options printed in diagnostics, say for in test.c:4:23: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long int’ [-Wformat=] the -Wformat= is underlined in some terminals and hovering on it shows https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat link. This works nicely on the GCC trunk, where the online documentation is regenerated every day from a cron job and more importantly, people rarely use the trunk snapshots for too long, so it is unlikely that further changes in the documentation will make too many links stale, because users will simply regularly update to newer snapshots. I think it doesn't work properly on release branches though. Some users only use the relased versions (i.e. MAJOR.MINOR.0) from tarballs but can use them for a couple of years, others use snapshots from the release branches, but again they could be in use for months or years and the above mentioned online docs which represent just the GCC trunk might diverge significantly. Now, for the relases we always publish also online docs for the release, which unlike the trunk online docs will not change further, under e.g. https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Warning-Options.html#index-Wformat or https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html#index-Wformat etc. So, I think at least for the MAJOR.MINOR.0 releases we want to use URLs like above rather than the trunk ones and we can use the same process of updating *.opt.urls as well for that. For the snapshots from release branches, we don't have such docs. One option (implemented in the patch below for the URL printing side) is point to the MAJOR.MINOR.0 docs even for MAJOR.MINOR.1 snapshots. Most of the links will work fine, for options newly added on the release branches (rare thing but still happens) can have until the next release no URLs for them and get them with the next point release. The question is what to do about make regenerate-opt-urls for the release branch snapshots. Either just document that users shouldn't make regenerate-opt-urls on release branches (and filter out *.opt.urls changes from their commits), add make regenerate-opt-urls task be RM responsibility before making first release candidate from a branch and adjust the autoregen CI to know about that. Or add a separate goal which instead of relying on make html created files would download copy of the html files from the last release from web (kind of web mirroring the https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ subtree locally) and doing regenerate-opt-urls on top of that? But how to catch the point when first release candidate is made and we want to update to what will be the URLs once the release is made (but will be stale URLs for a week or so)? Another option would be to add to cron daily regeneration of the online docs for the release branches. I don't think that is a good idea though, because as I wrote earlier, not all users update to the latest snapshot frequently, so there can be users that use gcc 13.1.1 20230525 for months or years, and other users which use gcc 13.1.1 20230615 for years etc. Another question is what is most sensible for users who want to override the default root and use the --with-documentation-root-url= configure option. Do we expect them to grab the whole onlinedocs tree or for release branches at least include gcc-14.1.0/ subdirectory under the root? If so, the patch below deals with that. Or should we just change the default documentation root url, so if user doesn't specify --with-documentation-root-url= and we are on a release branch, default that to https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ or https://gcc.gnu.org/onlinedocs/gcc-14.2.0/ etc. and don't add any infix in get_option_url/make_doc_url, but when people supply their own, let them point to the root of the tree which contains the right docs? Then such changes would go into gcc/configure.ac, some case based on "$gcc_version", from that decide if it is a release branch or trunk. 2024-04-17 Jakub Jelinek PR other/114738 * opts.cc (get_option_url): On release branches append gcc-MAJOR.MINOR.0/ after DOCUMENTATION_ROOT_URL. * gcc-urlifier.cc (gcc_urlifier::make_doc_url): Likewise. --- gcc/opts.cc.jj 2024-01-05 08:35:13.600828652 +0100 +++ gcc/opts.cc 2024-04-17 12:03:10.961525141 +0200 @@ -3761,7 +3761,19 @@ get_option_url (const diagnostic_context { label_text url_suffix = get_option_url_suffix (option_index, lang_mask); if (url_suffix.get ()) - return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (), nullptr); + { + char infix[32]; + /* On release branches, append to DOCUMENTATION_ROOT_URL the +subdirectory with documentation of the latest release made +from the branch. */ + if (BUILDING_GCC_MINOR != 0 && BUILDING_GC
[PATCH] c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
Hi! 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. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-04-17 Jakub Jelinek PR lto/113208 * cp-tree.h (maybe_optimize_cdtor): Declare. * decl2.cc (import_export_decl): Call it for cloned cdtors. * optimize.cc (maybe_optimize_cdtor): New function. * 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/cp-tree.h.jj 2024-04-16 17:18:37.286279533 +0200 +++ gcc/cp/cp-tree.h2024-04-16 17:45:01.685635709 +0200 @@ -7447,6 +7447,7 @@ extern bool handle_mo