Re: RFC: attributes documentation
On Mon, 2 Oct 2023, Sandra Loosemore wrote: > Going beyond that, though, I think we should also document that the standard > syntax is now the preferred way to do it, and change the examples (except for > the parts documenting the old syntax) to use the new standard syntax. It's > been accepted by the default -std= setting for both C and C++ since GCC 10, > and my understanding is that C2x will be official by the time GCC 14 is > released (so supporting the new syntax won't be just another GNU extension any > more). Does this sound OK to everybody? If you're documenting attributes in the [[]] form, you need to be a lot more careful to distinguish between an attribute on a declaration and one on the type of that declaration, for example, because those need to go in different places in the standard syntax (the laxity applied with __attribute__ that tries to guess what was meant and e.g. move an attribute from a declaration to its type doesn't apply with the standard syntax, which has precise rules in the standard for what entity an attribute in a given syntactic position appertains to). In some cases this means that the [[]] attribute needs to go in a different position from __attribute__ in examples - in int f (void) __attribute__ ((foo)); the GNU attribute in that position is considered a declaration attribute (but a function type attribute applied to a declaration will automatically be applied to the type instead) whereas int f (void) [[gnu::foo]]; is only a function type attribute, not a declaration attribute [*], and [[gnu::foo]] int f (void); is only a declaration attribute, not a function type attribute. (The version int [[gnu::foo]] f (void); applies the attribute to the int return type, which probably isn't what you want - unless you're e.g. using [[gnu::vector_size (16)]] to declare that the function returns a vector.) To complicate things further, some attributes that are implemented as declaration attributes maybe logically should be type attributes but support for them on types hasn't been implemented - and if making such a GNU attribute into a type attribute in future, we might then need to consider appropriate compatibility for allowing it in both places even with the standard syntax. [*] In the GNU syntax an attribute in this position is parsed as an attribute following a full declarator and any subsequent asm giving an external linkage name for the declaration. In the standard syntax, an attribute in this position is part of a declarator immediately following a function declarator (and so could appear on nested function declarators, for example). So while it looks superficially like "the same" position for the attributes, it's not at all the same syntactically. -- Joseph S. Myers jos...@codesourcery.com
Re: RFC: attributes documentation
On Tue, 3 Oct 2023, Sandra Loosemore wrote: > Is __attribute__ also considered more powerful than the standard [[]] syntax, > enough to recommend it over writing standard-conforming code? Anything that can be expressed with __attribute__ should also be expressible with [[]], so use of [[]] is probably a good idea for users not concerned with portability to older GCC versions. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] error for function with external and internal linkage [PR111708]
On Sat, 14 Oct 2023, Martin Uecker wrote: > + if (!initialized > + && storage_class != csc_static > + && storage_class != csc_auto > + && current_scope != file_scope) I think it would be better to use TREE_PUBLIC (decl) in place of storage_class != csc_static && storage_class != csc_auto. OK with that change. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 2/2] [c] Fix PR 101364: ICE after error due to diagnose_arglist_conflict not checking for error
On Sat, 14 Oct 2023, Andrew Pinski wrote: > When checking to see if we have a function declaration has a conflict due to > promotations, there is no test to see if the type was an error mark and then > calls > c_type_promotes_to. c_type_promotes_to is not ready for error_mark and causes > an > ICE. > > This adds a check for error before the call of c_type_promotes_to. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/2] Fix ICE due to c_safe_arg_type_equiv_p not checking for error_mark node
On Sat, 14 Oct 2023, Andrew Pinski wrote: > This is a simple error recovery issue when c_safe_arg_type_equiv_p > was added in r8-5312-gc65e18d3331aa999. The issue is that after > an error, an argument type (of a function type) might turn > into an error mark node and c_safe_arg_type_equiv_p was not ready > for that. So this just adds a check for error operand for its > arguments before getting the main variant. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK. -- Joseph S. Myers jos...@codesourcery.com
RE: [PATCH][GCC][front-end][build-machinery][opt-framework] Allow setting of stack-clash via configure options. [Patch (4/6)]
On Tue, 24 Jul 2018, tamar.christ...@arm.com wrote: > This patch defines a configure option to allow the setting of the default > guard size via configure flags when building the target. If you add a configure option, you must also add documentation for it in install.texi. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH], Add configuration checks to PowerPC --with-long-double-format=ieee
On Fri, 6 Jul 2018, Segher Boessenkool wrote: > Version checks are terrible. This is nothing new. The key principle behind --with-glibc-version is that you can pass that option *when building the static-only inhibit_libc bootstrap compiler without having built glibc yet* and it will result in the compiler being correctly configured for the specified glibc version, and thus able to build glibc binaries identical to those you get from a longer alternating sequence of compiler and glibc (headers) builds. At that point in a bootstrap of a cross toolchain you don't have any target glibc headers available (you might have target kernel headers) and so have no other way in which the compiler can possibly tell what glibc version is in use. > For cross builds you can just assume it works. That should work fine here. We definitely support building a new GCC using a sysroot of an old glibc (so that the new GCC can then be used to build binaries that will run on old distributions, for example). (This certainly works and is useful for x86_64; I don't assert whether or not it works, or makes sense, for powerpc64le.) Of course in that case the build can examine target headers to determine versions and features, but a new GCC release is still likely to be able to build a few past glibc release branches if anyone backported the required build fixes to those branches, and so the bootstrap case is still useful with somewhat older glibc versions. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] haiku: Initial build support
On Mon, 16 Jul 2018, Alexander von Gluck IV wrote: > * We have been dragging these around since gcc 4.x. > * Some tweaks will likely be needed, but this gets our foot > in the door. > > Authors: > Fredrik Holmqvist > Jerome Duval > Augustin Cavalier > François Revol > Simon South > Jessica Hamilton > Ithamar R. Adema > Oliver Tappe > Jonathan Schleifer > .. and maybe more! Before this can be reviewed, we'll need copyright assignments (with employer disclaimers where applicable) on file at the FSF from everyone who contributed a legally significant amount of code (more than around 15 lines). Without those, reviewers can't safely look at the changes in detail. https://gcc.gnu.org/contribute.html https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future Then, please make sure that only substantive changes are included - that there are no diff lines that are purely changing trailing whitespace in existing code, for example. Please ensure that all copyright and license notices follow current standards (which means using ranges of years ending in 2018, GPLv3 notices and a URL not an FSF postal address). For changes to existing code, especially, please make sure to include sufficient rationale in the patch submission to explain those changes, why they are needed and the approach taken to them. For new target OS support, I'd expect details to be provided of the test results on that OS for the various architectures supported by GCC. Are you planning, if the support is accepted in GCC, to maintain a bot that keeps running the GCC testsuite for GCC mainline for this OS for the various target architectures supported, at least daily or thereabouts, and posts the results to the gcc-testresults list, and to keep monitoring the test results and fixing OS-specific issues that show up? It's much better for issues to be identified within a day or two of the commit causing them than many months later, possibly only after a release has come out with the issue - but that requires an ongoing commitment to keep monitoring test results, posting them to gcc-testresults and keeping them in good shape. > diff --git a/libtool.m4 b/libtool.m4 If this an exact backport of a change from upstream libtool git? If so, please give the commit reference. If not, give the URL of the submission to upstream libtool. We don't want local libtool changes that aren't backports or at least proposed upstream without objections, to avoid making future updates from upstream libtool harder. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH][Fortran][v2] Use MIN/MAX_EXPR for min/max intrinsics
On Wed, 18 Jul 2018, Janne Blomqvist wrote: > minimumNumber(a, NaN) = minimumNumber(NaN, a) = a > > That is minimumNumber corresponds to minnum in IEEE 754-2008 and fmin* in No, it differs in the handling of signaling NaNs (with minimumNumber, if the NaN argument is signaling, it results in the "invalid" exception but the non-NaN argument is still returned, whereas with minNum, a quiet NaN was returned in that case). A new fminimum_num function is proposed as a C binding to the new operation. http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2273.pdf (The new operations are also more strictly defined regarding zero arguments, to treat -0 as less than +0, which was unspecified for minNum and fmin.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Merge Ignore and Deprecated in .opt files.
On Thu, 19 Jul 2018, Martin Liška wrote: > I must admit that was my intention :) In my eyes it makes it more consistent > and > it gives consumers feedback about usage of an option that does nothing. > For x86_64 there's list of options that are Ignore and don't produce a > warning: The design is meant to be that if the option is purely non-semantic - if it's an option for tuning/enabling/disabling some aspect of some optimization, and the implementation has changed so that the option no longer makes sense with the current set of optimizations, for example - then silent ignoring is appropriate. I think that generally applies to most of the options in your list. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Merge Ignore and Deprecated in .opt files.
On Fri, 20 Jul 2018, Martin Liška wrote: > +C++ ObjC++ Ignore) Stray ')' at the end of this line. -- Joseph S. Myers jos...@codesourcery.com
Re: [0/5] C-SKY port
On Tue, 24 Jul 2018, Sandra Loosemore wrote: > On 07/23/2018 10:17 PM, Sandra Loosemore wrote: > > > C-SKY proposes Han Mao and Yunhai Shang > > as maintainers for this port. > > I apologize, I made a mistake here. The correct proposed maintainers are > Xianmiao Qu and Yunhai Shang > . I'd like to confirm: will the maintainers be running a bot reporting test results for trunk for at least one C-SKY configuration to gcc-testresults (daily or thereabouts), and monitoring results for regressions and addressing those regressions promptly? We don't yet have such a requirement, but when an architecture-independent patch causes failures on a particular architectures, it's much better if the architecture maintainers raise the issue while the patch is still fresh in the author's mind (meaning within a few days at most of the commit) than if they only look at results and track down the patch causing the failure many months later (e.g. while reviewing test results in preparation for the following release). > > We expect that > > C-SKY will also be providing a public link to the processor and ABI > > documentation at some point. > > The ABI manual has been posted, but not the ISA documentation yet. (I'd guess > that when it does show up it will be in the same place, though.) > > https://github.com/c-sky/csky-doc Could you provide the proposed GCC website changes for the port (backends.html, readings.html, news item for index.html)? readings.html, in particular, would link to the ABI and ISA documentation, while backends.html gives summary information about the properties of both the architecture and the GCC port. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/7] Add __builtin_speculation_safe_value
On Wed, 25 Jul 2018, Richard Earnshaw (lists) wrote: > >> Port maintainers DO need to decide what to do about speculation, even if > >> it is explicitly that no mitigation is needed. > > > > Agreed. But I didn't yet see a request for maintainers to decide that? > > > > consider it made, then :-) I suggest the following as an appropriate process for anything needing attention from architecture maintainers: * Send a message to the gcc list, starting its own thread, CC:ed to all target architecture maintainers, stating explicitly in its first sentence that it is about something needing action from all such maintainers. The message needs to give all the information required for those maintainers to work out what is appropriate for their ports, or point to a wiki page with that information. For example, the message must not assume maintainer familiarity with Spectre - it must give sufficient information for maintainers unfamiliar with Spectre to work out what to do with their ports. * If the messages to any maintainers bounce, actively seek out alternative contact details for them or alternative people who might be interested in maintaining those ports. Likewise, it's necessary to check for any ports that do not have a listed maintainer in the MAINTAINERS file and find appropriate contacts for those. * Over the next few months, send occasional reminders, each including a list of the ports that have not been updated. * No backport should be considered for anything that might involve breakage, e.g. warnings, for unchanged ports, until either all ports have been updated or at least several months have passed. (Normally, a change needing architecture maintainer attention would be something for which backports are obviously inappropriate, e.g. a new C / C++ language feature requiring ABI decisions to be made for which there isn't an obvious default that can safely be applied to all architectures.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/7] Add __builtin_speculation_safe_value
On Fri, 27 Jul 2018, Richard Earnshaw (lists) wrote: > The c-c++-common/spec-barrier-1.c test will fail on any target that has > not been updated (it deliberately doesn't check for > __HAVE_SPECULATION_BARRIER before trying to use the new intrinsic). The > test contains a comment to that effect. That should be enough to alert > maintainers if they are tracking testsuite errors. Introducing a test failure is not enough. You need to *explicitly* alert target maintainers to the need for action, with a self-contained explanation not assuming any prior understanding of Spectre and its mitigations, and if any backport is to be considered you'll then need to track the status of different targets and remind maintainers as needed (so that you know when all targets have been updated, or not updated despite maintainers having been informed a few months previously without the emails bouncing and reminded a few times since, or not updated and the maintainers have explicitly acknowledged this and given their OK to a backport without updates for their architectures). A mail to the gcc list (that draws attention in the subject line and the first sentence to the need for architecture maintainer action) is the bare minimum (you should not assume target maintainers read gcc-patches discussions not mentioning their architecture in the subject), but CC:ing the architecture maintainers (immediately, or for those not fixed within a month or two) provides better assurance that the issue has been properly drawn to their attention. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Avoid infinite loop with duplicate anonymous union fields
On Fri, 27 Jul 2018, Bogdan Harjoc wrote: > If a struct contains an anonymous union and both have a field with the > same name, detect_field_duplicates_hash() will replace one of them > with NULL. If compilation doesn't stop immediately, it may later call > lookup_field() on the union, which falsely assumes the union's > LANG_SPECIFIC array is sorted, and may loop indefinitely because of > this. > > Reproduced on amd64 since gcc-5, on ubuntu-18.04 and gentoo. > > The patch falls back to iterating via DECL_CHAIN if there was an error > earlier during compilation. The patch should also add a testcase to the testsuite (which fails before and passes after the patch). > I ran the gcc testsuite with the result (the FAIL seems unrelated to the > patch): > > FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors) You should use contrib/gcc_update --touch to get timestamps in the right order when checking out. This failure is a symptom of badly ordered timestamps. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] haiku: Initial build support
On Fri, 27 Jul 2018, Ramana Radhakrishnan wrote: > Joseph, > > A lot of such information seems to come out from a number of reviewers > only during patch review from new contributors. Would you mind > improving https://gcc.gnu.org/contribute.html and especially around > "Testing patches" or start something like the glibc contribution > checklist on the wiki that actually makes a lot of this easy to find > rather than searching in mailing list archives for new contributors ? I think the information in my comments about proper contents of the patch submission is generally already present in contribute.html. There may be less information about expectations for target architecture / OS maintainers running tests on an ongoing basis. Perhaps at the Cauldron we should discuss whether we wish to move towards a requirement of bots posting test results for all new architecture and OS ports? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] haiku: Initial build support
On Fri, 27 Jul 2018, Alexander von Gluck IV wrote: > >> It's much better for issues to be identified within a day or two of the > >> commit causing them than many months later, possibly only after a release > >> has come out with the issue - but that requires an ongoing commitment to > >> keep monitoring test results, posting them to gcc-testresults and keeping > >> them in good shape. > > This is good information, however, does GCC have docs for this? We are a > small team of open source developers with maybe a few man-hours a month > available to dedicate to gcc maintainership. (no excuses, just trying > to set the expectations) Well, install.texi documents the use of contrib/test_summary to post test results. I think the documentation is better for one-off requirements for a patch, than for expectations for maintainers on an ongoing basis (in that maintainers would typically have been following development for some time, and so have seen what maintainers of other architectures and OSes do - for example, have seen how AIX people promptly raise issues when a patch breaks things for AIX, which is the sort of thing I'd expect target OS maintainers to do in general). > These steps seem like what's needed on a first-class platform (Linux, OS X, > etc). So the same requirements apply to all new GCC platforms code? Requirements for OS and architecture ports aren't well-defined and seem like a good topic for Cauldron discussion. But generically, it's important that new architecture and OS ports don't get in the way of global changes. This means it's important to have sufficient documentation available for architectures for the use of people doing global changes, that it's valuable to have simulators (with OS images as applicable) with information (e.g. DejaGnu board files) available for people wishing to test on the architecture or OS (or hardware systems available in the GCC Compile Farm, for architectures and OSes for which that works better), and that it's valuable if people can tell at a glance at recent gcc-testresults posts what shape the port is in. And, as a matter of working well with other developers, it's much better to find and point out breakage promptly rather than long after the patch that broke something went in (and, in particular, in the same development stage; if there's a design issue with a patch going in during stage 1 which requires design changes for some architectures or OSes, it's very unhelpful if that's only found much later when trunk is in regression-fixes-only mode for the next release). Also, unmaintained features and unused ports are liable to be removed, and having test results posted is important evidence of whether the port is in good shape or should be considered for removal. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Fix the damage done by my other patch from yesterday to strlenopt-49.c
On Mon, 30 Jul 2018, Bernd Edlinger wrote: > Hi, > > this is how I would like to handle the over length strings issue in the C FE. > If the string constant is exactly the right length and ends in one explicit > NUL character, shorten it by one character. I don't think shortening should be limited to that case. I think the case where the constant is longer than that (and so gets an unconditional pedwarn) should also have it shortened - any constant that doesn't fit in the object being initialized should be shortened to fit, whether diagnosed or not, we should define GENERIC / GIMPLE to disallow too-large string constants in initializers, and should add an assertion somewhere in the middle-end that no too-large string constants reach it. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Fix the damage done by my other patch from yesterday to strlenopt-49.c
On Mon, 30 Jul 2018, Bernd Edlinger wrote: > In the moment I would already be happy if all STRING_CSTs would > be zero terminated. generic.texi says they need not be. Making the STRING_CST contain only the bytes of the initializer and not the trailing NUL in the C case where the trailing NUL does not fit in the object initialized would of course mean you get non-NUL-terminated STRING_CSTs for valid C code as well. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Fix the damage done by my other patch from yesterday to strlenopt-49.c
On Mon, 30 Jul 2018, Jakub Jelinek wrote: > On Mon, Jul 30, 2018 at 03:52:39PM +0000, Joseph Myers wrote: > > On Mon, 30 Jul 2018, Bernd Edlinger wrote: > > > > > In the moment I would already be happy if all STRING_CSTs would > > > be zero terminated. > > > > generic.texi says they need not be. Making the STRING_CST contain only > > the bytes of the initializer and not the trailing NUL in the C case where > > the trailing NUL does not fit in the object initialized would of course > > mean you get non-NUL-terminated STRING_CSTs for valid C code as well. > > One thing is whether TREE_STRING_LENGTH includes the trailing NUL byte, > that doesn't need to be the case e.g. for the shortened initializers. > The other thing is whether we as a convenience for the compiler's internals > want to waste some memory for the NUL termination; I think we could avoid > some bugs that way. TREE_STRING_LENGTH includes the NUL if it is logically part of the object, but should not if the NUL is purely an implementation convenience. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Libraries' configure scripts should not read config-ml.in when multilib is disabled
On Mon, 30 Jul 2018, John Ericson wrote: > I understand this building them separately is not supported, but am > nevertheless hoping the patch can nevertheless be upstreamed on the grounds > that this generally cleans up the build system in accordance with the > principle that "feature foo" variables need not be written and should not be > read when feature foo is disabled. libgcc's configure script, for example, has On the contrary, I think an important principle here is that non-multilib and multilib builds follow the same code paths as far as possible, with the multilib variables just set to trivial values (modulo osdirname) in the case of a non-multilib build - a non-multilib build should be building libraries exactly the same, with the same logic and the same variable settings, as the default multilib in a multilib build. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Libraries' configure scripts should not read config-ml.in when multilib is disabled
On Mon, 30 Jul 2018, John Ericson wrote: > That said, it is my tentative understanding that the point of having config-ml > is to cordon-off all the necessarily-multilib-specific logic so it doesn't > pollute everything else. When that script isn't run, I think the Makefiles > already contain default "trivial values" for capitalized MULTI* variables > (which are the only ones actually used by the build itself), yielding > precisely that deduplication of code paths we both want. Well, if there are unnecessary settings in Makefiles the obvious thing to do is to remove them. I think ideally config-ml.in wouldn't contain any conditionals at all on whether multilibs are enabled, and would run just the same whether they are or not, because in the multilib-disabled case -print-multi-lib should print the appropriate line for exactly one multilib, and everything should just follow through from that setting (--disable-multilib should be equivalent to multilibs enabled but only one present). (More clearly, I think the special handling in config-ml.in for various architecture-specific configure options should go away. I don't know why it's there, but multilib-related configure options should result in -print-multi-lib doing the right thing, so top-level doesn't need to do further manipulations on top of that.) To build libraries separately, you can configure at top level with some of the subdirectories or their configure scripts that you don't need removed to avoid them getting built as well, and with --disable-multilib, and with my proposal above for config-ml.in not itself checking for --disable-multilib you might also need CC_FOR_TARGET etc. set to a wrapper that overrides the output of -print-multi-lib. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Avoid infinite loop with duplicate anonymous union fields
On Tue, 31 Jul 2018, Bogdan Harjoc wrote: > With fresh git sources and contrib/gcc_update the tests pass: > > === gcc Summary === > > # of expected passes 133500 > # of expected failures 422 > # of unsupported tests 2104 > > gcc-build/gcc/xgcc version 9.0.0 20180730 (experimental) (GCC) > > I wasn't able to reduce the input to avoid including and as > it only reproduces without -save-temps, it's not clear how to write a > testcase for this one. Could you give more details of the paths through the code that are involved in the infinite loop, and the different paths you get without -save-temps? Is this an issue of dependence on the values of pointers, or something like that? Is it possible to produce a test with more instances of the problem in it, say, so that the probability of the problem showing up at least once with the test is much higher? A binary search should not result in an infinite loop simply because the elements of the array are not sorted; in that case it should just converge on an unpredictable element. So more explanation of how the infinite loop occurs is needed. (But if choice of an unpredictable element results in e.g. subsequent diagnostics varying depending on pointer values, that by itself is a problem that may justify avoiding this code in the case where the array may not be sorted.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 5/5] Formatted printing for dump_* in the middle-end
On Tue, 31 Jul 2018, David Malcolm wrote: > I didn't exhaustively check every callsite to the changed calls; I'm > assuming that -Wformat during bootstrap has effectively checked that > for me. Though now I think about it, I note that we use > HOST_WIDE_INT_PRINT_DEC in many places: is this guaranteed to be a > valid input to pp_format on all of our configurations? HOST_WIDE_INT_PRINT_DEC should not be considered safe with pp_format (although since r197049 may have effectively stopped using %I64 on MinGW hosts, I'm not sure if there are current cases where it won't work). Rather, it is the job of pp_format to map the 'w' length specifier to HOST_WIDE_INT_PRINT_DEC etc. I think it clearly makes for cleaner code to limit use of HOST_WIDE_INT_PRINT_* to as few places as possible and to prefer use of internal printf-like functions that accept formats such as %wd where possible. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Fix endless loop in the C FE initializer handling (PR c/85704)
On Tue, 24 Jul 2018, Jakub Jelinek wrote: > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and > release branches? > > 2018-07-24 Jakub Jelinek > > PR c/85704 > * c-typeck.c (field_decl_cmp): New function. > (output_pending_init_elements): Use it for field comparisons > instead of pure bit_position comparisons. > > * gcc.c-torture/compile/pr85704.c: New test. OK, though I'm a bit uneasy about both c-decl.c and c-typeck.c having static field_decl_cmp functions that do completely different comparisons. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Fix endless loop in the C FE initializer handling (PR c/85704)
On Tue, 31 Jul 2018, Jakub Jelinek wrote: > On Tue, Jul 31, 2018 at 08:04:58PM +0000, Joseph Myers wrote: > > On Tue, 24 Jul 2018, Jakub Jelinek wrote: > > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and > > > release branches? > > > > > > 2018-07-24 Jakub Jelinek > > > > > > PR c/85704 > > > * c-typeck.c (field_decl_cmp): New function. > > > (output_pending_init_elements): Use it for field comparisons > > > instead of pure bit_position comparisons. > > > > > > * gcc.c-torture/compile/pr85704.c: New test. > > > > OK, though I'm a bit uneasy about both c-decl.c and c-typeck.c having > > static field_decl_cmp functions that do completely different comparisons. > > So would you like me to rename it somehow? > init_field_decl_cmp, or field_cmp, or fld_cmp, something else? init_field_decl_cmp seems reasonable (makes clearer what this particular comparison is for). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Avoid infinite loop with duplicate anonymous union fields
On Wed, 1 Aug 2018, Bogdan Harjoc wrote: > So array[0] < component < array[2], which loops (I removed the gdb p > commands for field_array[1] and so on). Is the key thing here that you end up with DECL_NAME (field) == NULL_TREE, but DECL_NAME (field_array[bot]) != NULL_TREE - and in this particular case of a bad ordering only, it's possible to loop without either top or bot being changed? (But other details of the DECL_NAME ordering are needed to actually get to that particular point.) seen_error () is the idiomatic way of testing whether an error has been reported. -- Joseph S. Myers jos...@codesourcery.com
Re: [V8][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650]
On Thu, 25 May 2023, Qing Zhao via Gcc-patches wrote: > > On May 25, 2023, at 4:51 PM, Joseph Myers wrote: > > > > The documentation in this case is OK, though claims about how a future > > version will behave have a poor track record (we tend to end up with such > > claims persisting in the documentation even though the change in question > > didn't get made and might sometimes no longer be considered desirable). > Then, do you have any suggestions on this claim? Shall we delete it from > the doc? Or keep it? My suggestion would be just to say the feature is deprecated without saying what a future version will do - also make sure to say it's deprecated in the GCC 14 release notes, and then if GCC 15 starts to warn, put something in the GCC 15 release notes as well. -- Joseph S. Myers jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Fri, 26 May 2023, Qing Zhao via Gcc-patches wrote: > > What if the string is a wide string? I don't expect that to work (either > > as a matter of interface design, or in the present code), but I think that > > case should have a specific check and error. > > Dump question: how to check whether the string is a wide string? -:) By examining the element type; the only valid case for the attribute would be an element type of (const) char. (I think it's reasonable to reject all of char8_t, char16_t, char32_t, wchar_t strings in this context.) -- Joseph S. Myers jos...@codesourcery.com
Re: [V8][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650]
On Fri, 26 May 2023, Qing Zhao via Gcc-patches wrote: > Another question: is it better for me to rearrange the Patch 1/2 and Patch > 2/2 a little bit, > to put the FE , doc change and corresponding testing case together into one > patch, (you have approved the FE part of change in Patch 1/2). > and then the mid-end change to tree-ojbect-size.cc and the corresponding > testing cases to another patch? I don't really see this patch as needing to be split up into multiple parts at all. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] -Wstringop-overflow for parameters with forward-declared sizes
On Fri, 26 May 2023, Martin Uecker via Gcc-patches wrote: > c: -Wstringop-overflow for parameters with forward-declared sizes > > Warnings from -Wstringop-overflow do not appear for parameters declared > as VLAs when the bound refers to a parameter forward declaration. This > is fixed by splitting the loop that passes through parameters into two, > first only recording the positions of all possible size expressions > and then processing the parameters. > > PR c/109970 > > gcc/c-family: > > * c-attribs.cc (build_attr_access_from_parms): Split loop to first > record all parameters. > > gcc/testsuite: > > * gcc.dg/pr109970.c: New test. > OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH 3/4] introduce ubsan checking for assigment of VM types 3/4
On Mon, 29 May 2023, Martin Uecker via Gcc-patches wrote: > Support instrumentation of function arguments for functions > called via a declaration. We can support only simple size What do you mean by "via a declaration"? If the *definition* is visible (and known to be the definition used at runtime rather than being interposed) then you can determine in some cases that there is UB from bad bounds. If only some other declaration is visible, or the definition might be interposed, VLA sizes in the declaration are equivalent to [*]; it's suspicious if they don't match, but it's not UB and so it would seem rather questionable for UBSan to treat it as such (cf. the rejection in GCC of sanitization for some questionable cases of unsigned integer overflow that aren't UB either). > + /* Give up. If we do not understand a size expression, we can > + also not instrument any of the others because it may have > + side effects affecting them. (We could restart and instrument > + the only the ones with integer constants.) */ > + warning_at (location, 0, "Function call not instrumented."); > + return void_node; This is not a properly formatted diagnostic message (should start with a lowercase letter and not end with '.'). -- Joseph S. Myers jos...@codesourcery.com
Re: [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650]
On Tue, 30 May 2023, Qing Zhao via Gcc-patches wrote: > Joseph, > > could you please review this patch and see whether it's Okay for commit > now? This version is OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Wed, 7 Jun 2023, Qing Zhao via Gcc-patches wrote: > Hi, Joseph, > > A question here: can an identifier in C be a wide char string? Identifiers and strings are different kinds of tokens; an identifier can't be a string of any kind, wide or narrow. It just so happens that the proposed interface here involves interpreting the contents of a string as referring to an identifier (presumably for parsing convenience compared to using an identifier directly in an attribute). -- Joseph S. Myers jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Wed, 7 Jun 2023, Qing Zhao via Gcc-patches wrote:
> Are you suggesting to use identifier directly as the argument of the
> attribute?
> I tried this in the beginning, however, the current parser for the attribute
> argument can not identify that this identifier is a field identifier inside
> the same structure.
>
> For example:
>
> int count;
> struct trailing_array_7 {
> Int count;
> int array_7[] __attribute ((element_count (count)));
> };
>
> The identifier “count” inside the attribute will refer to the variable
> “int count” outside of the structure.
c_parser_attribute_arguments is supposed to allow an identifier as an
attribute argument - and not look it up (the user of the attribute would
later need to look it up in the context of the containing structure).
Callers use attribute_takes_identifier_p to determine which attributes
take identifiers (versus expressions) as arguments, which would need
updating to cover the new attribute.
There is a ??? comment about the case where the identifier is declared as
a type name. That would simply be one of the cases carried over from the
old Bison parser, and it would seem reasonable to remove that
special-casing so that the attribute works even when the identifier is
declared as a typedef name as an ordinary identifier, since it's fine for
structure members to have the same name as a typedef name.
Certainly taking an identifier directly seems like cleaner syntax than
taking a string that then needs reinterpreting as an identifier.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [RFC] Add stdckdint.h header for C23
On Sat, 10 Jun 2023, Jakub Jelinek via Gcc-patches wrote: > I have looked at gnulib stdckdint.h and they are full of workarounds > for various compilers, EDG doesn't do this, clang <= 14 can't multiply > __int128, ..., so I think the header belongs into the compiler rather > than C library, because it would be a nightmare to maintain it there. While C2x only has type-generic macros in this header, there's a proposal N2868 (which didn't get consensus for C2x but may come back for a future standard version) for additional interfaces for structure types with a sticky overflow flag, including some functions that are expected to be defined with external linkage as usual for library functions. So if that gets adopted in future, we'd need to arrange to provide those library functions with external linkage - which is mostly not something we do in GCC, although there are a few atomic_* functions in libatomic in addition to the __atomic_* functions underlying type-generic macros. > What I'm struggling with is enforcing the weird restrictions > C23 imposes on these. It's not clear all those restrictions need to be enforced - this definitely seems like a case of undefined behavior to provide useful extension space, where for various of those restrictions there are unique sensible semantics to provide if the types in question are supported. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] Add stdckdint.h header for C23
On Tue, 13 Jun 2023, Jakub Jelinek via Gcc-patches wrote: > There is always the possibility to have the header co-owned by both > the compiler and C library, limits.h style. > Just > #if __has_include_next() > # include_next > #endif > perhaps guarded with some macro at the end of the GCC version and > do the same at the start of the glibc version again perhaps with some macro. > And leave the compiler specific part to the compiler (perhaps with some > fallback in the libc version if the compiler specific part is missing) and > have the library related part be provided by the C library? This seems a reasonable approach. If the structure types do make it into future standard versions, we'd need to work out exactly where the division is between compiler and library header responsibilities (where those pieces involving structure types but not library functions go, for example). For operations using structure types with overflow flag we'd need also to work out to what extent it's appropriate to implement those purely in the header with _Generic versus adding new built-in functions or extending what the existing ones can do. > > > What I'm struggling with is enforcing the weird restrictions > > > C23 imposes on these. > > > > It's not clear all those restrictions need to be enforced - this > > definitely seems like a case of undefined behavior to provide useful > > extension space, where for various of those restrictions there are unique > > sensible semantics to provide if the types in question are supported. > > So why does C2X say > Recommended practice > It is recommended to produce a diagnostic message if type2 or type3 are > not suitable integer types, or if *result is not a modifiable lvalue of > a suitable integer type. > ? > Or is it meant that a suitable integer type doesn't need to be necessarily > one that is listed in the previous paragraph? > Perhaps the checking could be guarded on #ifdef __STRICT_ANSI__, sure... Diagnostics are better than doing something completely random - but making it conditional when there are sensible semantics also makes sense. It seems likely a future standard version will support these operations for bit-precise types, at least. (Bit-precise types are generally tricky for type-generic operations; there's no standard way to select on them with _Generic beyond listing individual types with specific widths, and no standard way to determine the width of the bit-precise type of an argument. So implementing some type-generic operations for such types may need new language extensions, prompting WG14 caution about requiring such support - but this also makes support for such types in standard type-generic macros in headers particularly valuable, precisely because they can't be implemented purely in user code using standard language features.) -- Joseph S. Myers jos...@codesourcery.com
Re: [ping] driver: Forward '-lgfortran', '-lm' to offloading compilation
On Tue, 13 Jun 2023, Thomas Schwinge wrote: > Hi! > > On 2023-06-05T14:25:18+0200, I wrote: > > OK to push the attached > > "driver: Forward '-lgfortran', '-lm' to offloading compilation"? > > (We didn't have a PR open for that, or did we?) > > Ping. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] Add stdckdint.h header for C23
On Tue, 13 Jun 2023, Jakub Jelinek via Gcc-patches wrote:
> Yeah, having say __builtin_{clz,ctz,ffs,popcount,parity} variants which would
> be typegeneric and would allow say any normal integral or _BitInt type
> (or just unsigned versions thereof?) would be useful. Even without _BitInt
> we have the problem that we don't have builtins for __uint128_t.
>
> One question is if we should keep them UB on zero input or hardcode some
> particular
> behavior for clz/ctz. The backend defaults might not be appropriate, I
> think if we'd make it non-UB, using the precision of the type would be
> reasonable, whatever it is (__builtin_clzb ((unsigned _BitInt(126)) 0)
> might be 126 etc.).
FWIW the C2x stdbit.h operations all have defined semantics on special
cases, except for the stdc_bit_ceil operations (where there's an NB
comment on CD2 to be considered at next week's WG14 meeting requesting
defined semantics there as well). They're also all for unsigned
arguments. (Note there are also NB comments requesting removal of some of
the operations as duplicates or near-duplicates of others.)
The stdbit.h header does seem naturally to be something for libc, given
that (a) it has a lot of functions, not just type-generic macros, and (b)
the type-generic macros are generally easy to implement (at least for the
types supported in the standard) in a way that doesn't depend on any
compiler extensions (or even on _Generic, many of them can be implemented
just to call the function for unsigned long long). It makes sense in due
course for GCC to know the names there (after any get removed) as built-in
functions, but mapping in a header to existing __builtin_* is generally
easy until then.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [RFC] Add stdckdint.h header for C23
On Tue, 13 Jun 2023, Paul Eggert wrote: > > There is always the possibility to have the header co-owned by both > > the compiler and C library, limits.h style. > > Just > > #if __has_include_next() > > # include_next > > #endif > > I don't see how you could implement __has_include_next() for > arbitrary non-GCC compilers, which is what we'd need for glibc users. For > glibc internals we can use "#include_next" more readily, since we assume a > new-enough GCC. I.e. we could do something like this: Given the possibility of library functions being included in in future standard versions, it seems important to look at ways of splitting responsibility for the header between the compiler and library, whether with __has_include_next, or compiler version conditionals, or some other such variation. -- Joseph S. Myers jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Thu, 15 Jun 2023, Qing Zhao via Gcc-patches wrote:
> Comparing B with A, I don’t see too much benefit, either from
> user-interface point of view, or from implementation point of view.
>
> For implementation, both A and B need to search the fields of the
> containing structure by the name of the field “count”.
>
> For user interface, I think that A and B are similar.
But as a language design matter, there are no standard C interfaces that
interpret a string as an identifier, so doing so does not fit well with
the language.
> 1. Update the routine “c_parser_postfix_expression” (is this the right
> place? ) to accept the new designator syntax.
Any design that might work with an expression is the sort of thing that
would likely involve many iterations on the specification (i.e. proposed
wording changes to the C standard) for the interpretation of the new kinds
of expressions, including how to resolve syntactic ambiguities and how
name lookup works, before it could be considered ready to implement, and
then a lot more work on the specification based on implementation
experience.
Note that no expressions can start with the '.' token at present. As soon
as you invent a new kind of expression that can start with that token, you
have syntactic ambiguity.
struct s1 { int c; char a[(struct s2 { int c; char b[.c]; }) {.c=.c}.c]; };
Is ".c=.c" a use of the existing syntax for designated initializers, with
the first ".c" being a designator and the second being a use of the new
kind of expression, or is it an assignment expression, where both the LHS
and the RHS of the assignment use the new kind of expression? And do
those .c, when the use the new kind of expression, refer to the inner or
outer struct definition?
There are obvious advantages to using tokens that don't introduce such an
ambiguity with designators (i.e., not '.' as the token to start the new
kind of expression, but something that cannot start a designator), if such
tokens can be found. But you still have the name lookup question when
there are multiple nested structure definitions. And the question of when
expressions are considered to be evaluated, if they have side effects such
as ".c=.c" does.
"Whatever falls out of the implementation" is not a good approach for
language design here. If you want a new kind of expressions here, you
need a careful multi-implementation design phase that produces a proper
specification and has good reasons for the particular choices made in
cases of ambiguity.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Thu, 15 Jun 2023, Qing Zhao via Gcc-patches wrote:
> B. The argument of the new attribute “counted_by” is an identifier that can be
> accepted by “c_parser_attribute_arguments”:
>
> struct trailing_array_B {
> Int count;
> int array_B[] __attribute ((counted_by (count)));
> };
>
>
> From my current very limited understanding of the C FE source code, it’s
> not easy to extend the argument to an expression later for the above. Is
> this understanding right?
It wouldn't be entirely compatible: if you change to interpreting the
argument as an expression, then the above would suggest a global variable
count is used (as opposed to some other syntax for referring to an element
of the containing structure).
So an attribute that takes an element name might best be a *different*
attribute from any potential future one taking an expression (with some
new syntax to refer to an element).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [WIP RFC] Add support for keyword-based attributes
On Mon, 17 Jul 2023, Michael Matz via Gcc-patches wrote: > So, essentially you want unignorable attributes, right? Then implement > exactly that: add one new keyword "__known_attribute__" (invent a better > name, maybe :) ), semantics exactly as with __attribute__ (including using > the same underlying lists in our data structures), with only one single > deviation: instead of the warning you give an error for unhandled > attributes. Done. Assuming you also want the better-defined standard rules about how [[]] attributes appertain to particular entities, rather than the different __attribute__ rules, that would suggest something like [[!some::attr]] for the case of attributes that can't be ignored but otherwise are handled like standard [[]] attributes. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] range-op-float: Fix up -frounding-math frange_arithmetic +- handling [PR110755]
On Mon, 24 Jul 2023, Jakub Jelinek via Gcc-patches wrote: > I believe it is only +- which has this problematic behavior and I think fma has the same property (of rounding-mode-dependent exact results), but I think that's not relevant here? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 0/5] GCC _BitInt support [PR102989]
On Thu, 27 Jul 2023, Jakub Jelinek via Gcc-patches wrote:
> - _BitInt(N) bit-fields aren't supported yet (the patch rejects them); I'd
> like
> to enable those incrementally, but don't really see details on how such
> bit-fields should be laid-out in memory nor passed inside of function
> arguments; LLVM implements something, but it is a question if that is what
> the various ABIs want
So if the x86-64 ABI (or any other _BitInt ABI that already exists)
doesn't specify this adequately then an issue should be filed (at
https://gitlab.com/x86-psABIs/x86-64-ABI/-/issues in the x86-64 case).
(Note that the language specifies that e.g. _BitInt(123):45 gets promoted
to _BitInt(123) by the integer promotions, rather than left as a type with
the bit-field width.)
> - conversions between large/huge (see later) _BitInt and _Decimal{32,64,128}
> aren't support and emit a sorry; I'm not familiar enough with DFP stuff
> to implement that
Doing things incrementally might indicate first doing this only for BID
(so sufficing for x86-64), with DPD support to be added when _BitInt
support is added for an architecture using DPD, i.e. powerpc / s390.
This conversion is a mix of base conversion and things specific to DFP
types.
For conversion *from DFP to _BitInt*, the DFP value needs to be
interpreted (hopefully using existing libbid code) as the product of a
sign, an integer and a power of 10, with appropriate truncation of the
fractional part if there is one (and appropriate handling of infinity /
NaN / values where the integer part obviously doesn't fit in the type as
raising "invalid" and returning an arbitrary result). Then it's just a
matter of doing an integer multiplication and producing an appropriately
signed result (which might itself overflow the range of representable
values with the given sign, meaning "invalid" should be raised).
Precomputed tables of powers of 10 in binary might speed up the
multiplication process (don't know if various existing tables in libbid
are usable for that). It's unspecified whether "inexact" is raised for
non-integer DFP values.
For conversion *from _BitInt to DFP*, the _BitInt value needs to be
expressed in decimal. In the absence of optimized multiplication /
division for _BitInt, it seems reasonable enough to do this naively
(repeatedly dividing by a power of 10 that fits in one limb to determine
base 10^N digits from the least significant end, for example), modulo
detecting obvious overflow cases up front (if the absolute value is at
least 10^97, conversion to _Decimal32 definitely overflows in all rounding
modes, for example, so you just need to do an overflowing computation that
produces a result with the right sign in order to get the correct
rounding-mode-dependent result and exceptions). Probably it isn't
necessary to convert most of those base 10^N digits into base 10 digits.
Rather, it's enough to find the leading M (= precision of the DFP type in
decimal digits) base 10 digits, plus to know whether what follows is
exactly 0, exactly 0.5, between 0 and 0.5, or between 0.5 and 1.
Then adding two appropriate DFP values with the right sign produces the
final DFP result. Those DFP values would need to be produced from integer
digits together with the relevant power of 10. And there might be
multiple possible choices for the DFP quantum exponent; the preferred
exponent for exact results is 0, so the resulting exponent needs to be
chosen to be as close to 0 as possible (which also produces correct
results when the result is inexact). (If the result is 0, note that
quantum exponent of 0 is not the same as the zero from default
initialization, which has the least exponent possible.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 5/5] testsuite part 2 for _BitInt support [PR102989]
I think there should be tests for _Atomic _BitInt types. Hopefully atomic compound assignment just works via the logic for compare-and-exchange loops, but does e.g. atomic_fetch_add work with _Atomic _BitInt types? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 0/5] GCC _BitInt support [PR102989]
On Fri, 28 Jul 2023, Jakub Jelinek via Gcc-patches wrote:
> I had a brief look at libbid and am totally unimpressed.
> Seems we don't implement {,unsigned} __int128 <-> _Decimal{32,64,128}
> conversions at all (we emit calls to __bid_* functions which don't exist),
That's bug 65833.
> the library (or the way we configure it) doesn't care about exceptions nor
> rounding mode (see following testcase)
And this is related to the never-properly-resolved issue about the split
of responsibility between libgcc, libdfp and glibc.
Decimal floating point has its own rounding mode, set with fe_dec_setround
and read with fe_dec_getround (so this test is incorrect). In some cases
(e.g. Power), that's a hardware rounding mode. In others, it needs to be
implemented in software as a TLS variable. In either case, it's part of
the floating-point environment, so should be included in the state
manipulated by functions using fenv_t or femode_t. Exceptions are shared
with binary floating point.
libbid in libgcc has its own TLS rounding mode and exceptions state, but
the former isn't connected to fe_dec_setround / fe_dec_getround functions,
while the latter isn't the right way to do things when there's hardware
exceptions state.
libdfp - https://github.com/libdfp/libdfp - is a separate library, not
part of libgcc or glibc (and with its own range of correctness bugs) -
maintained, but not very actively (maybe more so than the DFP support in
GCC - we haven't had a listed DFP maintainer since 2019). It has various
standard DFP library functions - maybe not the full C23 set, though some
of the TS 18661-2 functions did get added, so it's not just the old TR
24732 set. That includes its own version of the libgcc support, which I
think has some more support for using exceptions and rounding modes. It
includes the fe_dec_getround and fe_dec_setround functions. It doesn't do
anything to help with the issue of including the DFP rounding state in the
state manipulated by functions such as fegetenv.
Being a separate library probably in turn means that it's less likely to
be used (although any code that uses DFP can probably readily enough
choose to use a separate library if it wishes). And it introduces issues
with linker command line ordering, if the user intends to use libdfp's
copy of the functions but the linker processes -lgcc first.
For full correctness, at least some functionality (such as the rounding
modes and associated inclusion in fenv_t) would probably need to go in
glibc. See
https://sourceware.org/pipermail/libc-alpha/2019-September/106579.html
for more discussion.
But if you do put some things in glibc, maybe you still don't want the
_BitInt conversions there? Rather, if you keep the _BitInt conversions in
libgcc (even when the other support is in glibc), you'd have some
libc-provided interface for libgcc code to get the DFP rounding mode from
glibc in the case where it's handled in software, like some interfaces
already present in the soft-float powerpc case to provide access to its
floating-point state from libc (and something along the lines of
sfp-machine.h could tell libgcc how to use either that interface or
hardware instructions to access the rounding mode and exceptions as
needed).
> and for integral <-> _Decimal32
> conversions implement them as integral <-> _Decimal64 <-> _Decimal32
> conversions. While in the _Decimal32 -> _Decimal64 -> integral
> direction that is probably ok, even if exceptions and rounding (other than
> to nearest) were supported, the other direction I'm sure can suffer from
> double rounding.
Yes, double rounding would be an issue for converting 64-bit integers to
_Decimal32 via _Decimal64 (it would be fine to convert 32-bit integers
like that since they can be exactly represented in _Decimal64; it would be
fine to convert 64-bit integers via _Decimal128).
> So, wonder if it wouldn't be better to implement these in the soft-fp
> infrastructure which at least has the exception and rounding mode support.
> Unlike DPD, decoding BID seems to be about 2 simple tests of the 4 bits
> below the sign bit and doing some shifts, so not something one needs a 10MB
> of a library for. Now, sure, 5MB out of that are generated tables in
Note that representations with too-large significand are defined to be
noncanonical representations of zero, so you need to take care of that in
decoding BID.
> bid_binarydecimal.c, but unfortunately those are static and not in a form
> which could be directly fed into multiplication (unless we'd want to go
> through conversions to/from strings).
> So, it seems to be easier to guess needed power of 10 from number of binary
> digits or vice versa, have a small table of powers of 10 (say those which
> fit into a limb) and construct larger powers of 10 by multiplicating those
> several times, _Decimal128 has exponent up to 6144 which is ~ 2552 bytes
> or 319 64-bit limbs, but having a table with all the 6144 po
Re: _BitInt vs. _Atomic
On Fri, 28 Jul 2023, Jakub Jelinek via Gcc-patches wrote: > The C++ way of dealing with this is using __builtin_clear_padding, > done on atomic stores/updates of the atomic memory (padding is cleared > if any on the value to be stored, or on the expected and desired values). > > I don't know enough about the C atomic requirements whether that is feasible > for it as well, or whether it is possible to make the padding bits partially > or fully set somehow non-atomically without invoking UB and then make it > never match. If padding bits not being reliably preserved causes problems for the compare-exchange loops in C in practice, then it would seem reasonable to use __builtin_clear_padding internally as part of implementing those cases of atomic compound assignment. > And another issue is that while __atomic_load, __atomic_store, > __atomic_exchange and __atomic_compare_exchange work on arbitrary _BitInt > sizes, others like __atomic_fetch_add only support _BitInt or other integral > types which have size of 1, 2, 4, 8 or 16 bytes, others emit an error > in c-family/c-common.cc (sync_resolve_size). So, either > resolve_overloaded_builtin should for the case when pointer is pointer to > _BitInt which doesn't have 1, 2, 4, 8 or 16 bytes size lower those into > a loop using __atomic_compare_exchange (or perhaps also if there is > padding), or should do that. The interfaces definitely need to work with _BitInt. My guess is that doing this with the built-in expansion would be more robust than putting more complicated definitions in the header that choose which built-in functions to use depending on properties of the type (and keeping the built-in functions limited to certain widths), but I don't know. Note also that those operations have no undefined behavior on signed integer overflow. If any ABIs require sign / zero extension of _BitInt values in memory, care would also be needed in the case of (size of 1, 2, 4, 8 or 16 bytes, but also has high bits required to be sign / zero extended) to ensure that the operations are implemented so as to leave the high bits with the expected values in case of overflow, which wouldn't result from simply using the underlying operation for a type with the full precision of its memory size. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC WIP PATCH] _BitInt bit-field support [PR102989]
On Fri, 28 Jul 2023, Jakub Jelinek via Gcc-patches wrote: > But I ran into a compiler divergence on _Generic with bit-field expressions. > My understanding is that _Generic controlling expression undergoes array > to pointer and function to pointer conversions, but not integral promotions > (otherwise it would never match for char, short etc. types). > C23 draft I have says: > "A bit-field is interpreted as having a signed or unsigned integer type > consisting of the specified number of bits" > but doesn't say which exact signed or unsigned integer type that is. Yes, the type used in _Generic isn't fully specified, just the type after integer promotions in contexts where those occur. > static_assert (expr_has_type (s4.c + 1uwb, _BitInt(389))); > static_assert (expr_has_type (s4.d * 0wb, _BitInt(2))); > static_assert (expr_has_type (s6.a + 0wb, _BitInt(2))); > That looks to me like LLVM bug, because > "The value from a bit-field of a bit-precise integer type is converted to > the corresponding bit-precise integer type." > specifies that s4.c has _BitInt(389) type after integer promotions > and s4.d and s6.a have _BitInt(2) type. Now, 1uwb has unsigned _BitInt(1) > type and 0wb has _BitInt(2) and the common type for those in all cases is > I believe the type of the left operand. Indeed, I'd expect those to pass, since in those cases integer promotions (to the declared _BitInt type of the bit-field, without the bit-field width) are applied. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH RESEND] c: add -Wmissing-variable-declarations [PR65213]
On Tue, 18 Jul 2023, Hamza Mahfooz wrote: > Resolves: > PR c/65213 - Extend -Wmissing-declarations to variables [i.e. add > -Wmissing-variable-declarations] > > gcc/c-family/ChangeLog: > > PR c/65213 > * c.opt (-Wmissing-variable-declarations): New option. > > gcc/c/ChangeLog: > > PR c/65213 > * c-decl.cc (start_decl): Handle > -Wmissing-variable-declarations. > > gcc/ChangeLog: > > PR c/65213 > * doc/invoke.texi (-Wmissing-variable-declarations): Document > new option. > > gcc/testsuite/ChangeLog: > > PR c/65213 > * gcc.dg/Wmissing-variable-declarations.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] gcc-ar: Handle response files properly [PR77576]
This patch is OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH v2] c-family: Implement pragma_lex () for preprocess-only mode
On Fri, 28 Jul 2023, Jason Merrill via Gcc-patches wrote: > > Thanks, I had thought there could be a potential issue with needing to also > > check cpp_get_options(pfile)->traditional. But looking at it more, there's > > no > > code path currently that can end up here in traditional mode, so yes we can > > eliminate stream_tokens_to_preprocessor and just check flag_preprocess_only. > > > > The attached simplified patch does this, bootstrap + regtest look good as > > well. > > LGTM, I'll let the C maintainers comment on the C parser change. The C parser change is OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PING 3] [PATCH] Less warnings for parameters declared as arrays [PR98541, PR98536]
On Mon, 31 Jul 2023, Martin Uecker via Gcc-patches wrote: > Joseph, I would appreciate if you could take a look at this? > > This fixes the remaining issues which requires me to turn the > warnings off with -Wno-vla-parameter and -Wno-nonnull in my > projects. The front-end changes are OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH RESEND] c: add -Wmissing-variable-declarations [PR65213]
On Mon, 31 Jul 2023, Hamza Mahfooz wrote: > Hey Joseph, > > On Fri, Jul 28 2023 at 08:32:31 PM +00:00:00, Joseph Myers > wrote: > > > OK. > > > > -- > > Joseph S. Myers > > jos...@codesourcery.com > > Since I don't have write access, do you mind pushing this for me? Done. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] preprocessor: c++: Support `#pragma GCC target' macros [PR87299]
On Mon, 31 Jul 2023, Lewis Hyatt via Gcc-patches wrote: > I added some additional testcases from the PR for x86. The other targets > that support `#pragma GCC target' (aarch64, arm, nios2, powerpc, s390) > already had tests verifying that the pragma sets macros as expected; here I > have added -save-temps to some of them, to test that it now works in > preprocess-only mode as well. It would seem better to have copies of the tests with and without -save-temps, to test in both modes, rather than changing what's tested by an existing test here. Or a test variant that #includes the original test but uses different options, if the original test isn't doing anything that would fail to work with that approach. -- Joseph S. Myers jos...@codesourcery.com
Re: _BitInt vs. _Atomic
On Tue, 1 Aug 2023, Michael Matz via Gcc-patches wrote: > Only because cmpxchg is defined in terms of memcpy/memcmp. If it were > defined in terms of the == operator (obviously applied recursively > member-wise for structs) and simple-assignment that wouldn't be a problem. It also wouldn't work for floating point, where I think clearly the atomic operations should consider positive and negative zero as different, and should consider different DFP quantum exponents for the same real number as different - but should also consider the same NaN (same payload, same choice of quiet / signaling) as being the same. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/2] RISC-V: Support _Float16 type.
On Thu, 7 Jul 2022, Kito Cheng wrote:
> +/* Implement TARGET_MANGLE_TYPE. */
> +
> +static const char *
> +riscv_mangle_type (const_tree type)
> +{
> + /* Half-precision float. */
> + if (TREE_CODE (type) == REAL_TYPE && TYPE_PRECISION (type) == 16)
> +return "Dh";
Are you sure you wish to use "Dh" instead of "DF16_" used on x86? The C++
ABI lists both
::= Dh # IEEE 754r half-precision floating point (16 bits)
::= DF _ # ISO/IEC TS 18661 binary floating point
type _FloatN (N bits)
without distinguishing which should be used when - maybe the choice made
for _Float16 on RISC-V needs documenting in the RISC-V psABI?
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] preprocessor: Set input_location to the most recently seen token
On Mon, 11 Jul 2022, Lewis Hyatt via Gcc-patches wrote: > Hello- > > As discussed here: > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598136.html > > Here is another short patch that improves "#pragma GCC diagnostic" handling. > Longer term, it will be desirable to make the handling of this pragma > independent of the global input_location. But in the meantime, some glitches > like this one can be readily addressed by making input_location point to > something better. In this case, input_location during preprocessing (-E or > -save-temps) is made to point to the most recently seen token rather than the > beginning of the file. To the best of my knowledge, nothing else besides > "#pragma GCC diagnostic" handling can observe input_location during > token streaming, so this is expected not to have any other > repercussions. Bootstrap + regtest does look clean on x86-64 Linux. > > By the way, the new testcase fails when compiled with C++, but it's not > because of pragma handling, it's rather because the C++ frontend changes the > location on the warning to the wrong place. Once done_lexing has been set to > true, it changes the location of all warnings to input_location, however > that's not correct when the location is the cached location of a macro > definition; the original location is preferable. I will file a separate PR > about that, and have xfailed that testcase for now, since I am not quite there > with grokking the reason it behaves this way, and anyway it's not related to > this 1-line fix for gcc -E. > > Please let me know how it looks? Thanks! OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] doc: Clarify FENV_ACCESS pragma semantics WRT `-ftrapping-math'
On Tue, 19 Jul 2022, Maciej W. Rozycki wrote: > Our documentation indicates that it is the `-frounding-math' invocation > option that controls whether we respect what the FENV_ACCESS pragma > would imply, should we implement it, regarding the floating point > environment. It is only a part of the picture however, because the > `-ftrapping-math' invocation option also affects how we handle said > environment. Clarify that in the description of both options then, as > well as the FENV_ACCESS pragma itself. > > gcc/ > * doc/implement-c.texi (Floating point implementation): Mention > `-fno-trapping-math' in the context of FENV_ACCESS pragma. > * doc/invoke.texi (Optimize Options): Clarify FENV_ACCESS pragma > implication in the descriptions of `-fno-trapping-math' and > `-frounding-math'. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/1] c++/106423: Fix pragma suppression of -Wc++20-compat diagnostics.
On Sun, 24 Jul 2022, Tom Honermann via Gcc-patches wrote: > Gcc's '#pragma GCC diagnostic' directives are processed in "early mode" > (see handle_pragma_diagnostic_early) for the C++ frontend and, as such, > require that the target diagnostic option be enabled for the preprocessor > (see c_option_is_from_cpp_diagnostics). This change modifies the > -Wc++20-compat option definition to register it as a preprocessor option > so that its associated diagnostics can be suppressed. The changes also There are lots of C++ warning options, all of which should support pragma suppression regardless of whether they are relevant to the preprocessor or not. Do they all need this kind of handling, or is it only -Wc++20-compat that has some kind of problem? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/3] C: Implement C2X N2653 char8_t and UTF-8 string literal changes
On Mon, 25 Jul 2022, Tom Honermann via Gcc-patches wrote: > diff --git a/gcc/ginclude/stdatomic.h b/gcc/ginclude/stdatomic.h > index bfcfdf664c7..75ed7965689 100644 > --- a/gcc/ginclude/stdatomic.h > +++ b/gcc/ginclude/stdatomic.h > @@ -49,6 +49,10 @@ typedef _Atomic long atomic_long; > typedef _Atomic unsigned long atomic_ulong; > typedef _Atomic long long atomic_llong; > typedef _Atomic unsigned long long atomic_ullong; > +#if (defined(__CHAR8_TYPE__) \ > + && (defined(_GNU_SOURCE) || defined(_ISOC2X_SOURCE))) > +typedef _Atomic __CHAR8_TYPE__ atomic_char8_t; > +#endif > typedef _Atomic __CHAR16_TYPE__ atomic_char16_t; > typedef _Atomic __CHAR32_TYPE__ atomic_char32_t; > typedef _Atomic __WCHAR_TYPE__ atomic_wchar_t; GCC headers don't test glibc feature test macros such as _GNU_SOURCE and _ISOC2X_SOURCE; they base things only on the standard version (whether directly, or indirectly as via __CHAR8_TYPE__) and standard-defined feature test macros. (There's one exception in glimits.h - testing __USE_GNU, the macro defined internally by glibc's headers - but I don't think that's something we want to emulate in new code.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 2/3] testsuite: Add tests for C2X N2653 char8_t and UTF-8 string literal changes
On Mon, 25 Jul 2022, Tom Honermann via Gcc-patches wrote:
> This change provides new tests for the core language and compiler
> dependent library changes adopted for C2X via WG14 N2653.
I'd expect this patch also to add tests verifying that u8"" strings have
the old type for C11 (unless there are existing such tests, but I don't
see them).
> diff --git a/gcc/testsuite/gcc.dg/atomic/c2x-stdatomic-lockfree-char8_t.c
> b/gcc/testsuite/gcc.dg/atomic/c2x-stdatomic-lockfree-char8_t.c
> new file mode 100644
> index 000..37ea4c8926c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/atomic/c2x-stdatomic-lockfree-char8_t.c
> @@ -0,0 +1,42 @@
> +/* Test atomic_is_lock_free for char8_t. */
> +/* { dg-do run } */
> +/* { dg-options "-std=c2x -D_ISOC2X_SOURCE -pedantic-errors" } */
I don't think _ISOC2X_SOURCE belongs in any GCC tests.
> diff --git a/gcc/testsuite/gcc.dg/atomic/gnu2x-stdatomic-lockfree-char8_t.c
> b/gcc/testsuite/gcc.dg/atomic/gnu2x-stdatomic-lockfree-char8_t.c
> new file mode 100644
> index 000..a017b134817
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/atomic/gnu2x-stdatomic-lockfree-char8_t.c
> @@ -0,0 +1,5 @@
> +/* Test atomic_is_lock_free for char8_t with -std=gnu2x. */
> +/* { dg-do run } */
> +/* { dg-options "-std=gnu2x -D_GNU_SOURCE -pedantic-errors" } */
Nor does _GNU_SOURCE (unless the test depends on glibc functionality
that's only available with _GNU_SOURCE, but in that case you also need
some effective-target conditionals to restrict it to appropriate glibc
targets).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 2/3 v2] testsuite: Add tests for C2X N2653 char8_t and UTF-8 string literal changes
On Mon, 1 Aug 2022, Tom Honermann via Gcc-patches wrote:
> diff --git a/gcc/testsuite/gcc.dg/c2x-predefined-macros.c
> b/gcc/testsuite/gcc.dg/c2x-predefined-macros.c
> new file mode 100644
> index 000..3456105563a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/c2x-predefined-macros.c
> @@ -0,0 +1,11 @@
> +/* Test C2X predefined macros. */
> +/* { dg-do compile } */
> +/* { dg-options "-std=c2x" } */
> +
> +#if !defined(__CHAR8_TYPE__)
> +# error __CHAR8_TYPE__ is not defined!
> +#endif
> +
> +#if !defined(__GCC_ATOMIC_CHAR8_T_LOCK_FREE)
> +# error __GCC_ATOMIC_CHAR8_T_LOCK_FREE is not defined!
> +#endif
These aren't macros defined by C2X. You could argue that they are part of
the stable interface provided by GCC for e.g. libc implementations to use,
and so should be tested as such, but any such test shouldn't suggest it's
testing a standard feature (and should have a better name to describe what
it's actually testing rather than suggesting it's about predefined macros
in general).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 2/3 v3] testsuite: Add tests for C2X N2653 char8_t and UTF-8 string literal changes
On Mon, 1 Aug 2022, Tom Honermann via Gcc-patches wrote: > This change provides new tests for the core language and compiler > dependent library changes adopted for C2X via WG14 N2653. Could you please send a complete patch series? I'm not sure what the matching patches 1 and 3 are. Also, I don't generally find it helpful for tests to be separated from the patch making the changes they test, since tests are necessary to review of that code. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH v4 1/2] C: Implement C2X N2653 char8_t and UTF-8 string literal changes
On Tue, 2 Aug 2022, Tom Honermann via Gcc-patches wrote: > This patch implements the core language and compiler dependent library > changes adopted for C2X via WG14 N2653. The changes include: > - Change of type for UTF-8 string literals from array of const char to > array of const char8_t (unsigned char). > - A new atomic_char8_t typedef. > - A new ATOMIC_CHAR8_T_LOCK_FREE macro defined in terms of the existing > __GCC_ATOMIC_CHAR8_T_LOCK_FREE predefined macro. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH v4 2/2] preprocessor/106426: Treat u8 character literals as unsigned in char8_t modes.
On Tue, 2 Aug 2022, Tom Honermann via Gcc-patches wrote: > This patch corrects handling of UTF-8 character literals in preprocessing > directives so that they are treated as unsigned types in char8_t enabled > C++ modes (C++17 with -fchar8_t or C++20 without -fno-char8_t). Previously, > UTF-8 character literals were always treated as having the same type as > ordinary character literals (signed or unsigned dependent on target or use > of the -fsigned-char or -funsigned char options). OK in the absence of C++ maintainer objections within 72 hours. (This is the case where, when I added support for such literals for C (commit 7c5890cc0a0ecea0e88cc39e9fba6385fb579e61), I raised the question of whether they should be unsigned in the preprocessor for C++ as well.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add warning options -W[no-]compare-distinct-pointer-types
On Fri, 5 Aug 2022, Jose E. Marchesi via Gcc-patches wrote: > +Wcompare-distinct-pointer-types > +C C++ Var(warn_compare_distinct_pointer_types) Warning Init(1) > +Warn if pointers of distinct types are compared without a cast. There's no implementation for C++ in this patch, so the option shouldn't be supported for C++ in c.opt. However, C options are normally supported for Objective-C; unless you have a specific reason why Objective-C support for this option would be a bad idea, "C ObjC" would be appropriate for the languages. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH v4 2/2] preprocessor/106426: Treat u8 character literals as unsigned in char8_t modes.
On Mon, 8 Aug 2022, Tom Honermann via Gcc-patches wrote: > On 8/2/22 6:14 PM, Joseph Myers wrote: > > On Tue, 2 Aug 2022, Tom Honermann via Gcc-patches wrote: > > > > > This patch corrects handling of UTF-8 character literals in preprocessing > > > directives so that they are treated as unsigned types in char8_t enabled > > > C++ modes (C++17 with -fchar8_t or C++20 without -fno-char8_t). > > > Previously, > > > UTF-8 character literals were always treated as having the same type as > > > ordinary character literals (signed or unsigned dependent on target or use > > > of the -fsigned-char or -funsigned char options). > > OK in the absence of C++ maintainer objections within 72 hours. (This is > > the case where, when I added support for such literals for C (commit > > 7c5890cc0a0ecea0e88cc39e9fba6385fb579e61), I raised the question of > > whether they should be unsigned in the preprocessor for C++ as well.) > > Joseph, would you be so kind as to commit this patch series for me? I don't > have commit access. Thank you in advance! Done. -- Joseph S. Myers jos...@codesourcery.com
RE: [PATCH 2/2][frontend]: Add novector C pragma
On Wed, 2 Aug 2023, Tamar Christina via Gcc-patches wrote: > Ping. > > > -Original Message- > > From: Tamar Christina > > Sent: Wednesday, July 26, 2023 8:35 PM > > To: Tamar Christina ; gcc-patches@gcc.gnu.org > > Cc: nd ; jos...@codesourcery.com > > Subject: RE: [PATCH 2/2][frontend]: Add novector C pragma > > > > Hi, This is a respin of the patch taking in the feedback received from the > > C++ > > part. > > > > Simultaneously it's also a ping 😊 OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c-family: Add _BitInt support for __atomic_*fetch* [PR102989]
On Thu, 3 Aug 2023, Jakub Jelinek via Gcc-patches wrote:
> --- gcc/testsuite/gcc.dg/bitint-18.c.jj 2023-08-03 12:26:35.510922996
> +0200
> +++ gcc/testsuite/gcc.dg/bitint-18.c 2023-08-03 12:26:42.114831050 +0200
> @@ -0,0 +1,44 @@
> +/* PR c/102989 */
> +/* { dg-do compile { target bitint } } */
It would be good to have execution tests for these operations (so probably
in gcc.dg/atomic so that libatomic is linked in automatically as needed).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] Add documentation for -Wflex-array-member-not-at-end.
On Thu, 3 Aug 2023, Qing Zhao via Gcc-patches wrote:
> +@opindex Wflex-array-member-not-at-end
> +@opindex Wno-flex-array-member-not-at-end
> +@item -Wflex-array-member-not-at-end
I'd expect this to have @r{(C and C++ only)} to indicate what languages
the option applies to. OK with that change.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [C PATCH] _Generic should not warn in non-active branches [PR68193,PR97100]
On Fri, 4 Aug 2023, Martin Uecker via Gcc-patches wrote: > Here is a patch to reduce false positives in _Generic. > > Bootstrapped and regression tested on x86_64-linux. > > Martin > > c: _Generic should not warn in non-active branches [PR68193,PR97100] > > To avoid false diagnostics, use c_inhibit_evaluation_warnings when > a generic association is known to match during parsing. We may still > generate false positives if the default branch comes earler than > a specific association that matches. > > PR c/68193 > PR c/97100 > > gcc/c/: > * c-parser.cc (c_parser_generic_selection): Inhibit evaluation > warnings branches that are known not be taken during parsing. > > gcc/testsuite/ChangeLog: > * gcc.dg/pr68193.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/5] Middle-end _BitInt support [PR102989]
On Fri, 4 Aug 2023, Richard Biener via Gcc-patches wrote: > > Sorry, I hoped it wouldn't take me almost 3 months and would be much shorter > > as well, but clearly I'm not good at estimating stuff... > > Well, it’s definitely feature creep with now the _Decimal and bitfield stuff … I think feature creep would more be adding new features *outside the scope of the standard* (_BitInt bit-fields and conversions to/from DFP are within the standard, as are _BitInt atomic operations). For example, features to help support type-generic operations on _BitInt, or type-generic versions of existing built-in functions (e.g. popcount) suitable for use on _BitInt - it's likely such features will be of use eventually, but they aren't needed for C23 (where the corresponding type-generic operations only support _BitInt types when they have the same width as some other type), so we can certainly get the standard features in first and think about additional features beyond that later (just as support for wider _BitInt can come later, not being required by the standard). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 00/24] Sync shared build infrastructure with binutils-gdb
On Tue, 8 Aug 2023, Arsen Arsenović via Gcc-patches wrote: > Yes. Libtool was forked over a decade ago. My next project is syncing > upstream and us back up. Unsure about pkg.m4. Note as per previous discussions that libtool commit 3334f7ed5851ef1e96b052f2984c4acdbf39e20c will need reverting in GCC when updating libtool because of incompatible usage of --with-sysroot. Reportedly libtool is based on upstream commit 2c9c38d8a12eb0a2ce7fe9c3862523026c3d5622 (with *many* local changes, some of which may not be present upstream). -- Joseph S. Myers jos...@codesourcery.com
Re: Intel AVX10.1 Compiler Design and Support
Do you have any comments on the interaction of AVX10 with the micro-architecture levels defined in the ABI (and supported with glibc-hwcaps directories in glibc)? Given that the levels are cumulative, should we take it that any future levels will be ones supporting 512-bit vector width for AVX10 (because x86-64-v4 requires the current AVX512F, AVX512BW, AVX512CD, AVX512DQ and AVX512VL) - and so any future processors that only support 256-bit vector width will be considered to match the x86-64-v3 micro-architecture level but not any higher level? -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] GCC Security policy
On Tue, 8 Aug 2023, David Malcolm via Gcc-patches wrote: > However, consider a situation in which someone attempted to, say, embed > libgccjit inside a web browser to generate machine code from > JavaScript, where the JavaScript is potentially controlled by an > attacker. I think we want to explicitly say that that if you're going > to do that, you need to put some other layer of defense in, so that > you're not blithely accepting the inputs to the compilation (sources > and options) from a potentially hostile source, where a crafted input > sources could potentially hit an ICE in the compiler and thus crash the > web browser. A binutils analogue of sorts: you might well want to use objdump etc. on untrusted input, e.g. as part of analysis of a captured malware sample. But if you are using binutils tools in malware analysis, you really, really need to do so in a heavily sandboxed environment, as the malware could well try to exploit any system investigating it. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 00/24] Sync shared build infrastructure with binutils-gdb
On Wed, 9 Aug 2023, Arsen Arsenović via Gcc-patches wrote: > Joseph Myers writes: > > > On Tue, 8 Aug 2023, Arsen Arsenović via Gcc-patches wrote: > > > >> Yes. Libtool was forked over a decade ago. My next project is syncing > >> upstream and us back up. Unsure about pkg.m4. > > > > Note as per previous discussions that libtool commit > > 3334f7ed5851ef1e96b052f2984c4acdbf39e20c will need reverting in GCC when > > updating libtool because of incompatible usage of --with-sysroot. > > I wanted to, somehow, coalesce the two back together, so that both are > available. Presumably, this means adding an option to libtool to accept > host-sysroot or such, but I haven't done too much looking into this. > > Is my interpretation of the issue correct? (i.e. GCC uses sysroot to > mean *target* sysroot rather than host sysroot) GCC's sysroot is a sysroot containing target headers and libraries, yes. Also note: * The --with-sysroot directory is the directory that would be used by GCC if installed in the configured --prefix (relocated if GCC ends up running from a different prefix). At build time, --with-build-sysroot takes precedence. * The --with-sysroot directory (and likewise the --with-build-sysroot directory or any directory specified with --sysroot= passed to GCC programs) may sometimes contain multiple sysroots in subdirectories; that directory gets combined with a suffix from SYSROOT_SUFFIX_SPEC to determine the actual subdirectory for the current multilib, within which there are subdirectories such as usr/include or usr/lib. -- Joseph S. Myers jos...@codesourcery.com
RE: Intel AVX10.1 Compiler Design and Support
On Wed, 9 Aug 2023, Wang, Phoebe via Gcc-patches wrote:
> Proposal 3: Change the ABI of 512-bit vector and always be
> passed/returned from memory.
Changing ABIs like that for existing code that has worked for some time on
existing hardware is a bad idea.
At this point it seems appropriate to remind people of another ABI
consideration for vector extensions. glibc's libmvec defines vector
versions of various functions, including AVX512 ones (of course those
function versions only work on hardware with the relevant instructions).
glibc's headers use both _Pragma ("omp declare simd notinbranch") and
__attribute__ ((__simd__ ("notinbranch"))) to declare, to the compiler
including those headers, what function variants are available in glibc.
Existing glibc versions need to continue to work with new compiler
versions. That is, it's part of the ABI, which must remain stable,
exactly which function versions the above pragma and attribute imply are
available - and of course the details of how those functions versions take
arguments / return results are also part of the ABI (it would be OK for a
new compiler to choose not to use some of those vector versions, but not
to start calling them with a different ABI).
Maybe you'll want to add new vector function versions, with different
interfaces, to libmvec in future. If so, you need a *different* pragma or
attribute to declare to the compiler that the libmvec version using that
pragma or attribute has the additional functions - so new compilers using
the existing header will not try to generate calls to new function
versions that don't exist in that glibc version (but new compilers using a
new header version from new glibc will see the new pragma or attribute and
so be able to generate the relevant calls to new functions). And once
you've defined the ABI for such a new pragma or attribute, that itself
then becomes a stable interface - so if you end up with vector extensions
involving yet another set of interfaces, they need another corresponding
new pragma / attribute for libmvec to declare to the compiler that the new
interfaces exist.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 0/12] GCC _BitInt support [PR102989]
On Wed, 9 Aug 2023, Jakub Jelinek via Gcc-patches wrote:
> - _Complex _BitInt(N) isn't supported; again mainly because none of the psABIs
> mention how those should be passed/returned; in a limited way they are
> supported internally because the internal functions into which
> __builtin_{add,sub,mul}_overflow{,_p} is lowered return COMPLEX_TYPE as a
> hack to return 2 values without using references/pointers
What happens when the usual arithmetic conversions are applied to
operands, one of which is a complex integer type and the other of which is
a wider _BitInt type? I don't see anything in the code to disallow this
case (which would produce an expression with a _Complex _BitInt type), or
any testcases for it.
Other testcases I think should be present (along with any corresponding
changes needed to the code itself):
* Verifying that the new integer constant suffix is rejected for C++.
* Verifying appropriate pedwarn-if-pedantic for the new constant suffix
for versions of C before C2x (and probably for use of _BitInt type
specifiers before C2x as well) - along with the expected -Wc11-c2x-compat
handling (in C2x mode) / -pedantic -Wno-c11-c2x-compat in older modes.
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH] config: Fix host -rdynamic detection for build != host != target
The GCC_ENABLE_PLUGINS configure logic for detecting whether -rdynamic is necessary and supported uses an appropriate objdump for $host binaries (running on $build) in cases where $host is $build or $target. However, it is missing such logic in the case where $host is neither $build nor $target, resulting in the compilers not being linked with -rdynamic and plugins not being usable with such a compiler. In fact $ac_cv_prog_OBJDUMP, as used when $build = $host, is always an objdump for $host binaries that runs on $build; that is, it's appropriate to use in this case as well. Tested in such a configuration that it does result in cc1 being linked with -rdynamic as expected. Also bootstrapped with no regressions for x86_64-pc-linux-gnu. config/ * gcc-plugin.m4 (GCC_ENABLE_PLUGINS): Use export_sym_check="$ac_cv_prog_OBJDUMP -T" also when host is not build or target. gcc/ * configure: Regenerate. libcc1/ * configure: Regenerate. diff --git a/config/gcc-plugin.m4 b/config/gcc-plugin.m4 index c731a6fab38..c30cfdd8fad 100644 --- a/config/gcc-plugin.m4 +++ b/config/gcc-plugin.m4 @@ -49,7 +49,7 @@ AC_DEFUN([GCC_ENABLE_PLUGINS], elif test x$host = x$target; then export_sym_check="$gcc_cv_objdump -T" else -export_sym_check= +export_sym_check="$ac_cv_prog_OBJDUMP -T" fi ;; esac diff --git a/gcc/configure b/gcc/configure index ea1ad6606a6..db5812d4a63 100755 --- a/gcc/configure +++ b/gcc/configure @@ -31975,7 +31975,7 @@ fi elif test x$host = x$target; then export_sym_check="$gcc_cv_objdump -T" else -export_sym_check= +export_sym_check="$ac_cv_prog_OBJDUMP -T" fi ;; esac diff --git a/libcc1/configure b/libcc1/configure index 1a63a0e4e1a..2a914a0bfc8 100755 --- a/libcc1/configure +++ b/libcc1/configure @@ -15120,7 +15120,7 @@ fi elif test x$host = x$target; then export_sym_check="$gcc_cv_objdump -T" else -export_sym_check= +export_sym_check="$ac_cv_prog_OBJDUMP -T" fi ;; esac -- Joseph S. Myers jos...@codesourcery.com
Re: c: Support for -Wuseless-cast [RR84510]
On Thu, 10 Aug 2023, Martin Uecker via Gcc-patches wrote: > c: Support for -Wuseless-cast [RR84510] > > Add support for Wuseless-cast C (and ObjC). > > PR c/84510 > > gcc/c/: > * c-typeck.cc (build_c_cast): Add warning. > > gcc/doc/: > * invoke.texi: Update. > > gcc/testsuite/: > * Wuseless-cast.c: New test. OK. Note "RR" should be "PR" in the commit subject. -- Joseph S. Myers jos...@codesourcery.com
Re: Intel AVX10.1 Compiler Design and Support
On Thu, 10 Aug 2023, Richard Biener via Gcc-patches wrote: > Isn't this situation similar to the not defined ABI when passing generic > vectors (via __attribute__((vector_size))) that do not map to vectors > supported > by the current ISA? There's cases like vector<2> char or vector<1> double > to consider for example that would fit in a lowpart of a supported vector > register and as in the AVX512 case vectors that are larger than any supported > vector register. Note there is a difference in some cases (I don't know if this is relevant for x86) between "vectors supported by the current ISA" and "vectors whose ABI, for ISAs that do support them, can be implemented using the current ISA". Specifically, when working on the VFP AAPCS variant for 32-bit Arm, I made sure that generic vectors had the same ABI on all processors supporting VFP, whether or not the vector parts of the instruction set were supported on the chosen processor. On 32-bit Arm that's possible because vector registers are the same as floating-point registers (and even the single-precision-only VFP variant has suitable load and store instructions). Of course if your ABI for some kinds of vectors uses registers not supported on all processors, and on the processors that do support those registers you use that ABI for corresponding generic vectors, then you won't be able to be compatible with that ABI for those generic vectors on processors without those registers. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c, v2: Add __typeof_unqual__ and __typeof_unqual support
On Thu, 10 Aug 2023, Jakub Jelinek via Gcc-patches wrote: > I'd like to ping this patch. Reposting it as the extend.texi hunk > didn't apply cleanly anymore. Bootstrapped/regtested again on x86_64-linux > and i686-linux, ok for trunk? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c, c++, v2: Accept __builtin_classify_type (typename)
On Thu, 10 Aug 2023, Jason Merrill via Gcc-patches wrote: > On 8/10/23 11:35, Jakub Jelinek wrote: > > Hi! > > > > I'd like to ping this patch. Reposting it as I found a typo in the > > documentation - s/builtin-in/built-in/. Bootstrapped/regtested again > > on x86_64-linux and i686-linux, ok for trunk? > > > > On Mon, Jun 12, 2023 at 09:57:17PM +0200, Jakub Jelinek via Gcc-patches > > wrote: > > > As mentioned in my stdckdint.h mail, __builtin_classify_type has > > > a problem that argument promotion (the argument is passed to ... > > > prototyped builtin function) means that certain type classes will > > > simply never appear. > > > I think it is too late to change how it behaves, lots of code in the > > > wild might rely on the current behavior. > > Hmm, you really think there's any code at all in the wild relying on > __builtin_classify_type + array/function decay? It's a (previously) > undocumented built-in, I wouldn't expect anyone outside the project to be > using it. So at first glance I'd be inclined to fix it whether or not we also > allow it to accept a type. But I don't actually know how it's used, so could > well be wrong... I don't know about array / function decay, but it seems quite plausible that code checking for pointers expects those to occur. Certainly glibc's tgmath.h has code expecting booleans to be treated like other integers (i.e. subject to integer promotions because passed in ...) by __builtin_classify_type (though you'd need a fairly old glibc version to use that implementation of the tgmath.h macros with current GCC rather than one based on __builtin_tgmath). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] stdckdint.h _BitInt test
On Thu, 10 Aug 2023, Jakub Jelinek via Gcc-patches wrote:
> Hi!
>
> The following patch (on top of the stdckdint.h patch and _BitInt patch
> series) adds a test for _BitInt diagnostics of ckd_{add,sub,mul} macros.
I remain unconvinced that diagnosing use with types where it's clear what
the right semantics for the operation are is a good idea (given, in the
_BitInt case, that you've already implemented the built-in functions for
_BitInt types). (Diagnosing for bool results *is* a good idea, since it's
not clear what the semantics should be. Likewise for enums with fixed
underlying type bool, whether or not it's diagnosed for other enums.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] c, v3: Add stdckdint.h header for C23
On Fri, 11 Aug 2023, Jakub Jelinek wrote: > All that is diagnosed is when result is bool or enum (any kind). Even for I'd suggest tests that other nonsense cases are diagnosed, such as floating-point or pointer arguments or results (hopefully such cases are already diagnosed and just need tests). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] c, v4: Add stdckdint.h header for C23
On Fri, 11 Aug 2023, Jakub Jelinek via Gcc-patches wrote: > On Fri, Aug 11, 2023 at 01:25:38PM +0000, Joseph Myers wrote: > > On Fri, 11 Aug 2023, Jakub Jelinek wrote: > > > > > All that is diagnosed is when result is bool or enum (any kind). Even for > > > > I'd suggest tests that other nonsense cases are diagnosed, such as > > floating-point or pointer arguments or results (hopefully such cases are > > already diagnosed and just need tests). > > So like this then? I think it should also test the diagnostic for when *result is const-qualified. OK with that change. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH v3] LoongArch:Implement 128-bit floating point functions in gcc.
On Tue, 15 Aug 2023, chenxiaolong wrote:
> In the implementation process, the "q" suffix function is
> Re-register and associate the "__float128" type with the
> "long double" type so that the compiler can handle the
> corresponding function correctly. The functions implemented
> include __builtin_{huge_valq infq, fabsq, copysignq, nanq,nansq}.
> On the LoongArch architecture, __builtin_{fabsq,copysignq} can
> be implemented with the instruction "bstrins.d", so that its
> optimization effect reaches the optimal value.
Why? If long double has binary128 format, you shouldn't need any of these
functions at all; if it doesn't, just the C23 _Float128 type name and f128
constant suffix, and associated built-in functions defined in
builtins.def, should suffice (and since we now have _FloatN support for
C++, C++ no longer provides a reason for adding __float128 either).
__float128 is a legacy type name and feature and shouldn't be needed on
any new architectures, which can just use the standard type name from the
start.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH v3] LoongArch:Implement 128-bit floating point functions in gcc.
On Wed, 16 Aug 2023, chenxiaolong wrote: > Thanks for the tip! Similar functions (e.g. __builtin_fabsf128 > (_Float128 a) are already supported by the compiler and can be handled > correctly, but functions that can be implemented on the LoongArch > architecture directly using the "bstrins" directive (e.g. fabsq, > copysignq, etc.) are better optimized because they generate fewer > assembly instructions. copysignq, etc.) on the LoongArch architecture > are better optimized because they generate fewer assembly instructions. Then you should make the existing built-in functions for _Float128 or long double generate the desired instructions, rather than adding a legacy and duplicative API to a new architecture. -- Joseph S. Myers jos...@codesourcery.com
Re: [WIP RFC] Add support for keyword-based attributes
On Wed, 16 Aug 2023, Richard Sandiford via Gcc-patches wrote: > Would it be OK to add support for: > > [[__extension__ ...]] > > to suppress the pedwarn about using [[]] prior to C2X? Then we can That seems like a plausible feature to add. -- Joseph S. Myers jos...@codesourcery.com
Re: [PING] Re: [PATCH v2] Re: [WIP] Have -Wpointer-sign be enabled by -Wextra, too [PR109836]
On Wed, 16 Aug 2023, Eric Gallager via Gcc-patches wrote:
> PING
>
> On Tue, Aug 8, 2023 at 8:17 PM Eric Gallager wrote:
> >
> > On Tue, May 30, 2023 at 5:42 PM Eric Gallager wrote:
> > >
> > > PR109836 is a request to have -Wpointer-sign enabled by default. There
> > > were points of disagreement raised in the bug report, so I figured
> > > that maybe as a compromise, the warning could just be enabled by
> > > -Wextra, as well (I have in fact seen some projects that enable
> > > -Wextra but not -Wall). This patch would implement my suggestion of
> > > adding it to -Wextra, but it's not ready to commit yet, as it still
> > > needs testing, documentation, and a ChangeLog entry. I'm just posting
> > > it here as an RFC; what do people think?
The documentation for -Wextra says "This enables some extra warning flags
that are not enabled by @option{-Wall}." (and this patch doesn't change
that documentation). I don't see any coherent reason for changing that to
add a single one of the -Wall warnings (but not any of the others). (I'm
*not* suggesting making -Wextra a superset of -Wall, but I don't think
this change is a sensible one.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH v3] LoongArch:Implement 128-bit floating point functions in gcc.
On Thu, 17 Aug 2023, Xi Ruoyao via Gcc-patches wrote:
> So I guess we just need
>
> builtin_define ("__builtin_fabsq=__builtin_fabsf128");
> builtin_define ("__builtin_nanq=__builtin_nanf128");
>
> etc. to map the "q" builtins to "f128" builtins if we really need the
> "q" builtins.
>
> Joseph: the problem here is many customers of LoongArch CPUs wish to
> compile their old code with minimal change. Is it acceptable to add
> these builtin_define's like rs6000-c.cc? Note "a new architecture" does
> not mean we'll only compile post-C2x-era programs onto it.
The powerpc support for __float128 started in GCC 6, predating the support
for _FloatN type names, built-in functions etc. in GCC 7 - that's why
there's such backwards compatibility support there. That name only exists
on a few architectures.
If people really want to compile code using the old __float128 names for
LoongArch I suppose you could have such #defines, but it would be better
for people to make their code use the standard names (as supported from
GCC 7 onwards, though only from GCC 13 in C++) and then put backwards
compatibility in their code for using the __float128 names if they want to
support the type with older GCC (GCC 6 or before for C; GCC 12 or before
for C++) on x86_64 / i386 / powerpc / ia64. Such backwards compatibility
in user code is more likely to be relevant for C++ than for C, given how
the C++ support was added to GCC much more recently. (Note: I haven't
checked when other compilers added support for the _Float128 name or
associated built-in functions, whether for C or for C++, which might also
affect when user code wants such compatibility.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH V4] Add warning options -W[no-]compare-distinct-pointer-types
On Thu, 17 Aug 2023, Jose E. Marchesi via Gcc-patches wrote:
> +@opindex Wcompare-distinct-pointer-types
> +@item -Wcompare-distinct-pointer-types
This @item should say @r{(C and Objective-C only)}, since the option isn't
implemented for C++. OK with that change.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] c: Add support for [[__extension__ ...]]
On Fri, 18 Aug 2023, Richard Sandiford via Gcc-patches wrote: > [[]] attributes are a recent addition to C, but as a GNU extension, > GCC allows them to be used in C11 and earlier. Normally this use > would trigger a pedwarn (for -pedantic, -Wc11-c2x-compat, etc.). > > This patch allows the pedwarn to be suppressed by starting the > attribute-list with __extension__. > > Also, :: is not a single lexing token prior to C2X, so it wasn't > possible to use scoped attributes in C11, even as a GNU extension. > The patch allows two colons to be used in place of :: when > __extension__ is used. No attempt is made to check whether the > two colons are immediately adjacent. > > gcc/ > * doc/extend.texi: Document the C [[__extension__ ...]] construct. > > gcc/c/ > * c-parser.cc (c_parser_std_attribute): Conditionally allow > two colons to be used in place of ::. > (c_parser_std_attribute_list): New function, split out from... > (c_parser_std_attribute_specifier): ...here. Allow the attribute-list > to start with __extension__. When it does, also allow two colons > to be used in place of ::. > > gcc/testsuite/ > * gcc.dg/c2x-attr-syntax-6.c: New test. > * gcc.dg/c2x-attr-syntax-7.c: Likewise. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: Darwin: Replace environment runpath with embedded [PR88590]
On Tue, 15 Aug 2023, FX Coudert via Gcc-patches wrote: > I am currently retesting the patches on various archs (Linux and Darwin) > after a final rebase, but various previous versions were > regression-tested, and have been shipped for a long time in Homebrew. > > OK to commit? The driver changes are OK. I think the new configure options and the new -nodefaultrpaths compiler option need documenting (I suppose there might be a case for the configure option defined in libtool code being documented somewhere in libtool, if there's somewhere appropriate, but I don't see that in the libtool patch submission). The help text for --enable-darwin-at-rpath refers to it as --enable-darwin-at-path. Somewhere the documentation ought to discuss the considerations around embedding such paths in binaries, and what's appropriate for building a binary on the system where it's going to be used, versus using the compiler to build redistributed binaries that will be run on a system that doesn't have the compiler installed (and so shouldn't have hardcoded paths that were only applicable on the system with the compiler, but will need to be able to find shared libraries - probably shipped with the binary - somehow). -- Joseph S. Myers jos...@codesourcery.com
Re: Darwin: Replace environment runpath with embedded [PR88590]
On Fri, 18 Aug 2023, Iain Sandoe via Gcc-patches wrote: > There is quite extensive Apple Developer documentation on delivering > packages with co-installed libraries using @rpath (that is the intended > mechanism for delivery since it allows drag-and-drop installation and > moving of built applications). > > The revised compiler has libraries already built in a suitable manner > for that distribution model. > > I would not propose that we repeated such information - but we could > refer to it? > > Generally, I’d prefer we suggested searching for such documentation, > rather than linking to it, since links can expire - does that seem > reasonable? I suppose the key thing is to note that, if building a program for distribution, you shouldn't build it to embed /path/to/compiler/install/lib, but instead should do . -- Joseph S. Myers jos...@codesourcery.com
Re: Patch ping Re: [PATCH 0/12] GCC _BitInt support [PR102989]
On Mon, 21 Aug 2023, Jakub Jelinek via Gcc-patches wrote: > Joseph, could I ask now at least for an overall design review of the > C patches (8-10,13) whether its interfaces with middle-end are ok, > so that Richi can review the middle-end parts? I am fine with the interface to the middle-end parts. I think the libgcc functions (i.e. those exported by libgcc, to which references are generated by the compiler) need documenting in libgcc.texi. Internal functions or macros in the libgcc patch need appropriate comments specifying their semantics; especially FP_TO_BITINT and FP_FROM_BITINT which have a lot of arguments and no comments saying what the semantics of the macros and their arguments are supposed to me. -- Joseph S. Myers jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Fri, 16 Jun 2023, Martin Uecker via Gcc-patches wrote:
> > Note that no expressions can start with the '.' token at present. As soon
> > as you invent a new kind of expression that can start with that token, you
> > have syntactic ambiguity.
> >
> > struct s1 { int c; char a[(struct s2 { int c; char b[.c]; }) {.c=.c}.c]; };
> >
> > Is ".c=.c" a use of the existing syntax for designated initializers, with
> > the first ".c" being a designator and the second being a use of the new
> > kind of expression, or is it an assignment expression, where both the LHS
> > and the RHS of the assignment use the new kind of expression? And do
> > those .c, when the use the new kind of expression, refer to the inner or
> > outer struct definition?
>
> I would treat this is one integrated feature. Essentially .c is
> somthing like this->c for the current struct for designated
> initializer *and* size expressions because it is semantically
> so close.In the initializer I would allow only
> the current use for designated initialization for all names of
> member of the currently initialized struct, so .c = .c would
> be invalid. It should never refer to the outer struct if there
I'm not clear on what the intended disambiguation rule here is, when "."
is seen in initializer list context - does this rule depend on whether the
following identifier is a member of the struct being initialized, so
".c=.c" would be OK above if the initialized struct didn't have a member
called c but the outer struct definition did? That seems like a rather
messy rule. And does "would allow only" apply other than in the ambiguous
context? That seems to be implied by ".c=.c" being invalid above, because
to make it invalid you need to disallow the new construct being used for
the second .c, not just make the first .c interpreted as a designator.
Again, this sort of thing needs a detailed written specification, with
multiple iterations discussed among different implementations. The above
paragraph doesn't make clear to me any of: the disambiguation rules; what
is allowed in what context; how name lookup works (consider tricky cases
such as a reference to an identifier declared *later* in the same struct,
possibly in the context of C2x tag compatibility where a previous
definition of the struct is visible); when these expressions get
evaluated; what the underlying principles are behind those choices.
Using a token (existing or new) other than '.' - one that doesn't
introduce ambiguity in any context where expressions can be used - would
help significantly, although some of the issues would still apply.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [V1][PATCH 1/3] Provide element_count attribute to flexible array member field (PR108896)
On Fri, 16 Jun 2023, Qing Zhao via Gcc-patches wrote:
> > So for
> >
> > struct foo { int c; int buf[(struct { int d; }){ .d = .c }]; };
> >
> > one knows during parsing that the .d is a designator
> > and that .c is not.
>
> Therefore, the above should be invalid based on this rule since .c is
> not a member in the current structure.
What do you mean by "current structure"? I think two different concepts
are being conflated: the structure *being initialized* (what the C
standard calls the "current object" for a brace-enclosed initializer
list), and the structure *being defined*. The former is what's relevant
for designators. The latter is what's relevant for the suggested new
syntax. And .c *is* a member of the structure being defined in this
example.
Those two structure types are always different, except for corner cases
with C2x tag compatibility (where an object of structure type might be
initialized in the middle of a redefinition of that type).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] Update array address space in c_build_qualified_type
On Wed, 21 Jun 2023, Richard Biener via Gcc-patches wrote: > > This patch sets the address space of the array type to that of the > > element type. > > > > Regression tests for avr look ok. Ok for trunk? > > The patch looks OK to me but please let a C frontend maintainer > double-check (I've CCed Joseph for this). The question would be whether there are any TYPE_QUALS uses in the C front end that behave incorrectly given TYPE_ADDR_SPACE (acting as qualifiers) being set on an array type - conceptually, before C2x, array types are unqualified, only the element types are qualified. The fact that this changed in C2x gives a shortcut to doing that analysis - you don't need to check all TYPE_QUALS uses in the front end, only a limited number of places that might handle qualifiers on arrays that already have conditionals to do things differently in C2x mode. But some sort of analysis of those places, to see how they'd react to an array type itself having TYPE_ADDR_SPACE set, would be helpful. If you're lucky, all those places only care about TYPE_QUALS on the element type and not on the array type itself. -- Joseph S. Myers jos...@codesourcery.com
