Re: [PATCH v1] Remove 'restrict' from 'nptr' in strtol(3)-like functions

[Richard Earnshaw (lists) via Gcc]

On 05/07/2024 17:11, Jonathan Wakely via Gcc wrote:
> On Fri, 5 Jul 2024 at 16:54, Alejandro Colomar via Gcc  
> wrote:
>> At least, I hope there's consensus that while current GCC doesn't warn
>> about this, ideally it should, which means it should warn for valid uses
>> of strtol(3), which means strtol(3) should be fixed, in all of ISO,
>> POSIX, and glibc.
> 
> I'm not convinced. It doesn't look like anybody else is convinced. I
> wouldn't call that consensus.

And what's more this prototype is defined by the C standard.  If you have a 
beef with that, then you should take it up with the committee; I can't see GCC 
wanting to be different on this.

R.

Re: [RFC] MAINTAINERS: require a BZ account field

[Richard Earnshaw (lists) via Gcc]

On 25/06/2024 20:08, Arsen Arsenović via Gcc wrote:
> Hi,
> 
> Richard Biener  writes:
> 
>> [snip]
>>> I was also proposing (and would like to re-air that here) enforcing
>>> that the committer field of each commit is a (valid) @gcc.gnu.org
>>> email.  This can be configured repo-locally via:
>>>
>>>   $ git config committer.email @gcc.gnu.org
>>>
>>> Git has supported this since 39ab4d0951ba64edcfae7809740715991b44fa6d
>>> (v2.22.0).
>>>
>>> This makes a permanent association of each commit to its authors
>>> Sourceware account.
>>
>> I'd welcome this - care to create a patch for
>> contrib/gcc-git-customization.sh?
> 
> Sure - I've attached a partial patch.  I didn't find the hook which runs
> on each push to check commits, so the current patch is minimal.  Is that
> also in the tree?  I could try hacking it to check commits.

This won't work for developers who don't have a gcc account.  Previously it 
didn't matter what you said here, but now it will mess up any personal tree you 
have.

+git config "committer.email" "${remote_id}@gcc.gnu.org"

R.

Re: [RFC] MAINTAINERS: require a BZ account field

[Richard Earnshaw (lists) via Gcc]

On 27/06/2024 13:29, Sam James via Gcc wrote:
> "Richard Earnshaw (lists)"  writes:
> 
>> On 24/06/2024 22:34, Sam James via Gcc wrote:
>>> Hi!
>>>
>>> This comes up in #gcc on IRC every so often, so finally
>>> writing an RFC.
>>>
>>> What?
>>> ---
>>>
>>> I propose that MAINTAINERS be modified to be of the form,
>>> adding an extra field for their GCC/sourceware account:
>>>   >> sourceware account>
>>>   Joe Bloggsjoeblo...@example.com  jblo...@gcc.gnu.org
>>>
>>> Further, that the field must not be blank (-> must have a BZ account;
>>> there were/are some without at all)!
>>>
>>> Why?
>>> ---
>>>
>>> 1) This is tied to whether or not people should use their committer email
>>> on Bugzilla or a personal email. A lot of people don't seem to use their
>>> committer email (-> no permissions) and end up not closing bugs, so
>>> pinskia (and often myself these days) end up doing it for them.
>>>
>>> 2) It's standard practice to wish to CC the committer of a bisect result
>>> - or to CC someone who you know wrote patches on a subject area. Doing
>>> this on Bugzilla is challenging when there's no map between committer
>>> <-> BZ account.
>>>
>>> Specifically, there are folks who have git committer+author as
>>> joeblo...@example.com (or maybe even coold...@example.com) where the
>>> local part of the address has *no relation* to their GCC/sw account,
>>> so finding who to CC is difficult without e.g. trawling through gcc-cvs
>>> mails or asking overseers for help.
>>>
>>> Summary
>>> ---
>>>
>>> TL;DR: The proposal is:
>>>
>>> 1) MAINTAINERS should list a field containing either the gcc.gnu.org
>>> email in full, or their gcc username (bikeshedding semi-welcome);
>>>
>>> 2) It should become a requirement that to be in MAINTAINERS, one must
>>> possess a Bugzilla account (ideally using their gcc.gnu.org email).
>>>
>>> thanks,
>>> sam
>>
>>
>> How does this work for cases where:
>> 1) Committer is pushing to a personal or other copy of the repository
>> 2) Developers who have used the 'fetch' model to pull another developer's 
>> patches from 1 above?
>>
>> Forcing these to be rewritten will break the hashes.
> 
> To be clear, my proposal doesn't touch on the actual git metadata people
> should use, although Arsen's followup one does.

Oops!  I've replied to Arsen's message directly.


>>
>> R.
> 
> thanks,
> sam

Re: [RFC] MAINTAINERS: require a BZ account field

[Richard Earnshaw (lists) via Gcc]

On 24/06/2024 23:34, Arsen Arsenović via Gcc wrote:
> Hi,
> 
> Sam James via Gcc  writes:
> 
>> Hi!
>>
>> This comes up in #gcc on IRC every so often, so finally
>> writing an RFC.
>>
>> What?
>> ---
>>
>> I propose that MAINTAINERS be modified to be of the form,
>> adding an extra field for their GCC/sourceware account:
>>   > account>
>>   Joe Bloggsjoeblo...@example.com  jblo...@gcc.gnu.org
>>
>> Further, that the field must not be blank (-> must have a BZ account;
>> there were/are some without at all)!
>>
>> Why?
>> ---
>>
>> 1) This is tied to whether or not people should use their committer email
>> on Bugzilla or a personal email. A lot of people don't seem to use their
>> committer email (-> no permissions) and end up not closing bugs, so
>> pinskia (and often myself these days) end up doing it for them.
>>
>> 2) It's standard practice to wish to CC the committer of a bisect result
>> - or to CC someone who you know wrote patches on a subject area. Doing
>> this on Bugzilla is challenging when there's no map between committer
>> <-> BZ account.
>>
>> Specifically, there are folks who have git committer+author as
>> joeblo...@example.com (or maybe even coold...@example.com) where the
>> local part of the address has *no relation* to their GCC/sw account,
>> so finding who to CC is difficult without e.g. trawling through gcc-cvs
>> mails or asking overseers for help.
> 
> I was also proposing (and would like to re-air that here) enforcing that
> the committer field of each commit is a (valid) @gcc.gnu.org email.
> This can be configured repo-locally via:
> 
>   $ git config committer.email @gcc.gnu.org
> 
> Git has supported this since 39ab4d0951ba64edcfae7809740715991b44fa6d
> (v2.22.0).
> 
> This makes a permanent association of each commit to its authors
> Sourceware account.
> 
> This should not inhibit pushes, as the committer should be a reflection
> of who /applied/ a patch, and anyone applying a patch that can also push
> has a Sourceware account.  It also should not inhibit any workflow, as
> it should be automatic.


I think this presumes a strict 'git apply' model for incorporating patches from 
mailing lists, but what about contributors who do not have a sourceware account 
and rely on another developer to fetch from their tree and push it afterwards 
(eg the linux bubble-up model)?  In that case the patch fetched will show the 
original author as the committer.

R.

> 
>> Summary
>> ---
>>
>> TL;DR: The proposal is:
>>
>> 1) MAINTAINERS should list a field containing either the gcc.gnu.org
>> email in full, or their gcc username (bikeshedding semi-welcome);
>>
>> 2) It should become a requirement that to be in MAINTAINERS, one must
>> possess a Bugzilla account (ideally using their gcc.gnu.org email).
> 

Re: [RFC] MAINTAINERS: require a BZ account field

[Richard Earnshaw (lists) via Gcc]

On 24/06/2024 22:34, Sam James via Gcc wrote:
> Hi!
> 
> This comes up in #gcc on IRC every so often, so finally
> writing an RFC.
> 
> What?
> ---
> 
> I propose that MAINTAINERS be modified to be of the form,
> adding an extra field for their GCC/sourceware account:
>account>
>   Joe Bloggsjoeblo...@example.com  jblo...@gcc.gnu.org
> 
> Further, that the field must not be blank (-> must have a BZ account;
> there were/are some without at all)!
> 
> Why?
> ---
> 
> 1) This is tied to whether or not people should use their committer email
> on Bugzilla or a personal email. A lot of people don't seem to use their
> committer email (-> no permissions) and end up not closing bugs, so
> pinskia (and often myself these days) end up doing it for them.
> 
> 2) It's standard practice to wish to CC the committer of a bisect result
> - or to CC someone who you know wrote patches on a subject area. Doing
> this on Bugzilla is challenging when there's no map between committer
> <-> BZ account.
> 
> Specifically, there are folks who have git committer+author as
> joeblo...@example.com (or maybe even coold...@example.com) where the
> local part of the address has *no relation* to their GCC/sw account,
> so finding who to CC is difficult without e.g. trawling through gcc-cvs
> mails or asking overseers for help.
> 
> Summary
> ---
> 
> TL;DR: The proposal is:
> 
> 1) MAINTAINERS should list a field containing either the gcc.gnu.org
> email in full, or their gcc username (bikeshedding semi-welcome);
> 
> 2) It should become a requirement that to be in MAINTAINERS, one must
> possess a Bugzilla account (ideally using their gcc.gnu.org email).
> 
> thanks,
> sam


How does this work for cases where:
1) Committer is pushing to a personal or other copy of the repository
2) Developers who have used the 'fetch' model to pull another developer's 
patches from 1 above?

Forcing these to be rewritten will break the hashes.

R.

Re: gcc git locked out for hours second day in a row

[Richard Earnshaw (lists) via Gcc]

On 12/06/2024 14:23, Mikael Morin via Gcc wrote:
> Le 12/06/2024 à 14:58, Jonathan Wakely a écrit :
>> On Wed, 12 Jun 2024 at 13:57, Mikael Morin via Gcc  wrote:
>>>
>>> Le 12/06/2024 à 13:48, Jakub Jelinek a écrit :
 Hi!

 Yesterday the gcc git repository was locked for 3 hours
 locked by user mikael at 2024-06-11 13:27:44.301067 (pid = 974167)
 78:06 python hooks/update.py 
 refs/users/mikael/tags/fortran-dev_merges/r10-1545 
  
 c2f9fe1d8111b9671bf0aa8362446516fd942f1d
 process until overseers killed it but today we have the same
 situation for 3 ours and counting again:
 locked by user mikael at 2024-06-12 08:35:48.137564 (pid = 2219652)
 78:06 python hooks/update.py refs/users/mikael/tags/toto 
  
 cca005166dba2cefeb51afac3ea629b3972acea3

 It is possible we have some bug in the git hook scripts, but it would
 be helpful trying to understand what exactly you're trying to commit
 and why nobody else (at least to my knowledge) has similarly stuck commits.

 The effect is that nobody can push anything else to gcc git repo
 for hours.

    Jakub

>>> Yes, sorry for the inconvenience.
>>> I tried pushing a series of tags labeling merge points between the
>>> fortran-dev branch and recent years master.
>>
>> Just pushing tags should not cause a problem, assuming all the commits
>> being tagged already exist. What exactly are you pushing?
>>
> Well, the individual commits to be merged do exist, but the merge points 
> don't and they are what I'm trying to push.
> 
> To be clear, the branch hasn't seen any update for years, and I'm trying to 
> reapply what happened on trunk since, in a step-wise manner.  With 300 merges 
> I'm summing up 6 commits of history.
> 
>>
>>> The number of merge points is a bit high (329) but I expected it to be a
>>> manageable number.  I tried again today with just the most recent merge
>>> point, but it got stuck again.  I should try with the oldest one, but
>>> I'm afraid locking the repository again.
>>>
>>> I waited for the push to finish for say one hour before killing it
>>> yesterday, and no more than 15 minutes today.  Unfortunately, killing
>>> the process doesn't seem to unlock things on the server side.
>>>
>>> It may be a misconfiguration on my side, but I have never had this
>>> problem before.
>>>
>>> Sorry again.
>>>
>>>
> 

Perhaps you could create a mirror version of the repo and do some experiments 
locally on that to identify where the bottle-neck is coming from?

R.

Re: Updated Sourceware infrastructure plans

[Richard Earnshaw (lists) via Gcc]

On 23/04/2024 09:56, Mark Wielaard wrote:
> Hi,
> 
> On Mon, Apr 22, 2024 at 11:51:00PM -0400, Jason Merrill wrote:
>> On Mon, Apr 22, 2024 at 11:24 PM Tom Tromey  wrote:
>>> Jason> Someone mentioned earlier that gerrit was previously tried
>>> Jason> unsuccessfully.
>>>
>>> We tried it and gdb and then abandoned it.  We tried to integrate it
>>> into the traditional gdb development style, having it send email to
>>> gdb-patches.  I found these somewhat hard to read and in the end we
>>> agreed not to use it.
>>>
>>> I've come around again to thinking we should probably abandon email
>>> instead.  For me the main benefit is that gerrit has patch tracking,
>>> unlike our current system, where losing patches is fairly routine.
>>
>> Indeed.  Though Patchwork is another option for patch tracking, that glibc
>> seem to be having success with.
> 
> Patchworks works if you have people that like it and keep on top of
> it. For elfutils Aaron and I are probably the only ones that use it,
> but if we just go over it once a week it keeps being managable and
> nobody else needs to care. That is also why it seems to work for
> glibc. If you can carve out an hour a week going over the submitted
> patches and delegate them then it is a really useful patch tracking
> tool. Obviously that only works if the patch flow isn't overwhelming
> to begin with...
> 
> I'll work with Sergio who setup the original gerrit instance to
> upgrade it to the latest gerrit version so people try it out. One nice
> thing he did was to automate self-service user registration. Although
> that is one of the things I don't really like about it. As Tom said it
> feels like gerrit is an all or nothing solution that has to be
> mandated to be used and requires everybody to have a centralized
> login. But if you do want that then how Sergio set it up is pretty
> nice. It is just one more thing to monitor for spam accounts...
> 
> Cheers,
> 
> Mark

I've been using patchwork with GCC since, roughly, last year's cauldron.  Its 
main weakness is a poor search function for finding relevant patches, which 
means that since most patches in the queue aren't being managed it's a bit 
hit-and-miss finding the relevant patches.

Its other problem is that it expects a particular workflow model, particularly 
not replying to an existing patch discussion with an updated patch (it expects 
patches to be reposted as an entire series with a new version number, 
Linux-style).

But there are some benefits to using it: I can integrate it with my mail client 
- it can show me the patch series in patchwork when I receive a mail directly; 
it integrates well with git with the git-pw module, so I can pull an entire 
patch series off the list into my worktree from the command line just by 
knowing the patch series number; and I can manage/track patches in bundles, 
which is great if I don't have time in any particular day to deal with the 
email volume.

Finally, it's been integrated with our CI systems (thanks Linaro!), so it can 
automatically pull reviews and run validations on them, then report the results 
back; often before I've even had time to look at the patch.

R.

Re: Updated Sourceware infrastructure plans

[Richard Earnshaw (lists) via Gcc]

On 23/04/2024 04:24, Tom Tromey wrote:
> Jason> Someone mentioned earlier that gerrit was previously tried
> Jason> unsuccessfully.
> 
> We tried it and gdb and then abandoned it.  We tried to integrate it
> into the traditional gdb development style, having it send email to
> gdb-patches.  I found these somewhat hard to read and in the end we
> agreed not to use it.
> 
> I've come around again to thinking we should probably abandon email
> instead.  For me the main benefit is that gerrit has patch tracking,
> unlike our current system, where losing patches is fairly routine.
> 
> Jason> I think this is a common pattern in GCC at least: someone has an
> Jason> idea for a workflow improvement, and gets it working, but it
> Jason> isn't widely adopted.
> 
> It essentially has to be mandated, IMO.
> 
> For GCC this seems somewhat harder since the community is larger, so
> there's more people to convince.
> 
> Tom

I've been forced to use gerrit occasionally.  I hate it.  No, I LOATHE it.  The 
UI is anything but intuitive with features hidden behind unobvious selections.  
IMO It's not a tool for a casual developer, which makes it a bad introduction 
to developing software.

R.

Re: Patches submission policy change

[Richard Earnshaw (lists) via Gcc]

On 03/04/2024 14:23, Christophe Lyon via Gcc wrote:
> On Wed, 3 Apr 2024 at 14:59, Joel Sherrill  wrote:
>>
>> Another possible issue which may be better now than in years past
>> is that the versions of autoconf/automake required often had to be
>> installed by hand. I think newlib has gotten better but before the
>> rework on its Makefile/configure, I had a special install of autotools
>> which precisely matched what it required.
>>
>> And that led to very few people being able to successfully regenerate.
>>
>> Is that avoidable?
>>
>> OTOH the set of people touching these files may be small enough that
>> pain isn't an issue.
>>
> 
> For binutils/gcc/gdb we still have to use specific versions which are
> generally not the distro's ones.

That's because at least some distros modify autoconf to their own taste/needs, 
so that it does not generate the same output as the officially released 
version.  Furthermore, they provide no mechanism to make their version revert 
back to the original behaviour.

R.

Re: Help needed with maintainer-mode

[Richard Earnshaw (lists) via Gcc]

On 06/03/2024 15:04, Andrew Carlotti via Gcc wrote:
> On Thu, Feb 29, 2024 at 06:39:54PM +0100, Christophe Lyon via Gcc wrote:
>> On Thu, 29 Feb 2024 at 12:00, Mark Wielaard  wrote:
>>>
>>> Hi Christophe,
>>>
>>> On Thu, Feb 29, 2024 at 11:22:33AM +0100, Christophe Lyon via Gcc wrote:
 I've noticed that sourceware's buildbot has a small script
 "autoregen.py" which does not use the project's build system, but
 rather calls aclocal/autoheader/automake/autoconf in an ad-hoc way.
 Should we replicate that?
>>>
>>> That python script works across gcc/binutils/gdb:
>>> https://sourceware.org/cgit/builder/tree/builder/containers/autoregen.py
>>>
>>> It is installed into a container file that has the exact autoconf and
>>> automake version needed to regenerate the autotool files:
>>> https://sourceware.org/cgit/builder/tree/builder/containers/Containerfile-autotools
>>>
>>> And it was indeed done this way because that way the files are
>>> regenerated in a reproducible way. Which wasn't the case when using 
>>> --enable-maintainer-mode (and autoreconfig also doesn't work).
>>
>> I see. So it is possibly incomplete, in the sense that it may lack
>> some of the steps that maintainer-mode would perform?
>> For instance, gas for aarch64 has some *opcodes*.c files that need
>> regenerating before committing. The regeneration step is enabled in
>> maintainer-mode, so I guess the autoregen bots on Sourceware would
>> miss a problem with these files?
>>
>> Thanks,
>>
>> Christophe
> 
> Speaking of opcodes/aarch64-{asm|dis|opc}-2.c - why are these in the source
> directory in the first place?  For a similar situation in GCC (gimple-match,
> generic-match, insn-emit, etc.) we write the generated files to the build
> directory, and generation is always enabled.  I see no reason not to do the
> same thing for aarch64-{asm|dis|opc}-2.c.
> 

GCC supports building a canadian cross, but binutils doesn't.  To put them in 
the build area would require setting up host compiler as well and I don't think 
there's currently enough appetite for doing that extra work.

But every do has its day; maybe the time has come...

R.

> Andrew
> 
>>>
>>> It is run on all commits and warns if it detects a change in the
>>> (checked in) generated files.
>>> https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen
>>> https://builder.sourceware.org/buildbot/#/builders/binutils-gdb-autoregen
>>>
>>> Cheers,
>>>
>>> Mark

Re: Help needed with maintainer-mode

[Richard Earnshaw (lists) via Gcc]

On 04/03/2024 20:04, Jonathan Wakely wrote:
> On Mon, 4 Mar 2024 at 19:27, Vladimir Mezentsev
>  wrote:
>>
>>
>>
>> On 3/4/24 09:38, Richard Earnshaw (lists) wrote:
>>> Tools like git (and svn before it) don't try to maintain time-stamps on 
>>> patches, the tool just modifies the file and the timestamp comes from the 
>>> time of the modification.  That's fine if there is nothing regenerated 
>>> within the repository (it's pure original source), but will cause problems 
>>> if there are generated files as their time stamps aren't necessarily 
>>> correct.  `gcc_update --touch` addresses that by ensuring all the generated 
>>> files are retouched when needed.
>>
>> Why do we save generated files in the source tree?
>> What will be the problem if we remove Makefile.in and configure from
>> source tree and will run `autoreconf -i -f` before building ?
> 
> Having the exact correct versions of autoconf and automake increases
> the barrier for new contributors to start work. And to regenerate
> everything, they also need autogen, mkinfo, etc.

It's worse than that.  They might need multiple versions of those tools because 
different subtrees are built with different, subtly incompatible, versions of 
those tools.

R.

Re: Help needed with maintainer-mode

[Richard Earnshaw (lists) via Gcc]

On 04/03/2024 16:42, Christophe Lyon wrote:
> On Mon, 4 Mar 2024 at 16:41, Richard Earnshaw  
> wrote:
>>
>>
>>
>> On 04/03/2024 15:36, Richard Earnshaw (lists) wrote:
>> > On 04/03/2024 14:46, Christophe Lyon via Gcc wrote:
>> >> On Mon, 4 Mar 2024 at 12:25, Jonathan Wakely  
>> >> wrote:
>> >>>
>> >>> On Mon, 4 Mar 2024 at 10:44, Christophe Lyon via Gcc  
>> >>> wrote:
>> 
>>  Hi!
>> 
>>  On Mon, 4 Mar 2024 at 10:36, Thomas Schwinge  
>>  wrote:
>> >
>> > Hi!
>> >
>> > On 2024-03-04T00:30:05+, Sam James  wrote:
>> >> Mark Wielaard  writes:
>> >>> On Fri, Mar 01, 2024 at 05:32:15PM +0100, Christophe Lyon wrote:
>>  [...], I read
>>  https://gcc.gnu.org/wiki/Regenerating_GCC_Configuration 
>>   
>>  >  >
>>  which basically says "run autoreconf in every dir where there is a
>>  configure script"
>>  but this is not exactly what autoregen.py is doing. IIRC it is based
>>  on a script from Martin Liska, do you know/remember why it follows a
>>  different process?
>> >>>
>> >>> CCing Sam and Arsen who helped refine the autoregen.py script, who
>> >>> might remember more details. We wanted a script that worked for both
>> >>> gcc and binutils-gdb. And as far as I know autoreconf simply didn't
>> >>> work in all directories. We also needed to skip some directories that
>> >>> did contain a configure script, but that were imported (gotools,
>> >>> readline, minizip).
>> >>
>> >> What we really need to do is, for a start, land tschwinge/aoliva's 
>> >> patches [0]
>> >> for AC_CONFIG_SUBDIRS.
>> >
>> > Let me allocate some time this week to get that one completed.
>> >
>> >> Right now, the current situation is janky and it's nowhere near
>> >> idiomatic autotools usage. It is not a comfortable experience
>> >> interacting with it even as someone who is familiar and happy with 
>> >> using
>> >> autotools otherwise.
>> >>
>> >> I didn't yet play with maintainer-mode myself but I also didn't see 
>> >> much
>> >> point given I knew of more fundamental problems like this.
>> >>
>> >> [0] 
>> >> https://inbox.sourceware.org/gcc-patches/oril72c4yh@lxoliva.fsfla.org/
>> >>  
>> >> 
>> >>  
>> >> > >>  
>> >> >
>> >
>> 
>>  Thanks for the background. I didn't follow that discussion at that time 
>>  :-)
>> 
>>  So... I was confused because I noticed many warnings when doing a simple
>>  find . -name configure |while read f; do echo $f;d=$(dirname $f) &&
>>  autoreconf -f $d && echo $d; done
>>  as suggested by https://gcc.gnu.org/wiki/Regenerating_GCC_Configuration 
>>   
>>  >  >
>> 
>>  Then I tried with autoregen.py, and saw the same and now just
>>  checked Sourceware's bot logs and saw the same numerous warnings at
>>  least in GCC (didn't check binutils yet). Looks like this is
>>  "expected" 
>> 
>>  I started looking at auto-regenerating these files in our CI a couple
>>  of weeks ago, after we received several "complaints" from contributors
>>  saying that our precommit CI was useless / bothering since it didn't
>>  regenerate files, leading to false alarms.
>>  But now I'm wondering how such contributors regenerate the files
>>  impacted by their patches before committing, they probably just
>>  regenerate things in their subdir of interest, not noticing the whole
>>  picture :-(
>> 
>>  As a first step, we can probably use autoregen.py too, and declare
>>  maintainer-mode broken. However, I do notice that besides the rules
>>  about regenerating configure/Makefile.in/..., maintainer-mode is also
>>  used to update some files.
>>  In gcc:
>>  fixincludes: fixincl.x
>>  libffi: doc/version.texi
>>  libgfortran: some stuff :-)
>>  libiberty: functions.texi
>> >>>
>> >>> My recently proposed patch adds the first of those to gcc_update, the
>> >>> other should be done too.
>> >>> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647027.html 
>> >>>  
>> >>> > >>> 

Re: Help needed with maintainer-mode

[Richard Earnshaw (lists) via Gcc]

On 04/03/2024 14:46, Christophe Lyon via Gcc wrote:
> On Mon, 4 Mar 2024 at 12:25, Jonathan Wakely  wrote:
>>
>> On Mon, 4 Mar 2024 at 10:44, Christophe Lyon via Gcc  wrote:
>>>
>>> Hi!
>>>
>>> On Mon, 4 Mar 2024 at 10:36, Thomas Schwinge  wrote:

 Hi!

 On 2024-03-04T00:30:05+, Sam James  wrote:
> Mark Wielaard  writes:
>> On Fri, Mar 01, 2024 at 05:32:15PM +0100, Christophe Lyon wrote:
>>> [...], I read
>>> https://gcc.gnu.org/wiki/Regenerating_GCC_Configuration
>>> which basically says "run autoreconf in every dir where there is a
>>> configure script"
>>> but this is not exactly what autoregen.py is doing. IIRC it is based
>>> on a script from Martin Liska, do you know/remember why it follows a
>>> different process?
>>
>> CCing Sam and Arsen who helped refine the autoregen.py script, who
>> might remember more details. We wanted a script that worked for both
>> gcc and binutils-gdb. And as far as I know autoreconf simply didn't
>> work in all directories. We also needed to skip some directories that
>> did contain a configure script, but that were imported (gotools,
>> readline, minizip).
>
> What we really need to do is, for a start, land tschwinge/aoliva's 
> patches [0]
> for AC_CONFIG_SUBDIRS.

 Let me allocate some time this week to get that one completed.

> Right now, the current situation is janky and it's nowhere near
> idiomatic autotools usage. It is not a comfortable experience
> interacting with it even as someone who is familiar and happy with using
> autotools otherwise.
>
> I didn't yet play with maintainer-mode myself but I also didn't see much
> point given I knew of more fundamental problems like this.
>
> [0] 
> https://inbox.sourceware.org/gcc-patches/oril72c4yh@lxoliva.fsfla.org/

>>>
>>> Thanks for the background. I didn't follow that discussion at that time :-)
>>>
>>> So... I was confused because I noticed many warnings when doing a simple
>>> find . -name configure |while read f; do echo $f;d=$(dirname $f) &&
>>> autoreconf -f $d && echo $d; done
>>> as suggested by https://gcc.gnu.org/wiki/Regenerating_GCC_Configuration
>>>
>>> Then I tried with autoregen.py, and saw the same and now just
>>> checked Sourceware's bot logs and saw the same numerous warnings at
>>> least in GCC (didn't check binutils yet). Looks like this is
>>> "expected" 
>>>
>>> I started looking at auto-regenerating these files in our CI a couple
>>> of weeks ago, after we received several "complaints" from contributors
>>> saying that our precommit CI was useless / bothering since it didn't
>>> regenerate files, leading to false alarms.
>>> But now I'm wondering how such contributors regenerate the files
>>> impacted by their patches before committing, they probably just
>>> regenerate things in their subdir of interest, not noticing the whole
>>> picture :-(
>>>
>>> As a first step, we can probably use autoregen.py too, and declare
>>> maintainer-mode broken. However, I do notice that besides the rules
>>> about regenerating configure/Makefile.in/..., maintainer-mode is also
>>> used to update some files.
>>> In gcc:
>>> fixincludes: fixincl.x
>>> libffi: doc/version.texi
>>> libgfortran: some stuff :-)
>>> libiberty: functions.texi
>>
>> My recently proposed patch adds the first of those to gcc_update, the
>> other should be done too.
>> https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647027.html
>>
> 
> This script touches files such that they appear more recent than their
> dependencies,
> so IIUC even if one uses --enable-maintainer-mode, it will have no effect.
> For auto* files, this is "fine" as we can run autoreconf or
> autoregen.py before starting configure+build, but what about other
> files?
> For instance, if we have to test a patch which implies changes to
> fixincludes/fixincl.x, how should we proceed?
> 1- git checkout (with possibly "wrong" timestamps)
> 2- apply patch-to-test
> 3- contrib/gcc_update -t
> 4- configure --enable-maintainer-mode

If you ran

git reset --hard master // restore state to 'master'
contrib/gcc_update // pull latest code

then anything coming from upstream will be touched automatically.  You really 
don't want to re-touch the files after patching unless you're sure they've all 
been patched correctly, it will break if there's anything regenerated that's 
missing.

R.

> 
> I guess --enable-maintainer-mode would be largely (if not completely)
> disabled by step 3 above?
> And we should probably swap steps 2 and 3, so that the effects of
> patch-to-test are handled by make?
> 
> Thanks,
> 
> Christophe
> 
>>
>>
>>>
>>> in binutils/bfd:
>>> gdb/sim
>>> bfd/po/SRC-POTFILES.in
>>> bfd/po/BLD-POTFILES.in
>>> bfd/bfd-in2.h
>>> bfd/libbfd.h
>>> bfd/libcoff.h
>>> binutils/po/POTFILES.in
>>> gas/po/POTFILES.in
>>> opcodes/i386*.h
>>> gdb/copying.c
>>> gdb/data-directory/*.xml
>>> gold/po/POTFILES.in

Re: Help needed with maintainer-mode

[Richard Earnshaw (lists) via Gcc]

On 29/02/2024 10:22, Christophe Lyon via Gcc wrote:
> Hi!
> 
> Sorry for cross-posting, but I'm not sure the rules/guidelines are the
> same in gcc vs binutils/gdb.
> 
> TL;DR: are there some guidelines about how to use/enable maintainer-mode?
> 
> In the context of the Linaro CI, I've been looking at enabling
> maintainer-mode at configure time in our configurations where we test
> patches before they are committed (aka "precommit CI", which relies on
> patchwork).
> 
> Indeed, auto-generated files are not part of patch submissions, and
> when a patch implies regenerating some files before building, we
> currently report wrong failures because we don't perform such updates.
> 
> I hoped improving this would be as simple as adding
> --enable-maintainer-mode when configuring, after making sure
> autoconf-2.69 and automake-1.15.1 were in the PATH (using our host's
> libtool and gettext seems OK).
> 
> However, doing so triggered several problems, which look like race
> conditions in the build system (we build at -j160):
> - random build errors in binutils / gdb with messages like "No rule to
> make target 'po/BLD-POTFILES.in". I managed to reproduce something
> similar manually once, I noticed an empty Makefile; the build logs are
> of course difficult to read, so I couldn't figure out yet what could
> cause this.
> 
> - random build failures in gcc in fixincludes. I think this is a race
> condition because fixincludes is updated concurrently both from
> /fixincludes and $buillddir/fixincludes. Probably fixable in gcc
> Makefiles.
> 
> - I've seen other errors when building gcc like
> configure.ac:25: error: possibly undefined macro: AM_ENABLE_MULTILIB
> from libquadmath. I haven't investigated this yet.
> 
> I've read binutils' README-maintainer-mode, which contains a warning
> about distclean, but we don't use this: we start our builds from a
> scratch directory.
> 
> So... I'm wondering if there are some "official" guidelines about how
> to regenerate files, and/or use maintainer-mode?  Maybe I missed a
> "magic" make target (eg 'make autoreconf-all') that should be executed
> after configure and before 'make all'?
> 
> I've noticed that sourceware's buildbot has a small script
> "autoregen.py" which does not use the project's build system, but
> rather calls aclocal/autoheader/automake/autoconf in an ad-hoc way.
> Should we replicate that?
> 
> Thanks,
> 
> Christophe

There are other potential gotchas as well, such as the manual copying of the 
generated tm.texi back into the source repo due to relicensing.  Perhaps we 
haven't encountered that one because patches generally contain that duplicated 
output.

If we want a CI to work reliably, then perhaps we should reconsider our policy 
of stripping out regenerated code.  We have a number of developer practices, 
such as replying to an existing patch with an updated version that the CI can't 
handle easily (especially if the patch is part of a series), so there may be 
space for a discussion on how to work smarter.

My calendar says we have a toolchain office hours meeting today, perhaps this 
would be worth bringing up.

R.

Re: Discussion about arm/aarch64 testcase failures seen with patch for PR111673

[Richard Earnshaw (lists) via Gcc]

On 14/12/2023 07:17, Surya Kumari Jangala via Gcc wrote:
> Hi Richard,
> Thanks a lot for your response!
> 
> Another failure reported by the Linaro CI is as follows:
> 
> Running gcc:gcc.dg/dg.exp ...
> FAIL: gcc.dg/ira-shrinkwrap-prep-1.c scan-rtl-dump pro_and_epilogue 
> "Performing shrink-wrapping"
> FAIL: gcc.dg/pr10474.c scan-rtl-dump pro_and_epilogue "Performing 
> shrink-wrapping"
> 
> I analyzed the failures and the root cause is the same for both the failures.
> 
> The test pr10474.c is as follows:
> 
> void f(int *i)
> {
> if (!i)
> return;
> else
> {
> __builtin_printf("Hi");
> *i=0;
> }
> }
> 
> 
> With the patch (for PR111673), x1 (volatile) is being assigned to hold value 
> of
> x0 (first parameter). Since it is a volatile, x1 is saved to the stack as 
> there
> is a call later on. The save to the stack is generated in the LRA pass. The 
> save
> is generated in the entry basic block. Due to the usage of the stack pointer 
> in
> the entry bb, the testcase fails to be shrink wrapped.

I'm not entirely sure I understand what you mean from a quick glance.  Do you 
mean that X1 has the /v flag marked on it (ie it's printed in dumps as 
"reg/v")?  If so, that's not volatile, it just means that the register is 
associated with a user variable (as opposed to a compiler-generated temporary 
variable):

>From the manual:

@item REG_USERVAR_P (@var{x})
In a @code{reg}, nonzero if it corresponds to a variable present in
the user's source code.  Zero for temporaries generated internally by
the compiler.  Stored in the @code{volatil} field and printed as
@samp{/v}.

There are several other cases where we re-use this bit on different RTL 
constructs to mean things other than 'volatile': it pretty much only has the 
conventional meaning on MEM objects.

> 
> The reason why LRA generates the store insn in the entry bb is as follows:
> LRA emits insns to save volatile registers in the inheritance/splitting pass.
> In this pass, LRA builds EBBs (Extended Basic Block) and traverses the insns 
> in
> the EBBs in reverse order from the last insn to the first insn. When LRA sees 
> a
> write to a pseudo (that has been assigned a volatile register), and there is a
> read following the write, with an intervening call insn between the write and 
> read,
> then LRA generates a spill immediately after the write and a restore 
> immediately
> before the read. In the above test, there is an EBB containing the entry bb 
> and
> the bb with the printf call. In the entry bb, there is a write to x1 
> (basically
> a copy from x0 to x1) and in the printf bb, there is a read of x1 after the 
> call
> insn. So LRA generates a spill in the entry bb.
> 
> Without patch, x19 is chosen to hold the value of x0. Since x19 is a 
> non-volatile,
> the input RTL to the shrink wrap pass does not have any code to save x19 to 
> the
> stack. Only the insn that copies x0 to x19 is present in the entry bb. In the
> shrink wrap pass, this insn is moved down the cfg to the bb containing the 
> call
> to printf, thereby allowing prolog to be allocated only where needed. Thus 
> shrink
> wrap succeeds.
> 
> 
> Shrink wrap can be made to succeed if the save of x1 occurs just before the 
> call
> insn, instead of generating it after the write to x1. This will ensure that 
> the
> spill does not occur in the entry bb. In fact, it is more efficient if the 
> save
> occurs only in the path containing the printf call instead of occurring in the
> entry bb.
> 
> I have a patch (bootstrapped and regtested on powerpc) that makes changes in
> LRA to save volatile registers before a call instead of after the write to the
> volatile. With this patch, both the above tests pass.
> 
> Since the patch for PR111673 has been approved by Vladimir, I plan to
> commit the patch to trunk. And I will fix the test failures after doing the
> commit.
> 

I think I'd probably understand this better if you could give some example RTL 
(before and after).  Do you have that?

R.

> Regards,
> Surya
> 
> 
> 
> On 28/11/23 7:18 pm, Richard Earnshaw wrote:
>>
>>
>> On 28/11/2023 12:52, Surya Kumari Jangala wrote:
>>> Hi Richard,
>>> Thanks a lot for your response!
>>>
>>> Another failure reported by the Linaro CI is as follows :
>>> (Note: I am planning to send a separate mail for each failure, as this will 
>>> make
>>> the discussion easy to track)
>>>
>>> FAIL: gcc.target/aarch64/sve/acle/general/cpy_1.c -march=armv8.2-a+sve 
>>> -moverride=tune=none  check-function-bodies dup_x0_m
>>>
>>> Expected code:
>>>
>>>    ...
>>>    add (x[0-9]+), x0, #?1
>>>    mov (p[0-7])\.b, p15\.b
>>>    mov z0\.d, \2/m, \1
>>>    ...
>>>    ret
>>>
>>>
>>> Code obtained w/o patch:
>>>  addvl   sp, sp, #-1
>>>  str p15, [sp]
>>>  add x0, x0, 1
>>>  mov p3.b, p15.b
>>>  mov z0.d, p3/m, x0
>>>  ldr p15, [sp]

Re: Register allocation cost question

[Richard Earnshaw (lists) via Gcc]

On 11/10/2023 09:58, Andrew Stubbs wrote:
> On 11/10/2023 07:54, Chung-Lin Tang wrote:
>>
>>
>> On 2023/10/10 11:11 PM, Andrew Stubbs wrote:
>>> Hi all,
>>>
>>> I'm trying to add a new register set to the GCN port, but I've hit a
>>> problem I don't understand.
>>>
>>> There are 256 new registers (each 2048 bit vector register) but the
>>> register file has to be divided between all the running hardware
>>> threads; if you can use fewer registers you can get more parallelism,
>>> which means that it's important that they're allocated in order.
>>>
>>> The problem is that they're not allocated in order. Somehow the IRA pass
>>> is calculating different costs for the registers within the class. It
>>> seems to prefer registers a32, a96, a160, and a224.
>>>
>>> The internal regno are 448, 512, 576, 640. These are not random numbers!
>>> They all have zero for the 6 LSB.
>>>
>>> What could cause this? Did I overrun some magic limit? What target hook
>>> might I have miscoded?
>>>
>>> I'm also seeing wrong-code bugs when I allow more than 32 new registers,
>>> but that might be an unrelated problem. Or the allocation is broken? I'm
>>> still analyzing this.
>>>
>>> If it matters, ... the new registers can't be used for general purposes,
>>> so I'm trying to set them up as a temporary spill destination. This
>>> means they're typically not busy. It feels like it shouldn't be this
>>> hard... :(
>>
>> Have you tried experimenting with REG_ALLOC_ORDER? I see that the GCN port 
>> currently isn't using this target macro.
> 
> The default definition is 0,1,2,3,4 and is already the desired behaviour.
> 
> Andrew

You may need to define HONOR_REG_ALLOC_ORDER though.

Re: Documenting common C/C++ options

[Richard Earnshaw (lists) via Gcc]

On 10/10/2023 11:46, Richard Earnshaw (lists) via Gcc wrote:
> On 10/10/2023 10:47, Florian Weimer via Gcc wrote:
>> Currently, -fsigned-char and -funsigned-char are only documented as C
>> language options, although they work for C++ as well (and Objective-C
>> and Objective-C++, I assume, but I have not tested this).  There does
>> not seem to be a place for this kind of options in the manual.
>>
>> The options -fshort-enums and -fshort-wchar are documented under
>> code-generation options, but this seems to be a bit of a stretch because
>> (at least for -fshort-wchar), these too seem to be more about front-end
>> behavior.
>>
>> What would be a good way to address this?
>>
>> Thanks,
>> Florian
>>
> 
> 
> All of these are ABI; so where ever it goes, it should be documented that 
> changing them will potentially cause issues with any pre-compiled object 
> files having different settings.
> 
> R.

And you can add -f[un]signed-bitfield to that list as well.

R.

Re: Documenting common C/C++ options

[Richard Earnshaw (lists) via Gcc]

On 10/10/2023 10:47, Florian Weimer via Gcc wrote:
> Currently, -fsigned-char and -funsigned-char are only documented as C
> language options, although they work for C++ as well (and Objective-C
> and Objective-C++, I assume, but I have not tested this).  There does
> not seem to be a place for this kind of options in the manual.
> 
> The options -fshort-enums and -fshort-wchar are documented under
> code-generation options, but this seems to be a bit of a stretch because
> (at least for -fshort-wchar), these too seem to be more about front-end
> behavior.
> 
> What would be a good way to address this?
> 
> Thanks,
> Florian
> 


All of these are ABI; so where ever it goes, it should be documented that 
changing them will potentially cause issues with any pre-compiled object files 
having different settings.

R.

Re: [PATCH 2/2] libstdc++: Add dg-require-thread-fence in several tests

[Richard Earnshaw (lists) via Gcc-patches]

On 11/09/2023 16:22, Jonathan Wakely via Gcc-patches wrote:
> On Mon, 11 Sept 2023 at 14:57, Christophe Lyon
>  wrote:
>>
>>
>>
>> On Mon, 11 Sept 2023 at 15:12, Jonathan Wakely  wrote:
>>>
>>> On Mon, 11 Sept 2023 at 13:36, Christophe Lyon
>>>  wrote:



 On Mon, 11 Sept 2023 at 12:59, Jonathan Wakely  wrote:
>
> On Sun, 10 Sept 2023 at 20:31, Christophe Lyon
>  wrote:
>>
>> Some targets like arm-eabi with newlib and default settings rely on
>> __sync_synchronize() to ensure synchronization.  Newlib does not
>> implement it by default, to make users aware they have to take special
>> care.
>>
>> This makes a few tests fail to link.
>
> Does this mean those features are unusable on the target, or just that
> users need to provide their own __sync_synchronize to use them?


 IIUC the user is expected to provide them.
 Looks like we discussed this in the past :-)
 In  https://gcc.gnu.org/legacy-ml/gcc-patches/2016-10/msg01632.html,
 see the pointer to Ramana's comment: 
 https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>>>
>>> Oh yes, thanks for the reminder!
>>>

 The default arch for arm-eabi is armv4t which is very old.
 When running the testsuite with something more recent (either as default 
 by configuring GCC --with-arch=XXX or by forcing -march/-mcpu via 
 dejagnu's target-board), the compiler generates barrier instructions and 
 there are no such errors.
>>>
>>> Ah yes, that's fine then.
>>>
 For instance, here is a log with the defaults:
 https://git.linaro.org/toolchain/ci/base-artifacts/tcwg_gnu_embed_check_gcc/master-arm_eabi.git/tree/00-sumfiles?h=linaro-local/ci/tcwg_gnu_embed_check_gcc/master-arm_eabi
 and a log when we target cortex-m0 which is still a very small cpu but has 
 barriers:
 https://git.linaro.org/toolchain/ci/base-artifacts/tcwg_gnu_embed_check_gcc/master-thumb_m0_eabi.git/tree/00-sumfiles?h=linaro-local/ci/tcwg_gnu_embed_check_gcc/master-thumb_m0_eabi

 I somehow wanted to get rid of such errors with the default 
 configuration
>>>
>>> Yep, that makes sense, and we'll still be testing them for newer
>>> arches on the target, so it's not completely disabling those parts of
>>> the testsuite.
>>>
>>> But I'm still curious why some of those tests need this change. I
>>> think the ones I noted below are probably failing for some other
>>> reasons.
>>>
>> Just looked at  23_containers/span/back_assert_neg.cc, the linker says it 
>> needs
>> arm-eabi/libstdc++-v3/src/.libs/libstdc++.a(debug.o) to resolve
>> ./back_assert_neg-back_assert_neg.o (std::__glibcxx_assert_fail(char const*, 
>> int, char const*, char const*))
>> and indeed debug.o has a reference to __sync_synchronize
> 
> Aha, that's just because I put __glibcxx_assert_fail in debug.o, but
> there are no dependencies on anything else in that file, including the
> _M_detach member function that uses atomics.
> 
> This would also be solved by -Wl,--gc-sections :-)
> 
> I think it would be better to move __glibcxx_assert_fail to a new
> file, so that it doesn't make every assertion unnecessarily depend on
> __sync_synchronize. I'll do that now.
> 
> We could also make the atomics in debug.o conditional, so that debug
> mode doesn't depend on __sync_synchronize for single-threaded targets.
> Does the arm4t arch have pthreads support in newlib?  I didn't bother
> making the use of atomics conditional, because performance is not
> really a priority for debug mode bookkeeping. But the problem here
> isn't just a slight performance overhead of atomics, it's that they
> aren't even supported for arm4t.

I might be wrong, but I don't think newlib has any support for pthreads.

R.
> 

Re: Cauldron schedule: diagnostics and security features talks

[Richard Earnshaw (lists) via Gcc]

On 08/09/2023 19:18, Siddhesh Poyarekar wrote:
> Hello,
> 
> I want to begin by apologizing because I know from first hand experience that 
> scheduling can be an immensely painful job.
> 
> The Cauldron 2023 schedule[1] looks packed and I noticed that Qing and 
> David's talks on security features and diagnostics respectively are in the 
> same time slot.  Both those sessions are likely to have pretty big overlaps 
> in audience IMO since the topics are thematically related.  Is there a way in 
> which they could be put in different time slots?
> 
> IIRC they were in different time slots before and were probably moved around 
> to cater for another conflict (hence maybe making it harder to move them 
> again) but I figured I'd rather air my request and be turned down than have 
> to make the difficult choice :)
> 
> Thanks!
> Sid
> 
> [1] https://gcc.gnu.org/wiki/cauldron2023

This has been pointed out privately as well.  I'll have one more go at juggling 
the order, but you're right, this has been the most painful part of the 
organising so far.

R.

Re: Question on aarch64 prologue code.

[Richard Earnshaw (lists) via Gcc]

On 06/09/2023 15:03, Iain Sandoe wrote:
> Hi Richard,
> 
>> On 6 Sep 2023, at 13:43, Richard Sandiford via Gcc  wrote:
>>
>> Iain Sandoe  writes:
> 
>>> On the Darwin aarch64 port, we have a number of cleanup test fails (pretty 
>>> much corresponding to the [still open] 
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39244).  However, let’s assume 
>>> that bug could be a red herring..
>>>
>>> the underlying reason is missing CFI for the set of the FP which [with 
>>> Darwin’s LLVM libunwind impl.] breaks the unwind through the function that 
>>> triggers a signal.
>>
>> Just curious, do you have more details about why that is?  If the unwinder
>> is sophisticated enough to process CFI, it seems odd that it requires the
>> CFA to be defined in terms of the frame pointer.
> 
> Let me see if I can answer that below.
> 
> 
> 
>>> <——
>>>
>>> I have currently worked around this by defining a 
>>> TARGET_FRAME_POINTER_REQUIRED which returns true unless the function is a 
>>> leaf (if that’s the correct solution, then all is fine).
>>
>> I suppose it depends on why the frame-pointer-based CFA is important
>> for Darwin.  If it's due to a more general requirement for a frame
>> pointer to be used, then yeah, that's probably the right fix.
> 
> The Darwin ABI  mandates a frame pointer (although it is omitted by clang for 
> leaf functions).
> 
>>  If it's
>> more a quirk of the unwinder. then we could probably expose whatever
>> that quirk is as a new status bit.  Target-independent code in
>> dwarf2cfi.cc would then need to be aware as well.
> 
> (I suspect) it is the interaction between the mandatory FP and the fact that 
> GCC lays out the stack differently from the other Darwin toolchains at 
> present [port Issue #19].
> 
> For the system toolchain, 30 and 29 are always placed first, right below the 
> SP (other callee saves are made below that in a specified order and always in 
> pairs - presumably, with an uneccessary spill half the time) - Actually, I 
> had a look at the weekend, but cannot find specific documentation on this 
> particular aspect of the ABI  (but, of course, the de facto ABI is what the 
> system toolchain does, regardless of presence/absence of any such doc).
> 
> However (speculation) that means that the FP is not saved where the system 
> tools expect it, maybe that is confusing the unwinder absent the fp cfa.  Of 
> course, it could also just be an unwinder bug that is never triggered by 
> clang’s codegen.
> 
> GCC’s different layout currently defeats compact unwinding on all but leaf 
> frames, so one day I want to fix it ...
> .. however making this change is quite heavy lifting and I think there are 
> higher priorities for the port (so, as long as we have working unwind and no 
> observable fallout, I am deferring that change).
> 
> Note that Darwin’s ABI also has a red zone (but we have not yet made any use 
> of this, since there is no existing aarch64 impl. and I’ve not had time to 
> get to it).  However, AFAICS that is an optimisation - we can still be 
> correct without it.
> 
>>> ———
>>>
>>> However, it does seem odd that the existing code sets up the FP, but never 
>>> produces any CFA for it.
>>>
>>> So is this a possible bug, or just that I misunderstand the relevant set of 
>>> circumstances?
>>
>> emit_frame_chain fulfills an ABI requirement that every non-leaf function
>> set up a frame-chain record.  When emit_frame_chain && !frame_pointer_needed,
>> we set up the FP for ABI purposes only.  GCC can still access everything
>> relative to the stack pointer, and it can still describe the CFI based
>> purely on the stack pointer.
> 
> Thanks that makes sense
> - I guess libunwind is never used with aarch64 linux, even in a clang/llvm 
> toolchain.
>>
>> glibc-based systems only need the CFA to be based on the frame pointer
>> if the stack pointer moves during the body of the function (usually due
>> to alloca or VLAs).
> 
> I’d have to poke more at the unwinder code and do some more debugging - it 
> seems reasonable that it could work for any unwinder that’s based on DWARF 
> (although, if we have completely missing unwind info, then the different 
> stack layout would surely defeat any fallback proceedure).
> 

This is only a guess, but it sounds to me like the issue might be that although 
we create a frame record, we don't use the frame pointer for accessing stack 
variables unless SP can't be used (eg: because the function calls alloca()).  
This tends to be more efficient because offset addressing for SP is more 
flexible.  If we wanted to switch to making FP be the canonical frame address 
register we'd need to change all the code gen to use FP in addressing as well 
(or end up with some really messy translation when emitting debug information).

R.

> thanks
> Iain
> 
>>
>> Thanks,
>> Richard
> 

Re: [PATCH] rtl: Forward declare rtx_code

[Richard Earnshaw (lists) via Gcc-patches]

On 23/08/2023 16:49, Richard Sandiford via Gcc-patches wrote:
> Richard Earnshaw via Gcc-patches  writes:
>> Now that we require C++ 11, we can safely forward declare rtx_code
>> so that we can use it in target hooks.
>>
>> gcc/ChangeLog
>>  * coretypes.h (rtx_code): Add forward declaration.
>>  * rtl.h (rtx_code): Make compatible with forward declaration.
>> ---
>>  gcc/coretypes.h | 4 
>>  gcc/rtl.h   | 2 +-
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/coretypes.h b/gcc/coretypes.h
>> index ca8837cef67..51e9ce0 100644
>> --- a/gcc/coretypes.h
>> +++ b/gcc/coretypes.h
>> @@ -100,6 +100,10 @@ struct gimple;
>>  typedef gimple *gimple_seq;
>>  struct gimple_stmt_iterator;
>>  
>> +/* Forward declare rtx_code, so that we can use it in target hooks without
>> +   needing to pull in rtl.h.  */
>> +enum rtx_code : unsigned;
>> +
>>  /* Forward decls for leaf gimple subclasses (for individual gimple codes).
>> Keep this in the same order as the corresponding codes in gimple.def.  */
>>  
>> diff --git a/gcc/rtl.h b/gcc/rtl.h
>> index e1c51156f90..0e9491b89b4 100644
>> --- a/gcc/rtl.h
>> +++ b/gcc/rtl.h
>> @@ -45,7 +45,7 @@ class predefined_function_abi;
>>  /* Register Transfer Language EXPRESSIONS CODES */
>>  
>>  #define RTX_CODEenum rtx_code
>> -enum rtx_code  {
>> +enum rtx_code : unsigned {
>>  
>>  #define DEF_RTL_EXPR(ENUM, NAME, FORMAT, CLASS)   ENUM ,
>>  #include "rtl.def"  /* rtl expressions are documented here */
> 
> Given:
> 
>   #define RTX_CODE_BITSIZE 8
> 
> there might be some value in making it uint8_t rather than unsigned.
> Preapproved if you agree.
> 
> But the patch as posted is a strict improvement over the status quo,
> so it's also OK as-is.
> 
> Thanks,
> Richard

I did think about that, but there were two reasons for not doing so:
- it presumes we would never want more than 8 bits for rtx_code (well, not 
quite, 
but it would make it more work to change this).
- it would probably lead to more zero-extension operations happening in the 
compiler

I'll put my patch in as is.

R.

Re: [PATCH] aarch64: fix format specifier

[Richard Earnshaw (lists) via Gcc-patches]

On 18/08/2023 17:37, FX Coudert via Gcc-patches wrote:

A rather trivial fix for fprintf() specifier of a HOST_WIDE_INT value.
Tested on aarch64-apple-darwin. OK to commit?

FX



OK.

R.

Re: [RFC] GCC Security policy

[Richard Earnshaw (lists) via Gcc-patches]

On 08/08/2023 20:39, Carlos O'Donell via Gcc-patches wrote:

On 8/8/23 13:46, David Edelsohn wrote:

I believe that upstream projects for components that are imported
into GCC should be responsible for their security policy, including
libgo, gofrontend, libsanitizer (other than local patches), zlib,
libtool, libphobos, libcody, libffi, eventually Rust libcore, etc.


I agree completely.

We can reference the upstream and direct people to follow upstream security
policy for these bundled components.

Any other policy risks having conflicting guidance between the projects,
which is not useful for security policy.

There might be exceptions to this rule, particularly when the downstream
wants to accept particular risks while upstream does not; but none of these
components are in that case IMO.



I agree with that, but with one caveat.  Our policy should state what we 
 do once upstream has addressed the issue.


R.

Re: [RFC] GCC Security policy

[Richard Earnshaw (lists) via Gcc-patches]

On 08/08/2023 15:40, Siddhesh Poyarekar wrote:

On 2023-08-08 10:37, Jakub Jelinek wrote:

On Tue, Aug 08, 2023 at 10:30:10AM -0400, Siddhesh Poyarekar wrote:

Do you have a suggestion for the language to address libgcc, libstdc++,
etc. and libiberty, libbacktrace, etc.?


I'll work on this a bit and share a draft.


BTW, I think we should perhaps differentiate between production ready
libraries (e.g. libgcc, libstdc++, libgomp, libatomic, libgfortran, 
libquadmath,
libssp) vs. e.g. the sanitizer libraries which are meant for debugging 
and


Agreed, that's why I need some time to sort all of the libraries gcc 
builds to categorize them into various levels of support in terms of 
safety re. untrusted input.


Thanks,
Sid


Related to this, our coding standards should really reflect what we 
consider good practice these days.  eg.  There are many library APIs 
around that were once considered acceptable that frankly we would be 
better uninventing.


R.

Re: There Might a Bug in the Compiler: When Calling Weak Defined Function

[Richard Earnshaw (lists) via Gcc]

On 07/08/2023 16:51, Şahin Duran via Gcc wrote:

Dear GCC Developers,

I think I've just discovered a bug/ undefined situation in the compiler.
When I try to call a weakly defined function, compiler successfully
generates the code of calling procedure. However, this calling procedure is
nothing but branching to address 0 which results in segmentation fault. I
am not sure if this is the case for the latest version of GCC but it is for
GCC 4.9.2 and many online compilers. I just thought that maybe including a
rule that generates compilation error when the user defines a weak function
and calls it without actually implementing it. You may find the results in
the attachments.

Kind regards,
I am looking forward to hearing from you about this.
Şahin Duran




If a function might be weak, you need to test that it is defined before 
calling it.  So this is a bug in your program.  You need to write


  if (add)
add (31, 31);

Or something like that which checks that the symbol has been defined.

R.


Attachments:

Source Code:
#include 
#include 
#include "header.h"

__attribute__((weak)) int add(int,int);

int main(int argc, char *argv[]) {
printf("%x",add);
add(31,31);
return 0;
}
terminal result : 0

Disassembly (on a 64bit AMD Machine):
0x00401530 <+0>: push   rbp
0x00401531 <+1>: movrbp,rsp
0x00401534 <+4>: subrsp,0x20
0x00401538 <+8>: movDWORD PTR [rbp+0x10],ecx
0x0040153b <+11>: movQWORD PTR [rbp+0x18],rdx
0x0040153f <+15>: call   0x402100 <__main>
0x00401544 <+20>: movrdx,QWORD PTR [rip+0x2ed5]#
0x404420 <.refptr.add>
0x0040154b <+27>: learcx,[rip+0x2aae]# 0x404000
0x00401552 <+34>: call   0x402b18 
=> 0x00401557 <+39>: movedx,0x1f
0x0040155c <+44>: movecx,0x1f
0x00401561 <+49>: call   0x0
0x00401566 <+54>: moveax,0x0
0x0040156b <+59>: addrsp,0x20
0x0040156f <+63>: poprbp
0x00401570 <+64>: ret

Re: GNU Tools Cauldron 2023

[Richard Earnshaw (lists) via Gcc]

We have now finalized the ticket price for this year's Cauldron at £75.
Sarah is now contacting those who have already registered to arrange
payment.

If you have not yet registered then there is still time.  Registration
closes at 12 Noon BST (7am EDT) on Friday 1st September, and all tickets 
must be paid for by 6pm BST (1pm EDT) on Monday 4th September.  This is 
to allow time to finalize catering requirements before the event.


We have a full schedule for the event, so registration of talks is now
closed.  If you have not yet submitted a talk but wish to do so, please
do contact the organisers: we will fit you in if we can, but priority
will go to those who have already submitted something.

Richard.

On 25/07/2023 18:01, Richard Earnshaw via Gcc wrote:
It is now just under 2 months until the GNU Tools Cauldron. Registration 
is still open, but we would really appreciate it if you could register 
as soon as possible so that we have a clear idea of the numbers.


Richard.

On 05/06/2023 14:59, Richard Earnshaw wrote:

We are pleased to invite you all to the next GNU Tools Cauldron,
taking place in Cambridge, UK, on September 22-24, 2023.

As for the previous instances, we have setup a wiki page for
details:

https://gcc.gnu.org/wiki/cauldron2023 




Like last year, we are having to charge for attendance.  We are still
working out what we will need to charge, but it will be no more than 
£250.


Attendance will remain free for community volunteers and others who do
not have a commercial backer and we will be providing a small number of
travel bursaries for students to attend.

For all details of how to register, and how to submit a proposal for a
track session, please see the wiki page.

The Cauldron is organized by a group of volunteers. We are keen to add
some more people so others can stand down. If you'd like to be part of
that organizing committee, please email the same address.

This announcement is being sent to the main mailing list of the
following groups: GCC, GDB, binutils, CGEN, DejaGnu, newlib and glibc.

Please feel free to share with other groups as appropriate.

Richard (on behalf of the GNU Tools Cauldron organizing committee).

Re: [PATCH] aarch64: Fix warnings during libgcc build

[Richard Earnshaw (lists) via Gcc-patches]

On 11/07/2023 15:54, Richard Earnshaw (lists) via Gcc-patches wrote:

On 11/07/2023 10:37, Florian Weimer via Gcc-patches wrote:

libgcc/

* config/aarch64/aarch64-unwind.h (aarch64_cie_signed_with_b_key):
Add missing const qualifier.  Cast from const unsigned char *
to const char *.  Use __builtin_strchr to avoid an implicit
function declaration.
* config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
Add missing cast.

---
diff --git a/libgcc/config/aarch64/linux-unwind.h 
b/libgcc/config/aarch64/linux-unwind.h

index 00eba866049..93da7a9537d 100644
--- a/libgcc/config/aarch64/linux-unwind.h
+++ b/libgcc/config/aarch64/linux-unwind.h
@@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context 
*context,

  }
    rt_ = context->cfa;
-  sc = _->uc.uc_mcontext;
+  sc = (struct sigcontext *) _->uc.uc_mcontext;
  /* This define duplicates the definition in aarch64.md */
  #define SP_REGNUM 31




This looks somewhat dubious.  I'm not particularly familiar with the 
kernel headers, but a quick look suggests an mcontext_t is nothing like 
a sigcontext_t.  So isn't the cast just papering over some more 
fundamental problem?


R.


Sorry, I was looking at the wrong set of headers.  It looks like these 
have to match. But in that case, I think we should have a comment about 
that here to explain the suspicious cast.


R.

Re: [PATCH] aarch64: Fix warnings during libgcc build

[Richard Earnshaw (lists) via Gcc-patches]

On 11/07/2023 10:37, Florian Weimer via Gcc-patches wrote:

libgcc/

* config/aarch64/aarch64-unwind.h (aarch64_cie_signed_with_b_key):
Add missing const qualifier.  Cast from const unsigned char *
to const char *.  Use __builtin_strchr to avoid an implicit
function declaration.
* config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
Add missing cast.

---
diff --git a/libgcc/config/aarch64/linux-unwind.h 
b/libgcc/config/aarch64/linux-unwind.h
index 00eba866049..93da7a9537d 100644
--- a/libgcc/config/aarch64/linux-unwind.h
+++ b/libgcc/config/aarch64/linux-unwind.h
@@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context *context,
  }
  
rt_ = context->cfa;

-  sc = _->uc.uc_mcontext;
+  sc = (struct sigcontext *) _->uc.uc_mcontext;
  
  /* This define duplicates the definition in aarch64.md */

  #define SP_REGNUM 31




This looks somewhat dubious.  I'm not particularly familiar with the 
kernel headers, but a quick look suggests an mcontext_t is nothing like 
a sigcontext_t.  So isn't the cast just papering over some more 
fundamental problem?


R.

Re: wishlist: support for shorter pointers

[Richard Earnshaw (lists) via Gcc]

On 03/07/2023 17:42, Rafał Pietrak via Gcc wrote:

Hi Ian,

W dniu 3.07.2023 o 17:07, Ian Lance Taylor pisze:
On Wed, Jun 28, 2023 at 11:21 PM Rafał Pietrak via Gcc 
 wrote:

[]

I was thinking about that, and it doesn't look as requiring that deep
rewrites. ABI spec, that  could accomodate the functionality could be as
little as one additional attribute to linker segments.


If I understand correctly, you are looking for something like the x32
mode that was available for a while on x86_64 processors:
https://en.wikipedia.org/wiki/X32_ABI .  That was a substantial amount
of work including changes to the compiler, assembler, linker, standard
library, and kernel.  And at least to me it's never seemed
particularly popular.


Yes.

And WiKi reporting up to 40% performance improvements in some corner 
cases is impressive and encouraging. I believe, that the reported 
average of 5-8% improvement would be significantly better within MCU 
tiny resources environment. In MCU world, such improvement could mean 
fit-nofit of a project into a particular device.


-R


I think you need to be very careful when reading benchmarketing (sic) 
numbers like this.  Firstly, this is a 32-bit vs 64-bit measurement; 
secondly, the benchmark (spec 2000) is very old now and IIRC was not 
fully optimized for 64-bit processors (it predates the 64-bit version of 
the x86 instruction set); thirdly, there are benchmarks in SPEC which 
are very sensitive to cache size and the 32-bit ABI just happened to 
allow them to fit enough data in the caches to make the numbers leap.


R.

Re: gcc tricore porting

[Richard Earnshaw (lists) via Gcc]

On 03/07/2023 15:34, Joel Sherrill wrote:

On Mon, Jul 3, 2023, 4:33 AM Claudio Eterno 
wrote:


Hi Joel, I'll give an answer ASAP on the newlib and libgloss...
I supposed your question were about the licences question on newlib,
instead you were really asking what changed on the repo libs...



It was a bit of both. If they put the right licenses on the newlib and
libgloss ports, you should be able to use them and eventually submit them.
But GCC, binutils, and gdb would be gpl and require an assignment to the
FSF. That is all I meant.


It's not quite as restricted as that.  For GCC, I suggest reading 
https://gcc.gnu.org/contribute.html#legal for more details.


I think there are similar processes in place for binutils as well.  (I'm 
not quite so sure for GDB).


R.



An option here is to reach out to the authors and ask if they are willing
to do the FSF assignment. If they are, then any GPL licensed code from them
might be a baseline.

It looks like their current products may be based on LLVM.

--joel


C.



Il giorno dom 2 lug 2023 alle ore 19:53 Claudio Eterno <
eterno.clau...@gmail.com> ha scritto:


Hi Joel, can you give me more info regarding newlib or libgloss cases?
Unfortunately I'm a newbie on th9is world...
Thank you,
Claudio

Il giorno dom 2 lug 2023 alle ore 17:38 Joel Sherrill 
ha scritto:




On Sun, Jul 2, 2023, 3:29 AM Claudio Eterno 
wrote:


Hi, Joel and Mikael
taking a look at the code it seems that the repo owner is higtech
 but we have no confirmations.
In fact, after a comparison with gcc 9.4.0 original files i see this on
a lot of ("WITH_HIGHTEC") [intl.c]:
[image: image.png]
Probably this version of gcc is a basic version of their tricore-gcc
and probably works fine but that repo doesn't show any extra info.
Seems also impossible to contact the owner (that account doesn't show
any email or other info)..
Honestly with these conditions, from gcc development point of view,
that repo has no value.



Without an assignment, you can't submit that code. That's a blocker on
using it if there isn't one.

But you can file an issue against the repo asking questions.


Anyway this is a good starting point...




Maybe not if you can't submit it. Anything that needs to be GOL licensed
and owned by the FSF is off limits.

But areas with permissive licenses might be ok if they stuck with those.
Look at what they did with newlib and libgloss.

--joel



C.



Il giorno lun 19 giu 2023 alle ore 18:55 Joel Sherrill 
ha scritto:




On Mon, Jun 19, 2023, 10:36 AM Mikael Pettersson via Gcc <
gcc@gcc.gnu.org> wrote:


(Note I'm reading the gcc mailing list via the Web archives, which
doesn't let me
create "proper" replies. Oh well.)

On Sun Jun 18 09:58:56 GMT 2023,  wrote:

Hi, this is my first time with open source development. I worked in
automotive for 22 years and we (generally) were using tricore

series for

these products. GCC doesn't compile on that platform. I left my

work some

days ago and so I'll have some spare time in the next few months. I

would

like to know how difficult it is to port the tricore platform on

gcc and if

during this process somebody can support me as tutor and... also if

the gcc

team is interested in this item...


https://github.com/volumit has a port of gcc + binutils + newlib +
gdb
to Tricore,
and it's not _that_ ancient. I have no idea where it originates from
or how complete
it is, but I do know the gcc-4.9.4 based one builds with some tweaks.




https://github.com/volumit/package_494 says there is a port in

process to gcc 9. Perhaps digging in and assessing that would be a good
start.



One question is whether that code has proper assignments on file for
ultimate inclusion. That should be part of your assessment.

--joel





I don't know anything more about it, I'm just a collector of

cross-compilers for
obscure / lost / forgotten / abandoned targets.

/Mikael





--
Claudio Eterno
via colle dell'Assietta 17
10036 Settimo Torinese (TO)





--
Claudio Eterno
via colle dell'Assietta 17
10036 Settimo Torinese (TO)




--
Claudio Eterno
via colle dell'Assietta 17
10036 Settimo Torinese (TO)

Re: wishlist: support for shorter pointers

[Richard Earnshaw (lists) via Gcc]

On 28/06/2023 17:07, Martin Uecker wrote:

Am Mittwoch, dem 28.06.2023 um 16:44 +0100 schrieb Richard Earnshaw (lists):

On 28/06/2023 15:51, Rafał Pietrak via Gcc wrote:

Hi Martin,

W dniu 28.06.2023 o 15:00, Martin Uecker pisze:


Sounds like named address spaces to me:
https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html


Only to same extend, and only in x86 case.

The goal of the wish-item I've describe is to shorten pointers. I may be
wrong and have misread the specs, but the "address spaces"
implementation you've pointed out don't look like doing that. In
particular the AVR variant applies to devices that have a "native int"
of 16-bits, and those devices (most of them) have address space no
larger. So there is no gain. Their pointers cover all their address
space and if one wanted to have shorter pointers ... like 12-bits -
those wouldn't "nicely fit into register", or 8-bits - those would
reduce the "addressable" space to 256 bytes, which is VERY tight for any
practical application.

Additionally, the AVR case is explained as "only for rodata" - this
completely dismisses it from my use.

To explain a little more: the functionality I'm looking for is something
like x86 implementation of that "address spaces". The key functionality
here is the additional register like fs/gs (an address offset register).
IMHO the feature/implementation in question would HAVE TO use additional
register instead of letting linker adjust them at link time, because
those "short" pointers would need to be load-and-stored dynamically and
changed dynamically at runtime. That's why I've put an example of ARM
instruction that does this. Again IMHO the only "syntactic" feature,that
is required for a compiler to do "the right thing" is to make compiler
consider segment (segment name, ordinary linker segment name) where a
particular pointer target resides. Then if that segment where data (of
that pointer) reside is declared "short pointers", then compiler loads
and uses additional register pointing to the base of that segment. Quite
like intel segments work in hardware.

Naturally, although I have hints on such mechanism behavior, I have no
skills to even imagine where to tweak the sources to achieve that.



I think I understand what you're asking for but:
1) You'd need a new ABI specification to handle this, probably involving
register assignments (for the 'segment' addresses), the initialization
of those at startup, assembler and linker extensions to allow for
relocations describing the symbols, etc.
2) Implementations for all of the above (it would be a lot of work -
weeks to months, not days).  Little existing code, including most of the
hand-written assembly routines is likely to be compatible with the
register conventions you'd need to define, so all that code would need
auditing and alternatives developed.
3) I doubt it would be an overall win in the end.

I base the last assertion on the fact that you'd now have three values
in many addresses, the base (segment), the pointer and then a final
offset.  This means quite a bit more code being generated, so you trade
smaller pointers in your data section for more code in your code
section.  For example,

struct f
{
    int a;
    int b;
};

int func (struct f *p)
{
    return p->b;
}

would currently compile to something like

ldr r0, [r0, #4]
bx lr

but with the new, shorter, pointer you'd end up with

add r0, r_seg, r0
ldr r0, [r0, #4]
bx lr

In some cases it might be even worse as you'd end up with
zero-extensions of the pointer values as well.



I do not quite understand why this wouldn't work with
named address spaces?

__near struct {
   int x;
   int y;
};

int func (__near struct f *p)
{
return p->b;
}

could produce exactly such code?   If you need multiple
such segments one could have __near0, ..., __near9.

Such a pointer could also be converted to a regular
pointer, which could reduce code overhead.

Martin


Named address spaces, as they exist today, don't really do anything (at 
least, in the Arm port).  A pointer is still 32-bits in size, so they 
become just syntactic sugar.


If you're going to use them as 'bases', then you still have to define 
how the base address is accessed - it doesn't just happen by magic.


R.






R.


-R



Best,
Martin

Am Dienstag, dem 27.06.2023 um 14:26 +0200 schrieb Rafał Pietrak via Gcc:

Hello everybody,

I'm not quite sure if this is correct mailbox for this suggestion (may
be "embedded" would be better), but let me present it first (and while
the examples is from ARM stm32 environment, the issue would equally
apply to i386 or even amd64). So:

1. Small MPU (like stm32f103) would normally have small amount of RAM,
and even somewhat larger variant do have its memory "partitioned/
dedicated" to various subsystems (like CloseCoupledMemory, Ethernet
buffers, USB buffs, etc).

2. to address any location within those sections of that memory (or
their entire RAM) it would 

Re: wishlist: support for shorter pointers

[Richard Earnshaw (lists) via Gcc]

On 28/06/2023 15:51, Rafał Pietrak via Gcc wrote:

Hi Martin,

W dniu 28.06.2023 o 15:00, Martin Uecker pisze:


Sounds like named address spaces to me:
https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html


Only to same extend, and only in x86 case.

The goal of the wish-item I've describe is to shorten pointers. I may be 
wrong and have misread the specs, but the "address spaces" 
implementation you've pointed out don't look like doing that. In 
particular the AVR variant applies to devices that have a "native int" 
of 16-bits, and those devices (most of them) have address space no 
larger. So there is no gain. Their pointers cover all their address 
space and if one wanted to have shorter pointers ... like 12-bits - 
those wouldn't "nicely fit into register", or 8-bits - those would 
reduce the "addressable" space to 256 bytes, which is VERY tight for any 
practical application.


Additionally, the AVR case is explained as "only for rodata" - this 
completely dismisses it from my use.


To explain a little more: the functionality I'm looking for is something 
like x86 implementation of that "address spaces". The key functionality 
here is the additional register like fs/gs (an address offset register). 
IMHO the feature/implementation in question would HAVE TO use additional 
register instead of letting linker adjust them at link time, because 
those "short" pointers would need to be load-and-stored dynamically and 
changed dynamically at runtime. That's why I've put an example of ARM 
instruction that does this. Again IMHO the only "syntactic" feature,that 
is required for a compiler to do "the right thing" is to make compiler 
consider segment (segment name, ordinary linker segment name) where a 
particular pointer target resides. Then if that segment where data (of 
that pointer) reside is declared "short pointers", then compiler loads 
and uses additional register pointing to the base of that segment. Quite 
like intel segments work in hardware.


Naturally, although I have hints on such mechanism behavior, I have no 
skills to even imagine where to tweak the sources to achieve that.



I think I understand what you're asking for but:
1) You'd need a new ABI specification to handle this, probably involving 
register assignments (for the 'segment' addresses), the initialization 
of those at startup, assembler and linker extensions to allow for 
relocations describing the symbols, etc.
2) Implementations for all of the above (it would be a lot of work - 
weeks to months, not days).  Little existing code, including most of the 
hand-written assembly routines is likely to be compatible with the 
register conventions you'd need to define, so all that code would need 
auditing and alternatives developed.

3) I doubt it would be an overall win in the end.

I base the last assertion on the fact that you'd now have three values 
in many addresses, the base (segment), the pointer and then a final 
offset.  This means quite a bit more code being generated, so you trade 
smaller pointers in your data section for more code in your code 
section.  For example,


struct f
{
  int a;
  int b;
};

int func (struct f *p)
{
  return p->b;
}

would currently compile to something like

ldr r0, [r0, #4]
bx lr

but with the new, shorter, pointer you'd end up with

add r0, r_seg, r0
ldr r0, [r0, #4]
bx lr

In some cases it might be even worse as you'd end up with 
zero-extensions of the pointer values as well.


R.


-R



Best,
Martin

Am Dienstag, dem 27.06.2023 um 14:26 +0200 schrieb Rafał Pietrak via Gcc:

Hello everybody,

I'm not quite sure if this is correct mailbox for this suggestion (may
be "embedded" would be better), but let me present it first (and while
the examples is from ARM stm32 environment, the issue would equally
apply to i386 or even amd64). So:

1. Small MPU (like stm32f103) would normally have small amount of RAM,
and even somewhat larger variant do have its memory "partitioned/
dedicated" to various subsystems (like CloseCoupledMemory, Ethernet
buffers, USB buffs, etc).

2. to address any location within those sections of that memory (or
their entire RAM) it would suffice to use 16-bit pointers.

3. still, declaring a pointer in GCC always allocate "natural" size of a
pointer in given architecture. In case of ARM stm32 it would be 32-bits.

4. programs using pointers do keep them around in structures. So
programs with heavy use of pointers have those structures like 2 times
larger then necessary  if only pointers were 16-bit. And memory in
those devices is scarce.

5. the same thing applies to 64-bit world. Programs that don't require
huge memories but do use pointers excessively, MUST take up 64-bit for a
pointer no matter what.

So I was wondering if it would be feasible for GCC to allow SEGMENT to
be declared as "small" (like 16-bit addressable in 32-bit CPU, or 32-bit
addressable in 64-bit CPU), and ANY pointer declared to reference
location 

Re: [PATCH 2/2] [testsuite, arm]: Make mve_fp_fpu[12].c accept single or double precision FPU

[Richard Earnshaw (lists) via Gcc-patches]

On 28/06/2023 10:26, Christophe Lyon via Gcc-patches wrote:

This tests currently expect a directive containing .fpu fpv5-sp-d16
and thus may fail if the test is executed for instance with
-march=armv8.1-m.main+mve.fp+fp.dp

This patch accepts either fpv5-sp-d16 or fpv5-d16 to avoid the failure.

2023-06-28  Christophe Lyon  

gcc/testsuite/
* gcc.target/arm/mve/intrinsics/mve_fp_fpu1.c: Fix .fpu
scan-assembler.
* gcc.target/arm/mve/intrinsics/mve_fp_fpu2.c: Likewise.
---
  gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu1.c | 2 +-
  gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu2.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu1.c 
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu1.c
index e375327fb97..8358a616bb5 100644
--- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu1.c
+++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu1.c
@@ -12,4 +12,4 @@ foo1 (int8x16_t value)
return b;
  }
  
-/* { dg-final { scan-assembler "\.fpu fpv5-sp-d16" }  } */

+/* { dg-final { scan-assembler "\.fpu fpv5(-sp|)-d16" }  } */
diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu2.c 
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu2.c
index 1fca1100cf0..5dd2feefc35 100644
--- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu2.c
+++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fp_fpu2.c
@@ -12,4 +12,4 @@ foo1 (int8x16_t value)
return b;
  }
  
-/* { dg-final { scan-assembler "\.fpu fpv5-sp-d16" }  } */

+/* { dg-final { scan-assembler "\.fpu fpv5(-sp|)-d16" }  } */


OK.

Re: [PATCH 1/2] [testsuite,arm]: Make nomve_fp_1.c require arm_fp

[Richard Earnshaw (lists) via Gcc-patches]

On 28/06/2023 10:26, Christophe Lyon via Gcc-patches wrote:

If GCC is configured with the default (soft) -mfloat-abi, and we don't
override the target_board test flags appropriately,
gcc.target/arm/mve/general-c/nomve_fp_1.c fails for lack of
-mfloat-abi=softfp or -mfloat-abi=hard, because it doesn't use
dg-add-options arm_v8_1m_mve (on purpose, see comment in the test).

Require and use the options needed for arm_fp to fix this problem.

2023-06-28  Christophe Lyon  

gcc/testsuite/
* gcc.target/arm/mve/general-c/nomve_fp_1.c: Require arm_fp.
---
  gcc/testsuite/gcc.target/arm/mve/general-c/nomve_fp_1.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/gcc/testsuite/gcc.target/arm/mve/general-c/nomve_fp_1.c 
b/gcc/testsuite/gcc.target/arm/mve/general-c/nomve_fp_1.c
index 21c2af16a61..c9d279ead68 100644
--- a/gcc/testsuite/gcc.target/arm/mve/general-c/nomve_fp_1.c
+++ b/gcc/testsuite/gcc.target/arm/mve/general-c/nomve_fp_1.c
@@ -1,9 +1,11 @@
  /* { dg-do compile } */
  /* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
  /* Do not use dg-add-options arm_v8_1m_mve, because this might expand to "",
 which could imply mve+fp depending on the user settings. We want to make
 sure the '+fp' extension is not enabled.  */
  /* { dg-options "-mfpu=auto -march=armv8.1-m.main+mve" } */
+/* { dg-add-options arm_fp } */
  
  #include 
  


OK.

Re: [PATCH v5] MIPS: Add speculation_barrier support

[Richard Earnshaw (lists) via Gcc-patches]

On 01/06/2023 05:26, YunQiang Su wrote:

speculation_barrier for MIPS needs sync+jr.hb (r2+),
so we implement __speculation_barrier in libgcc, like arm32 does.

gcc/ChangeLog:
* config/mips/mips-protos.h (mips_emit_speculation_barrier): New
 prototype.
* config/mips/mips.cc (speculation_barrier_libfunc): New static
 variable.
(mips_init_libfuncs): Initialize it.
(mips_emit_speculation_barrier): New function.
* config/mips/mips.md (speculation_barrier): Call
 mips_emit_speculation_barrier.

libgcc/ChangeLog:
* config/mips/lib1funcs.S: New file.
define __speculation_barrier and include mips16.S.
* config/mips/t-mips: define LIB1ASMSRC as mips/lib1funcs.S.
define LIB1ASMFUNCS as _speculation_barrier.
set version info for __speculation_barrier.
* config/mips/libgcc-mips.ver: New file.
* config/mips/t-mips16: don't define LIB1ASMSRC as mips16.S
included in lib1funcs.S now.
---


Please remember to cite PR86793 when committing this fix.

R.


  gcc/config/mips/mips-protos.h  |  2 +
  gcc/config/mips/mips.cc| 12 ++
  gcc/config/mips/mips.md| 12 ++
  libgcc/config/mips/lib1funcs.S | 65 ++
  libgcc/config/mips/libgcc-mips.ver | 21 ++
  libgcc/config/mips/t-mips  |  7 
  libgcc/config/mips/t-mips16|  3 +-
  7 files changed, 120 insertions(+), 2 deletions(-)
  create mode 100644 libgcc/config/mips/lib1funcs.S
  create mode 100644 libgcc/config/mips/libgcc-mips.ver

diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 20483469105..da7902c235b 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -388,4 +388,6 @@ extern void mips_register_frame_header_opt (void);
  extern void mips_expand_vec_cond_expr (machine_mode, machine_mode, rtx *);
  extern void mips_expand_vec_cmp_expr (rtx *);
  
+extern void mips_emit_speculation_barrier_function (void);

+
  #endif /* ! GCC_MIPS_PROTOS_H */
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index ca491b981a3..c1d1691306e 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -13611,6 +13611,9 @@ mips_autovectorize_vector_modes (vector_modes *modes, 
bool)
return 0;
  }
  
+

+static GTY (()) rtx speculation_barrier_libfunc;
+
  /* Implement TARGET_INIT_LIBFUNCS.  */
  
  static void

@@ -13680,6 +13683,7 @@ mips_init_libfuncs (void)
synchronize_libfunc = init_one_libfunc ("__sync_synchronize");
init_sync_libfuncs (UNITS_PER_WORD);
  }
+  speculation_barrier_libfunc = init_one_libfunc ("__speculation_barrier");
  }
  
  /* Build up a multi-insn sequence that loads label TARGET into $AT.  */

@@ -19092,6 +19096,14 @@ mips_avoid_hazard (rtx_insn *after, rtx_insn *insn, 
int *hilo_delay,
}
  }
  
+/* Emit a speculation barrier.

+   JR.HB is needed, so we put speculation_barrier_libfunc in libgcc.  */
+void
+mips_emit_speculation_barrier_function ()
+{
+  emit_library_call (speculation_barrier_libfunc, LCT_NORMAL, VOIDmode);
+}
+
  /* A SEQUENCE is breakable iff the branch inside it has a compact form
 and the target has compact branches.  */
  
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md

index ac1d77afc7d..5d04ac566dd 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -160,6 +160,8 @@
;; The `.insn' pseudo-op.
UNSPEC_INSN_PSEUDO
UNSPEC_JRHB
+
+  VUNSPEC_SPECULATION_BARRIER
  ])
  
  (define_constants

@@ -7455,6 +7457,16 @@
mips_expand_conditional_move (operands);
DONE;
  })
+
+(define_expand "speculation_barrier"
+  [(unspec_volatile [(const_int 0)] VUNSPEC_SPECULATION_BARRIER)]
+  ""
+  "
+  mips_emit_speculation_barrier_function ();
+  DONE;
+  "
+)
+
  
  ;;
  ;;  
diff --git a/libgcc/config/mips/lib1funcs.S b/libgcc/config/mips/lib1funcs.S
new file mode 100644
index 000..97a3655e8ab
--- /dev/null
+++ b/libgcc/config/mips/lib1funcs.S
@@ -0,0 +1,65 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+
+This file is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by the
+Free Software Foundation; either version 3, or (at your option) any
+later version.
+
+This file is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, 

Re: [PATCH][GCC][AArch64] convert some patterns to new MD syntax

[Richard Earnshaw (lists) via Gcc-patches]

On 08/06/2023 11:00, Tamar Christina via Gcc-patches wrote:

Hi All,

This converts some patterns in the AArch64 backend to use the new
compact syntax.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

gcc/ChangeLog:

* config/aarch64/aarch64.md (arches): Add nosimd.
(*mov_aarch64, *movsi_aarch64, *movdi_aarch64): Rewrite to
compact syntax.

Thanks,
Tamar


A few nits but ok apart from that:



--- inline copy of patch ---

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 
8b8951d7b14aa1a8858fdc24bf6f9dd3d927d5ea..601173338a9068f7694867c8e6e78f9b10f32a17
 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -366,7 +366,7 @@ (define_constants
  ;; As a convenience, "fp_q" means "fp" + the ability to move between
  ;; Q registers and is equivalent to "simd".
  
-(define_enum "arches" [ any rcpc8_4 fp fp_q simd sve fp16])

+(define_enum "arches" [ any rcpc8_4 fp fp_q simd nosimd sve fp16])
  
  (define_enum_attr "arch" "arches" (const_string "any"))
  
@@ -397,6 +397,9 @@ (define_attr "arch_enabled" "no,yes"

(and (eq_attr "arch" "fp_q, simd")
 (match_test "TARGET_SIMD"))
  
+	(and (eq_attr "arch" "nosimd")

+(match_test "!TARGET_SIMD"))
+
(and (eq_attr "arch" "fp16")
 (match_test "TARGET_FP_F16INST"))
  
@@ -1206,44 +1209,27 @@ (define_expand "mov"

  )
  
  (define_insn "*mov_aarch64"

-  [(set (match_operand:SHORT 0 "nonimmediate_operand" "=r,r,w,r  ,r,w, 
m,m,r,w,w")
-   (match_operand:SHORT 1 "aarch64_mov_operand"  " 
r,M,D,Usv,m,m,rZ,w,w,rZ,w"))]
+  [(set (match_operand:SHORT 0 "nonimmediate_operand")
+   (match_operand:SHORT 1 "aarch64_mov_operand"))]
"(register_operand (operands[0], mode)
  || aarch64_reg_or_zero (operands[1], mode))"
-{
-   switch (which_alternative)
- {
- case 0:
-   return "mov\t%w0, %w1";
- case 1:
-   return "mov\t%w0, %1";
- case 2:
-   return aarch64_output_scalar_simd_mov_immediate (operands[1],
-   mode);
- case 3:
-   return aarch64_output_sve_cnt_immediate (\"cnt\", \"%x0\", operands[1]);
- case 4:
-   return "ldr\t%w0, %1";
- case 5:
-   return "ldr\t%0, %1";
- case 6:
-   return "str\t%w1, %0";
- case 7:
-   return "str\t%1, %0";
- case 8:
-   return TARGET_SIMD ? "umov\t%w0, %1.[0]" : "fmov\t%w0, %s1";
- case 9:
-   return TARGET_SIMD ? "dup\t%0., %w1" : "fmov\t%s0, %w1";
- case 10:
-   return TARGET_SIMD ? "dup\t%0, %1.[0]" : "fmov\t%s0, %s1";
- default:
-   gcc_unreachable ();
- }
-}
-  ;; The "mov_imm" type for CNT is just a placeholder.
-  [(set_attr "type" "mov_reg,mov_imm,neon_move,mov_imm,load_4,load_4,store_4,
-store_4,neon_to_gp,neon_from_gp,neon_dup")
-   (set_attr "arch" "*,*,simd,sve,*,*,*,*,*,*,*")]
+  {@ [cons: =0, 1; attrs: type, arch]
+ [r , r; mov_reg, * ] mov\t%w0, %w1

  ^
This space seems redundant as all alternatives have a single letter for 
the first constraint.  Perhaps this is a hang-over from when the first 
alternative had '=r'?




+ [r , M; mov_imm, * ] mov\t%w0, %1
+ [w , D; neon_move  , simd  ] << 
aarch64_output_scalar_simd_mov_immediate (operands[1], mode);
+ /* The "mov_imm" type for CNT is just a placeholder.  */
+ [r , Usv  ; mov_imm, sve   ] << aarch64_output_sve_cnt_immediate ("cnt", 
"%x0", operands[1]);
+ [r , m; load_4 , * ] ldr\t%w0, %1
+ [w , m; load_4 , * ] ldr\t%0, %1
+ [m , rZ   ; store_4, * ] str\\t%w1, %0


I'd write "rZ" as "r Z" to make it clear that the constraints are not a 
multi-letter constraint.



+ [m , w; store_4, * ] str\t%1, %0
+ [r , w; neon_to_gp  , simd  ] umov\t%w0, %1.[0]
+ [r , w; neon_to_gp  , nosimd] fmov\t%w0, %s1 /*foo */
+ [w , rZ   ; neon_from_gp, simd  ] dup\t%0., %w1
+ [w , rZ   ; neon_from_gp, nosimd] fmov\t%s0, %w1
+ [w , w; neon_dup   , simd  ] dup\t%0, %1.[0]
+ [w , w; neon_dup   , nosimd] fmov\t%s0, %s1
+  }
  )
  
  (define_expand "mov"

@@ -1280,79 +1266,71 @@ (define_expand "mov"
  )
  
  (define_insn_and_split "*movsi_aarch64"

-  [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,r, r,w, m, m,  r,  
r,  r, w,r,w, w")
-   (match_operand:SI 1 "aarch64_mov_operand"  " 
r,r,k,M,n,Usv,m,m,rZ,w,Usw,Usa,Ush,rZ,w,w,Ds"))]
+  [(set (match_operand:SI 0 "nonimmediate_operand")
+   (match_operand:SI 1 "aarch64_mov_operand"))]
"(register_operand (operands[0], SImode)
  || aarch64_reg_or_zero (operands[1], SImode))"
-  "@
-   mov\\t%w0, %w1
-   mov\\t%w0, %w1
-   mov\\t%w0, %w1
-   mov\\t%w0, %1
-   #
-   * return aarch64_output_sve_cnt_immediate (\"cnt\", \"%x0\", operands[1]);
-   ldr\\t%w0, %1
-   ldr\\t%s0, %1
-   

Re: [PATCH v2] machine descriptor: New compact syntax for insn and insn_split in Machine Descriptions.

[Richard Earnshaw (lists) via Gcc-patches]

On 08/06/2023 11:29, Richard Earnshaw (lists) via Gcc-patches wrote:

On 08/06/2023 11:12, Andreas Schwab wrote:

On Jun 08 2023, Tamar Christina via Gcc-patches wrote:

@@ -713,6 +714,183 @@ you can use @samp{*} inside of a @samp{@@} 
multi-alternative template:

  @end group
  @end smallexample
+@node Compact Syntax
+@section Compact Syntax
+@cindex compact syntax
+
+In cases where the number of alternatives in a @code{define_insn} or
+@code{define_insn_and_split} are large then it may be beneficial to 
use the


 is large



Or perhaps better still:

When a define_insn or define_insn_and split has many alternatives it may 
be beneficial to ...


R.


Or perhaps even s/many/multiple/.  It doesn't have to have very many to 
make this new syntax preferable, IMO.


R.

Re: [PATCH v2] machine descriptor: New compact syntax for insn and insn_split in Machine Descriptions.

[Richard Earnshaw (lists) via Gcc-patches]

On 08/06/2023 11:12, Andreas Schwab wrote:

On Jun 08 2023, Tamar Christina via Gcc-patches wrote:


@@ -713,6 +714,183 @@ you can use @samp{*} inside of a @samp{@@} 
multi-alternative template:
  @end group
  @end smallexample
  
+@node Compact Syntax

+@section Compact Syntax
+@cindex compact syntax
+
+In cases where the number of alternatives in a @code{define_insn} or
+@code{define_insn_and_split} are large then it may be beneficial to use the


 is large



Or perhaps better still:

When a define_insn or define_insn_and split has many alternatives it may 
be beneficial to ...


R.

Re: [PATCH v2] machine descriptor: New compact syntax for insn and insn_split in Machine Descriptions.

[Richard Earnshaw (lists) via Gcc-patches]

On 06/06/2023 13:49, Richard Sandiford via Gcc-patches wrote:

Tamar Christina  writes:

int operand_number; /* Operand index in the big array.  */
int output_format;  /* INSN_OUTPUT_FORMAT_*.  */
+  bool compact_syntax_p;
struct operand_data operand[MAX_MAX_OPERANDS];  };

@@ -700,12 +702,57 @@ process_template (class data *d, const char

*template_code)

  if (sp != ep)
message_at (d->loc, "trailing whitespace in output template");

- while (cp < sp)
+ /* Check for any unexpanded iterators.  */
+ if (bp[0] != '*' && d->compact_syntax_p)


I assume the bp[0] != '*' condition skips the check for C code blocks.
Genuine question, but are you sure we want that?  C code often includes asm
strings (in quotes), such as for the SVE CNT[BHWD] example.

Extending the check would mean that any use of <...> for C++ templates will
need to be quoted, but explicit instantiation is pretty rare in .md files.  It 
would
also look weird for conditions.

Either way is fine, just asking.


I excluded it entirely to avoid also running afoul of the binary operators. So 
e.g.
* a < b && b > c ? foo : bar shouldn't trigger it.   It seemed more trouble 
than it's
worth to try to get correct.


Yeah.  I agree it's probably better to skip.


+  }
+
+  /* Adds a character to the end of the string.  */  void add (char
+ c)  {
+con += c;
+  }
+
+  /* Output the string in the form of a brand-new char *, then effectively
+ clear the internal string by resetting len to 0.  */  char * out
+ ()


Formatting: no need for a space before "out".


+  {
+/* Final character is always a trailing comma, so strip it out.
+ */


trailing ',', ';' or ']', rather than just a comma?


Ah no, this is a bit of a lazy intercalate, when the alternatives are pushed in 
it's
not easy to tell how many there will be (because we don't keep track of it in 
this part),
so we just always add a trailing "," and ignore the last char on output.  
Validation of the
alternative counts themselves is done later by the normal machinery.


Ah, I get it now, thanks.


+}
+
+  return index;
+}
+
+/* Modify the attributes list to make space for the implicitly declared
+   attributes in the attrs: list.  */
+
+static void
+create_missing_attributes (rtx x, file_location /* loc */,
+vec_conlist ) {
+  if (attrs.empty ())
+return;
+
+  unsigned int attr_index = GET_CODE (x) == DEFINE_INSN ? 4 : 3;
+ vec_conlist missing;
+
+  /* This is an O(n*m) loop but it's fine, both n and m will always be very
+ small.  */


Agreed that quadraticness isn't a problem.  But I wonder how many people
would write an explicit placeholder set_attr.  Unlike match_operand and
match_scratch, a placeholder set_attr doesn't carry any additional
information.

It might be simpler to drop add_attributes and add all attributes
unconditionally in this function instead.  If the user tries to specify the same
attribute using both syntaxes, the pattern would end up with two definitions
of the same attribute, which ought to be flagged by existing code.



This was done to support the (in arm backend) common thing of having attributes
which are either too complex to add inline in the new syntax or that just 
repeat a
value.

i.e. it's to allow cases like this:

   [(set_attr "length")
(set_attr "predicable" "yes")
(set_attr "predicable_short_it")
(set_attr "arch")
(set (attr "type") (if_then_else (match_operand 2 "const_int_operand" "")
  (const_string "alu_imm")
  (const_string "alu_sreg")))

Where your attrs contains:

   {@ [cons: =0, 1, 2; attrs: length, predicable_short_it, arch]


Yeah, agree it needs to be possible to define things like "type"
in this way.


You also want it for the case where every alternative takes the same 
value, eg the "predicable - yes" attr.


R.




However you're right, I could simply say that you must omit the set_attr in 
attrs and just
merge the two lists?  I think that's what you were alluding to?


Yeah, that's right.  Or just concatenate them and rely on later
error checking (which should give reasonable diagnostics).

Thanks,
Richard

Re: Wrong cost computation / conclusion ins insn combine?

[Richard Earnshaw (lists) via Gcc]

On 23/05/2023 19:41, Georg-Johann Lay wrote:

For some time now I am staring at the following test case and what
combine does with it:

typedef struct
{
     unsigned b0 : 1;
     unsigned b1 : 1;
     unsigned b2 : 1;
     unsigned b3 : 1;
     unsigned b4 : 1;
     unsigned b5 : 1;
     unsigned b6 : 1;
     unsigned b7 : 1;
} b_t;

Prior to combine, there is:

insn_cost 4 for    18: r52:QI = r24:QI
insn_cost 4 for 2: r47:QI = r52:QI
insn_cost 4 for 6: r48:QI = zero_extract(r47:QI,0x1,0)
insn_cost 4 for 7: r50:QI = 0x1
insn_cost 4 for 8: r49:QI = r48:QI ^ r50:QI
insn_cost 8 for 9: zero_extract(r47:QI,0x1,0) = r49:QI
insn_cost 4 for    15: r24:QI = r47:QI

So insn 6 extracts bit 0, insn 8 flips it, and insn 9 inserts
it as bit 0 again.

Combine then starts looking for combinations, and at some point
comes up with:


Trying 7 -> 9:
     7: r50:QI = ~r47:QI
     9: zero_extract(r47:QI,0x1,0) = r50:QI
Successfully matched this instruction:
(set (zero_extract:QI (reg/v:QI 47 [ a ])
   (const_int 1 [0x1])
   (const_int 0 [0]))
  (not:QI (reg/v:QI 47 [ a ])))
allowing combination of insns 7 and 9
original costs 4 + 8 = 12
replacement cost 12
deferring deletion of insn with uid = 7.
modifying insn i3 9: zero_extract(r47:QI,0x1,0)=~r47:QI
deferring rescan insn with uid = 9.

So the cost is 12 and this insn is accepted. But down the line,
combine cooks up this:

Trying 2, 9 -> 15:
     2: r47:QI = r52:QI
     9: zero_extract(r47:QI,0x1,0) = ~r47:QI
    15: r24:QI = r47:QI
...
Successfully matched this instruction:
(set (reg/i:QI 24 r24)
  (ior:QI (and:QI (reg:QI 52)
  (const_int -2 [0xfffe]))
  (and:QI (reg/v:QI 47 [ a ])
  (const_int 1 [0x1]
allowing combination of insns 2, 9 and 15
original costs 4 + 12 + 4 = 20
replacement costs 4 + 12 = 16
deferring deletion of insn with uid = 2.
modifying insn i2 9: r47:QI=~r52:QI
deferring rescan insn with uid = 9.
modifying insn i3    15: r24:QI=r52:QI&0xfffe|r47:QI&0x1
deferring rescan insn with uid = 15.

So this one has a cost of 16 which is more expensive than the first
combination. For example it still needs to flip the bit.

So why is combine choosing the expensive replacement over the cheap one?


Because it thinks it is cheaper.  As your log shows, the calculation is:

original costs 4 + 12 + 4 = 20
replacement costs 4 + 12 = 16

But the real problem is that two of the instructions in this example are 
simple register-register move operations which will likely be eliminated 
during register allocation anyway (due to coalescing) and this throws 
off the cost calculations.


I've seen this sort of thing before; perhaps the best solution would be 
to override the cost of a simple (register to register) set and give it 
a cost of zero.  Then we'd see that this new sequence is worse than the 
original.




Also it combines hard-registers in the 2nd case, but not in the
1st one.  So the costs get biased towards 2nd.

Can someone explain why combine takes the more expensive solution?

Target is avr, compiled with

$ avr-gcc-14 bits.c -dumpbase "" -S -Os -fdump-rtl-combine-details -dp

Johann


R.

Re: [PATCH] RFC: New compact syntax for insn and insn_split in Machine Descriptions

[Richard Earnshaw (lists) via Gcc-patches]

On 24/04/2023 09:33, Richard Sandiford via Gcc-patches wrote:

Richard Sandiford  writes:

Tamar Christina  writes:

Hi All,

This patch adds support for a compact syntax for specifying constraints in
instruction patterns. Credit for the idea goes to Richard Earnshaw.

I am sending up this RFC to get feedback for it's inclusion in GCC 14.
With this new syntax we want a clean break from the current limitations to make
something that is hopefully easier to use and maintain.

The idea behind this compact syntax is that often times it's quite hard to
correlate the entries in the constrains list, attributes and instruction lists.

One has to count and this often is tedious.  Additionally when changing a single
line in the insn multiple lines in a diff change, making it harder to see what's
going on.

This new syntax takes into account many of the common things that are done in MD
files.   It's also worth saying that this version is intended to deal with the
common case of a string based alternatives.   For C chunks we have some ideas
but those are not intended to be addressed here.

It's easiest to explain with an example:

normal syntax:

(define_insn_and_split "*movsi_aarch64"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,r, r,w, m, m,  r,  
r,  r, w,r,w, w")
(match_operand:SI 1 "aarch64_mov_operand"  " 
r,r,k,M,n,Usv,m,m,rZ,w,Usw,Usa,Ush,rZ,w,w,Ds"))]
   "(register_operand (operands[0], SImode)
 || aarch64_reg_or_zero (operands[1], SImode))"
   "@
mov\\t%w0, %w1
mov\\t%w0, %w1
mov\\t%w0, %w1
mov\\t%w0, %1
#
* return aarch64_output_sve_cnt_immediate (\"cnt\", \"%x0\", operands[1]);
ldr\\t%w0, %1
ldr\\t%s0, %1
str\\t%w1, %0
str\\t%s1, %0
adrp\\t%x0, %A1\;ldr\\t%w0, [%x0, %L1]
adr\\t%x0, %c1
adrp\\t%x0, %A1
fmov\\t%s0, %w1
fmov\\t%w0, %s1
fmov\\t%s0, %s1
* return aarch64_output_scalar_simd_mov_immediate (operands[1], SImode);"
   "CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), 
SImode)
 && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0]))"
[(const_int 0)]
"{
aarch64_expand_mov_immediate (operands[0], operands[1]);
DONE;
 }"
   ;; The "mov_imm" type for CNT is just a placeholder.
   [(set_attr "type" "mov_reg,mov_reg,mov_reg,mov_imm,mov_imm,mov_imm,load_4,

load_4,store_4,store_4,load_4,adr,adr,f_mcr,f_mrc,fmov,neon_move")
(set_attr "arch"   "*,*,*,*,*,sve,*,fp,*,fp,*,*,*,fp,fp,fp,simd")
(set_attr "length" "4,4,4,4,*,  4,4, 4,4, 4,8,4,4, 4, 4, 4,   4")
]
)

New syntax:

(define_insn_and_split "*movsi_aarch64"
   [(set (match_operand:SI 0 "nonimmediate_operand")
(match_operand:SI 1 "aarch64_mov_operand"))]
   "(register_operand (operands[0], SImode)
 || aarch64_reg_or_zero (operands[1], SImode))"
   "@@ (cons: 0 1; attrs: type arch length)
[=r, r  ; mov_reg  , *   , 4] mov\t%w0, %w1
[k , r  ; mov_reg  , *   , 4] ^
[r , k  ; mov_reg  , *   , 4] ^
[r , M  ; mov_imm  , *   , 4] mov\t%w0, %1
[r , n  ; mov_imm  , *   , *] #
[r , Usv; mov_imm  , sve , 4] << aarch64_output_sve_cnt_immediate ('cnt', 
'%x0', operands[1]);
[r , m  ; load_4   , *   , 4] ldr\t%w0, %1
[w , m  ; load_4   , fp  , 4] ldr\t%s0, %1
[m , rZ ; store_4  , *   , 4] str\t%w1, %0
[m , w  ; store_4  , fp  , 4] str\t%s1, %0
[r , Usw; load_4   , *   , 8] adrp\t%x0, %A1;ldr\t%w0, [%x0, %L1]
[r , Usa; adr  , *   , 4] adr\t%x0, %c1
[r , Ush; adr  , *   , 4] adrp\t%x0, %A1
[w , rZ ; f_mcr, fp  , 4] fmov\t%s0, %w1
[r , w  ; f_mrc, fp  , 4] fmov\t%w0, %s1
[w , w  ; fmov , fp  , 4] fmov\t%s0, %s1
[w , Ds ; neon_move, simd, 4] << aarch64_output_scalar_simd_mov_immediate 
(operands[1], SImode);"
   "CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), 
SImode)
 && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0]))"
   [(const_int 0)]
   {
 aarch64_expand_mov_immediate (operands[0], operands[1]);
 DONE;
   }
   ;; The "mov_imm" type for CNT is just a placeholder.
)

The patch contains some more rewritten examples for both Arm and AArch64.  I
have included them for examples in this RFC but the final version posted in
GCC 14 will have these split out.

The main syntax rules are as follows (See docs for full rules):
   - Template must start with "@@" to use the new syntax.
   - "@@" is followed by a layout in parentheses which is "cons:" followed by
 a list of match_operand/match_scratch IDs, then a semicolon, then the
 same for attributes ("attrs:"). Both sections are optional (so you can
 use only cons, or only attrs, or both), and cons must come before attrs
 if present.
   - Each alternative begins with any amount of whitespace.
   - Following the whitespace is a comma-separated list of constraints and/or
 attributes within brackets [], with sections separated by a semicolon.
   - Following the closing ']' is any amount of 

Re: More C type errors by default for GCC 14

[Richard Earnshaw (lists) via Gcc]

On 12/05/2023 13:30, Jakub Jelinek via Gcc wrote:

On Fri, May 12, 2023 at 11:33:01AM +0200, Martin Jambor wrote:

One fairly big GCC-internal task is to clear up the C test suite so that
it passes with the new compiler defaults.  I already have an offer of
help for that, so I think we can complete this work in a reasonable time
frame.


I'd prefer to keep at least significant portion of those tests as is with
-fpermissive added (plus of course we need new tests that verify the errors
are emitted), so that we have some testsuite coverage for those.


Whilst there is historical precedent for -fpermissive, I have to say 
that I don't like it.  The problem is that it is too imprecise as to 
what it means (and changes over time).  It also becomes the lazy way to 
paper over the problems being exposed and, in future, may mean that 
other (perhaps more important) issues that are detectable will be 
silently ignored if it becomes widely used.


R.

Re: More C type errors by default for GCC 14

[Richard Earnshaw (lists) via Gcc]

On 10/05/2023 03:38, Eli Zaretskii via Gcc wrote:

From: Arsen Arsenović 
Cc: Eli Zaretskii , Jakub Jelinek ,
  jwakely@gmail.com, gcc@gcc.gnu.org
Date: Tue, 09 May 2023 22:21:03 +0200


The concern is using the good will of the GNU Toolchain brand as the tip of
the spear or battering ram to motivate software packages to fix their
problems. It's using GCC as leverage in a manner that is difficult for
package maintainers to avoid.  Maybe that's a necessary approach, but we
should be clear about the reasoning.  Again, I'm not objecting, but let's
clarify why we are choosing this approach.


Both the GNU Toolchain and the GNU Toolchain users will benefit from a
stricter toolchain.

People can and have stopped using the GNU Toolchain due to lackluster
and non-strict defaults.  This is certainly not positive for the brand,
and I doubt it buys it much good will.


It is not GCC's business to force developers of packages to get their
act together.  It is the business of those package developers
themselves.  GCC should give those developers effective and convenient
means of detecting any unsafe and dubious code and of correcting it as
they see fit.  Which GCC already does by emitting warnings.  GCC
should only error out if it is completely unable to produce valid
code, which is not the case here, since it has been producing valid
code for ages.

It is a disservice to GCC users if a program that compiled yesterday
and worked perfectly well suddenly cannot be built because GCC was
upgraded, perhaps due to completely unrelated reasons.  It would be a
grave mistake on the part of GCC to decide that part of its mission is
to teach package developers how to write their code and when and how
to modify it.


That argument doesn't really wash.  We already upgrade the 'default' 
language version (-std=...) from time to time and that can impact 
existing programs (eg we changed from gnu-inline to std-inline model).


If this really isn't legal C, then my suggestion would be to tie this to 
a setting of -std, so -std=c2 would default to being more 
aggressive in enforcing this (via changing the warning to -werror=) and 
then -std=gnu2 might follow a bit behind that. 
Furthermore, we can trail this aggressively in release notes so that 
nobody can really claim to be surprised.


At some point that std setting will become the default and the overall 
goal is achieved.


R.

Re: [GCC][PATCH 13/15, v5] arm: Add support for dwarf debug directives and pseudo hard-register for PAC feature.

[Richard Earnshaw (lists) via Gcc-patches]

On 13/01/2023 22:12, Jakub Jelinek wrote:

On Fri, Jan 13, 2023 at 09:58:26PM +, Richard Earnshaw (lists) wrote:

> I'm afraid increasing number of DWARF registers is ABI incompatible change.
> E.g. libgcc __frame_state_for function fills in:
> typedef struct frame_state
> {
>    void *cfa;
>    void *eh_ptr;
>    long cfa_offset;
>    long args_size;
>    long reg_or_offset[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
>    unsigned short cfa_reg;
>    unsigned short retaddr_column;
>    char saved[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
> } frame_state;
> 
> structure, where PRE_GCC3_DWARF_FRAME_REGISTERS defaults to

> __LIBGCC_DWARF_FRAME_REGISTERS__, which is defined to
> DWARF_FRAME_REGISTERS, which defaults to FIRST_PSEUDO_REGISTER.
> So, changing FIRST_PSEUDO_REGISTER is an ABI change unless you arrange for
> PRE_GCC3_DWARF_FRAME_REGISTERS to be defined to the old value.
> 
>  Jakub
> 


So where's the red flag that warns about this?

I also note that Richard Sandiford made a similar type of change for AArch64
in r10-4195 (183bfdafc6f1f98711c5400498a7268cc1441096) and nothing was said
about that at the time.

It seems incredibly fragile to me to have some ABI based off the number of
machine registers.


It is.  The new unwinder fortunately doesn't suffer from this (at least I
think it doesn't), but in older gccs the unwinder could be split across 
different

objects, having e.g. parts of the unwinder in one shared library and another
part in another one, each built by different GCC version.

Guess targets which weren't supported in GCC 2.x are ok, while
__frame_state_for is in libgcc, nothing calls it, so while such changes
change the ABI, nothing likely cares.
But for older targets it is a problem.

And it is hard to catch this in the testsuite, one would either need to
hardcode the count for each target in the test, or test with mixing GCC 2.x
compiled code with current trunk.

Before the introduction of libgcc_eh.a etc., parts of the unwinder was e.g.
exported from glibc.
See e.g. 
https://gcc.gnu.org/legacy-ml/gcc-patches/2001-07/threads.html#00472 


for some details.

     Jakub



So:
1) GCC-2.* didn't support the EABI, which is all we support these days.
2) the Arm port updated FIRST_PSEUDO_REGISTER in 2019 in r10-4441 
(16155ccf588a403c033ccd7743329671bcfb27d5) and I didn't see any fallout 
from that.
3) The Arm port uses the unwinding mechanism defined by the ABI, not the 
dwarf2 based tables.


So I'm inclined to think this probably isn't going to be a problem in 
reality.


R.

Re: [GCC][PATCH 13/15, v5] arm: Add support for dwarf debug directives and pseudo hard-register for PAC feature.

[Richard Earnshaw (lists) via Gcc-patches]

On 13/01/2023 18:02, Jakub Jelinek via Gcc-patches wrote:

On Fri, Jan 13, 2023 at 05:44:15PM +, Srinath Parvathaneni via Gcc-patches 
wrote:

Hello,

This patch teaches the DWARF support in gcc about RA_AUTH_CODE pseudo 
hard-register and also
updates the ".save", ".cfi_register", ".cfi_offset", ".cfi_restore" directives 
accordingly.
This patch also adds support to emit ".pacspval" directive when "pac ip, lr, 
sp" instruction
in generated in the assembly.

RA_AUTH_CODE register number is 107 and it's dwarf register number is 143.


I'm afraid increasing number of DWARF registers is ABI incompatible change.
E.g. libgcc __frame_state_for function fills in:
typedef struct frame_state
{
   void *cfa;
   void *eh_ptr;
   long cfa_offset;
   long args_size;
   long reg_or_offset[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
   unsigned short cfa_reg;
   unsigned short retaddr_column;
   char saved[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
} frame_state;

structure, where PRE_GCC3_DWARF_FRAME_REGISTERS defaults to
__LIBGCC_DWARF_FRAME_REGISTERS__, which is defined to
DWARF_FRAME_REGISTERS, which defaults to FIRST_PSEUDO_REGISTER.
So, changing FIRST_PSEUDO_REGISTER is an ABI change unless you arrange for
PRE_GCC3_DWARF_FRAME_REGISTERS to be defined to the old value.

Jakub



So where's the red flag that warns about this?

I also note that Richard Sandiford made a similar type of change for 
AArch64 in r10-4195 (183bfdafc6f1f98711c5400498a7268cc1441096) and 
nothing was said about that at the time.


It seems incredibly fragile to me to have some ABI based off the number 
of machine registers.


R.

[COMMITTED] arm: fix bootstrap failure following automatic mode selection patch

[Richard Earnshaw (lists) via Gcc-patches]

Fix a signed vs unsigned comparison in last change.

gcc:
* common/config/arm/arm-common.c (arm_config_default): Change type
of 'i' to unsigned.
---
 gcc/common/config/arm/arm-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/common/config/arm/arm-common.c
b/gcc/common/config/arm/arm-common.c
index 5b03b86724d..9980af6885c 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -248,7 +248,7 @@ check_isa_bits_for (const enum isa_feature* bits,
enum isa_feature bit)
 static const char *
 arm_config_default (const char *name)
 {
-  int i;
+  unsigned i;
if (configure_default_options[0].name == NULL)
 return NULL;
-- 
2.30.0

Re: [PATCH] arm: Ignore --with-mode when CPU only supports one instruction set.

[Richard Earnshaw (lists) via Gcc-patches]

On 03/03/2021 14:11, Christophe Lyon via Gcc-patches wrote:
> On Wed, 3 Mar 2021 at 14:55, Richard Earnshaw (lists)
>  wrote:
>>
>> Hopefully this change will reduce the number of times Christophe is
>> needing to tweak the testsuite.
>>
> 
> Thanks!
> 
> I guess this means we can now do some cleanup in the testsuite?
> Did you have a quick look at the amount of tests involved?
> 

No, I wasn't expecting to change the existing tests again where you've
already done this.  But hopefully you won't need to do any more changes
for this reason in future.

R.

> Christophe
> 
>> --
>>
>> Arm processors can support up to two instruction sets.  Some early
>> cores only support the traditional A32 (Arm) instructions, while some
>> more recent devices only support T32 (Thumb) instructions.
>>
>> When configuring the compiler, --with-mode can be used to select the
>> default instruction set to target if the user has not made an explicit
>> choice, but this can cause needless problems if the default is not
>> supported by the requested CPU.
>>
>> To fix this this patch adjusts the way that the --with-mode selection
>> is processed so that it can take into account the selected CPU or
>> architecture and not create a meaningless combination.
>>
>> gcc:
>> * common/config/arm/arm-common.c: Include configargs.h.
>> (arm_config_default): New function.
>> (arm_target_mode): Renamed from arm_target_thumb_only.  Handle
>> processors that do not support Thumb.  Take into account the
>> --with-mode configuration setting for selecting the default.
>> * config/arm/arm.h (OPTION_DEFAULT_SPECS): Remove entry for 'mode'.
>> (TARGET_MODE_SPEC_FUNCTIONS): Update for function name change.
>> ---
>>  gcc/common/config/arm/arm-common.c | 49 ++
>>  gcc/config/arm/arm.h   | 10 +++---
>>  2 files changed, 49 insertions(+), 10 deletions(-)
>>
>>

[PATCH] arm: Ignore --with-mode when CPU only supports one instruction set.

[Richard Earnshaw (lists) via Gcc-patches]

Hopefully this change will reduce the number of times Christophe is
needing to tweak the testsuite.

--

Arm processors can support up to two instruction sets.  Some early
cores only support the traditional A32 (Arm) instructions, while some
more recent devices only support T32 (Thumb) instructions.

When configuring the compiler, --with-mode can be used to select the
default instruction set to target if the user has not made an explicit
choice, but this can cause needless problems if the default is not
supported by the requested CPU.

To fix this this patch adjusts the way that the --with-mode selection
is processed so that it can take into account the selected CPU or
architecture and not create a meaningless combination.

gcc:
* common/config/arm/arm-common.c: Include configargs.h.
(arm_config_default): New function.
(arm_target_mode): Renamed from arm_target_thumb_only.  Handle
processors that do not support Thumb.  Take into account the
--with-mode configuration setting for selecting the default.
* config/arm/arm.h (OPTION_DEFAULT_SPECS): Remove entry for 'mode'.
(TARGET_MODE_SPEC_FUNCTIONS): Update for function name change.
---
 gcc/common/config/arm/arm-common.c | 49 ++
 gcc/config/arm/arm.h   | 10 +++---
 2 files changed, 49 insertions(+), 10 deletions(-)


diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index 98824517c7b..5b03b86724d 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -33,6 +33,8 @@
 #include "sbitmap.h"
 #include "diagnostic.h"
 
+#include "configargs.h"
+
 /* Set default optimization options.  */
 static const struct default_options arm_option_optimization_table[] =
   {
@@ -240,16 +242,34 @@ check_isa_bits_for (const enum isa_feature* bits, enum isa_feature bit)
   return false;
 }
 
+/* Look up NAME in the configuration defaults for this build of the
+   the compiler.  Return the value associated with that name, or NULL
+   if no value is found.  */
+static const char *
+arm_config_default (const char *name)
+{
+  int i;
+
+  if (configure_default_options[0].name == NULL)
+return NULL;
+
+  for (i = 0; i < ARRAY_SIZE (configure_default_options); i++)
+if (strcmp (configure_default_options[i].name, name) == 0)
+  return configure_default_options[i].value;
+
+  return NULL;
+}
+
 /* Called by the driver to check whether the target denoted by current
-   command line options is a Thumb-only target.  ARGV is an array of
-   tupples (normally only one) where the first element of the tupple
-   is 'cpu' or 'arch' and the second is the option passed to the
-   compiler for that.  An architecture tupple is always taken in
-   preference to a cpu tupple and the last of each type always
+   command line options is a Thumb-only, or ARM-only, target.  ARGV is
+   an array of tupples (normally only one) where the first element of
+   the tupple is 'cpu' or 'arch' and the second is the option passed
+   to the compiler for that.  An architecture tupple is always taken
+   in preference to a cpu tupple and the last of each type always
overrides any earlier setting.  */
 
 const char *
-arm_target_thumb_only (int argc, const char **argv)
+arm_target_mode (int argc, const char **argv)
 {
   const char *arch = NULL;
   const char *cpu = NULL;
@@ -285,6 +305,9 @@ arm_target_thumb_only (int argc, const char **argv)
   if (arch_opt && !check_isa_bits_for (arch_opt->common.isa_bits,
 	   isa_bit_notm))
 	return "-mthumb";
+  if (arch_opt && !check_isa_bits_for (arch_opt->common.isa_bits,
+	   isa_bit_thumb))
+	return "-marm";
 }
   else if (cpu)
 {
@@ -294,6 +317,20 @@ arm_target_thumb_only (int argc, const char **argv)
   if (cpu_opt && !check_isa_bits_for (cpu_opt->common.isa_bits,
 	  isa_bit_notm))
 	return "-mthumb";
+  if (cpu_opt && !check_isa_bits_for (cpu_opt->common.isa_bits,
+	   isa_bit_thumb))
+	return "-marm";
+}
+
+  const char *default_mode = arm_config_default ("mode");
+  if (default_mode)
+{
+  if (strcmp (default_mode, "thumb") == 0)
+	return "-mthumb";
+  else if (strcmp (default_mode, "arm") == 0)
+	return "-marm";
+  else
+	gcc_unreachable ();
 }
 
   /* Compiler hasn't been configured with a default, and the CPU
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 6bc03ada0bf..113c015c455 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -390,7 +390,10 @@ emission of floating point pcs attributes.  */
--with-float is ignored if -mfloat-abi is specified.
--with-fpu is ignored if -mfpu is specified.
--with-abi is ignored if -mabi is specified.
-   --with-tls is ignored if -mtls-dialect is specified. */
+   --with-tls is ignored if -mtls-dialect is specified.
+   Note: --with-mode is not handled here, that has a special rule
+   TARGET_MODE_CHECK that also takes into account the 

[PATCH] arm: force use of r4 for __gnu_cmse_nonsecure_call when !FPCXT [PR99271]

[Richard Earnshaw (lists) via Gcc-patches]

Commit r10-6017 relaxed the constraint on thumb2 calls to
__gnu_cmse_nonsecure_call to allow any register for the call address.
Although the initial code expansion continues to use r4 with the FPCXT
extension is not enabled, the change was unsafe because subsequent
optimizations could use the additional freedom to change which
register was being used.

To fix this we need to split the output patterns in the machine
description to use distinct recognizers: one with the additional
freedom when FPCXT is enabled an another that retains the original
restrictions when the extension is not available.

gcc:
PR target/99271
* config/arm/thumb2.md (nonsecure_call_reg_thumb2_fpcxt): New pattern.
(nonsecure_call_value_reg_thumb2_fpcxt): Likewise.
(nonsecure_call_reg_thumb2): Restrict to using r4 for the callee
address and disable when the FPCXT is not available.
(nonsecure_call_value_reg_thumb2): Likewise.

gcc/testsuite:
* gcc.target/arm/cmse/cmse-18.c: New test.
---
 gcc/config/arm/thumb2.md| 47 ++---
 gcc/testsuite/gcc.target/arm/cmse/cmse-18.c | 11 +
 2 files changed, 42 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/cmse/cmse-18.c


diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index d7fd96c270e..5772f4d0b76 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -536,19 +536,26 @@ (define_insn "*call_reg_thumb2"
   [(set_attr "type" "call")]
 )
 
-(define_insn "*nonsecure_call_reg_thumb2"
+(define_insn "*nonsecure_call_reg_thumb2_fpcxt"
   [(call (unspec:SI [(mem:SI (match_operand:SI 0 "s_register_operand" "l*r"))]
 		UNSPEC_NONSECURE_MEM)
 	 (match_operand 1 "" ""))
(use (match_operand 2 "" ""))
(clobber (reg:SI LR_REGNUM))]
-  "TARGET_THUMB2 && use_cmse"
-  {
-if (TARGET_HAVE_FPCXT_CMSE)
-  return "blxns\\t%0";
-else
-  return "bl\\t__gnu_cmse_nonsecure_call";
-  }
+  "TARGET_THUMB2 && use_cmse && TARGET_HAVE_FPCXT_CMSE"
+  "blxns\\t%0"
+  [(set_attr "length" "4")
+   (set_attr "type" "call")]
+)
+
+(define_insn "*nonsecure_call_reg_thumb2"
+  [(call (unspec:SI [(mem:SI (reg:SI R4_REGNUM))]
+		UNSPEC_NONSECURE_MEM)
+	 (match_operand 0 "" ""))
+   (use (match_operand 1 "" ""))
+   (clobber (reg:SI LR_REGNUM))]
+  "TARGET_THUMB2 && use_cmse && !TARGET_HAVE_FPCXT_CMSE"
+  "bl\\t__gnu_cmse_nonsecure_call"
   [(set_attr "length" "4")
(set_attr "type" "call")]
 )
@@ -564,7 +571,7 @@ (define_insn "*call_value_reg_thumb2"
   [(set_attr "type" "call")]
 )
 
-(define_insn "*nonsecure_call_value_reg_thumb2"
+(define_insn "*nonsecure_call_value_reg_thumb2_fpcxt"
   [(set (match_operand 0 "" "")
 	(call
 	 (unspec:SI [(mem:SI (match_operand:SI 1 "register_operand" "l*r"))]
@@ -572,13 +579,21 @@ (define_insn "*nonsecure_call_value_reg_thumb2"
 	 (match_operand 2 "" "")))
(use (match_operand 3 "" ""))
(clobber (reg:SI LR_REGNUM))]
-  "TARGET_THUMB2 && use_cmse"
-  {
-if (TARGET_HAVE_FPCXT_CMSE)
-  return "blxns\\t%1";
-else
-  return "bl\\t__gnu_cmse_nonsecure_call";
-  }
+  "TARGET_THUMB2 && use_cmse && TARGET_HAVE_FPCXT_CMSE"
+  "blxns\\t%1"
+  [(set_attr "length" "4")
+   (set_attr "type" "call")]
+)
+
+(define_insn "*nonsecure_call_value_reg_thumb2"
+  [(set (match_operand 0 "" "")
+	(call
+	 (unspec:SI [(mem:SI (reg:SI R4_REGNUM))] UNSPEC_NONSECURE_MEM)
+	 (match_operand 1 "" "")))
+   (use (match_operand 2 "" ""))
+   (clobber (reg:SI LR_REGNUM))]
+  "TARGET_THUMB2 && use_cmse && !TARGET_HAVE_FPCXT_CMSE"
+  "bl\\t__gnu_cmse_nonsecure_call"
   [(set_attr "length" "4")
(set_attr "type" "call")]
 )
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-18.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-18.c
new file mode 100644
index 000..e1ff09257b7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-18.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-mcmse -fdump-rtl-final-slim" } */
+
+typedef void (*f)(int) __attribute__((cmse_nonsecure_call));
+
+void bar(f func, int a)
+{
+  func(a);
+}
+
+/* { dg-final { scan-rtl-dump "call unspec\\\[\\\[r4:SI\\\]\\\]" "final" } } */

Re: [PATCH v3] arm: Low overhead loop handle long range branches [PR98931]

[Richard Earnshaw (lists) via Gcc-patches]

On 10/02/2021 17:44, Andrea Corallo via Gcc-patches wrote:
> Andrea Corallo via Gcc-patches  writes:
> 
>> "Richard Earnshaw (lists)"  writes:
>>
>>> On 09/02/2021 16:27, Andrea Corallo via Gcc-patches wrote:
 Jakub Jelinek  writes:

> On Tue, Feb 09, 2021 at 03:09:43PM +0100, Jakub Jelinek via Gcc-patches 
> wrote:
>>>"TARGET_32BIT && TARGET_HAVE_LOB"
>>> -  "le\t%|lr, %l0")
>>> +  "*
>>> +  if (get_attr_length (insn) == 4)
>>> +return \"le\\t%|lr, %l0\";
>>> +  else
>>> +return \"subs\\t%|lr, #1\;bne\\t%l0\";
>>> +  "
>>
>> Why not
>> {
>>   if (get_attr_length (insn) == 4)
>> return "le\t%|lr, %l0";
>>   else
>> return "subs\t%|lr, #1;bne\t%l0";
>> }
>> instead?  Seems the arm backend uses "*..." more than the more modern {},
>> but one needs to backslash prefix a lot which makes it less readable?
>
> Where "more modern" is introduced 19.5 years ago ;)
>
>   Jakub

 I guess we really like traditions :)

 Attached second version addressing this.

 Thanks

   Andrea

>>>
>>> You're missing a clobber of the condition codes in the RTL.  This wasn't
>>> needed before, but is now.
>>>
>>> R.
>>
>> Hi Richard,
>>
>> thanks for reviewing, I guess this is going to be a good learning moment
>> for me.
>>
>> What we originally expand is:
>>
>> (insn 2396 2360 2397 3 (parallel [
>> (set (reg:CC_NZ 100 cc)
>> (compare:CC_NZ (plus:SI (reg:SI 14 lr)
>> (const_int -1 [0x]))
>> (const_int 0 [0])))
>> (set (reg:SI 14 lr)
>> (plus:SI (reg:SI 14 lr)
>> (const_int -1 [0x])))
>> ]) "p1.c":4:21 -1
>>  (nil))
>> (jump_insn 2397 2396 2365 3 (set (pc)
>> (if_then_else (ne (reg:CC_NZ 100 cc)
>> (const_int 0 [0]))
>> (label_ref:SI 2361)
>> (pc))) "p1.c":4:21 273 {arm_cond_branch}
>>  (expr_list:REG_DEAD (reg:CC_NZ 100 cc)
>> (int_list:REG_BR_PROB 1062895996 (nil)))
>>  -> 2361)
>>
>> Combine recognizing cc:CC_NZ as a dead reg and rewriting the two insns
>> as:
>>
>> (jump_insn 2397 2396 2365 3 (parallel [
>> (set (pc)
>> (if_then_else (ne (reg:SI 14 lr)
>> (const_int 1 [0x1]))
>> (label_ref:SI 2361)
>> (pc)))
>> (set (reg:SI 14 lr)
>> (plus:SI (reg:SI 14 lr)
>> (const_int -1 [0x])))
>> ]) "p1.c":4:21 1047 {*doloop_end_internal}
>>  (int_list:REG_BR_PROB 1062895996 (nil))
>>  -> 2361)
>>
>> I originally thought that because the write of reg:CC_NZ is explicit in
>> the first pattern we expand this was sufficient, but I now understand
>> I'm wrong and combine should produce a pattern still expressing this.
>> Now the question is how to do that.
>>
>> If I add the clobber '(clobber (reg:CC CC_REGNUM))' inside the parallel
>> of *doloop_end_internal as last element of the vector we ICE in
>> 'add_clobbers' called during combine, apparently 'add_clobbers' does not
>> handle the insn_code_number.
>>
>> If I add it as second element of the parallel combine is not combining
>> the two insns.
>>
>> If I place the clobber outside the parallel as a second element of the
>> insn vector combine is crashing in 'recog_for_combine_1'.
>>
>> So the question is probably: where should the clobber be positioned
>> canonically to have this working?
>>
>> Thanks!
>>
>>   Andrea
> 
> Righ, I've been explained by a knowledgeable colleague that the
> 'parallel' is implicit in the 'define_insn' and there's no need to
> express it (interestgly this is confusing the code generating
> 'add_clobbers').
> 
> The attached patch rewrites the pattern as such and adds the missing
> clobber.
> 
> Tests are running, okay for trunk when done with these?
> 
> Regards
> 
>   Andrea
> 
+  [(set (attr "length")
+(if_then_else
+(lt (minus (pc) (match_dup 0)) (const_int 1024))
+   (const_int 4)
+   (const_int 6)))
+   (set_attr "type" "branch")])

Shouldn't that be using "ltu" rather than "lt", so that if, for some
reason, the branch has been retargeted to come after the branch, then
the test will still fail and we'll get the comparison variant back.

Otherwise OK.

R.

Re: [PATCH v2] arm: Low overhead loop handle long range branches [PR98931]

[Richard Earnshaw (lists) via Gcc-patches]

On 09/02/2021 16:27, Andrea Corallo via Gcc-patches wrote:
> Jakub Jelinek  writes:
> 
>> On Tue, Feb 09, 2021 at 03:09:43PM +0100, Jakub Jelinek via Gcc-patches 
>> wrote:
"TARGET_32BIT && TARGET_HAVE_LOB"
 -  "le\t%|lr, %l0")
 +  "*
 +  if (get_attr_length (insn) == 4)
 +return \"le\\t%|lr, %l0\";
 +  else
 +return \"subs\\t%|lr, #1\;bne\\t%l0\";
 +  "
>>>
>>> Why not
>>> {
>>>   if (get_attr_length (insn) == 4)
>>> return "le\t%|lr, %l0";
>>>   else
>>> return "subs\t%|lr, #1;bne\t%l0";
>>> }
>>> instead?  Seems the arm backend uses "*..." more than the more modern {},
>>> but one needs to backslash prefix a lot which makes it less readable?
>>
>> Where "more modern" is introduced 19.5 years ago ;)
>>
>>  Jakub
> 
> I guess we really like traditions :)
> 
> Attached second version addressing this.
> 
> Thanks
> 
>   Andrea
> 

You're missing a clobber of the condition codes in the RTL.  This wasn't
needed before, but is now.

R.

Re: [PATCH] arm: [testsuite] fix lob tests for -mfloat-abi=hard

[Richard Earnshaw (lists) via Gcc-patches]

On 26/11/2020 13:53, Andrea Corallo via Gcc-patches wrote:
> Hi all,
> 
> I'd like to submit the following simple patch to clean some Low Loop
> Overhead test failing on hard float configurations.
> 
> lob2.c and lob5.c are failing with: "'-mfloat-abi=hard': selected 
> processor lacks an FPU".
> 
> lob3.c and lob5.c got "-mfloat-abi=soft and -mfloat-abi=hard may not
> be used together".
> 
> Okay for trunk?
> 
> Thanks
>   Andrea
>   
> 

I think it would be better to try to do this with suitable
require-effective-target rules (or something similar).  Forcing options
should generally be a last resort and in particular using -mfpu should
really be avoided as we're trying to move away from that.

diff --git a/gcc/testsuite/gcc.target/arm/lob4.c
b/gcc/testsuite/gcc.target/arm/lob4.c
...
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
"-marm" "-mcpu=*" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
"-marm" "-mcpu=*" "-mfloat-abi=hard" } } */
 /* { dg-options "-march=armv8.1-m.main -mthumb -O3 --save-temps
-mfloat-abi=soft" } */
 /* { dg-require-effective-target arm_softfloat } */

Why is the effective target arm_softfloat not solving this particular
conflict?

R.

[PATCH] arm: correctly handle negating INT_MIN in arm_split_atomic_op [PR97534]

[Richard Earnshaw (lists) via Gcc-patches]

arm_split_atomic_op handles subtracting a constant by converting it
into addition of the negated constant.  But if the type of the operand
is int and the constant is -1 we currently end up generating invalid
RTL which can lead to an abort later on.

The problem is that in a HOST_WIDE_INT, INT_MIN is represented as
0x8000 and the negation of this is 0x8000, but
that's not a valid constant for use in SImode operations.

The fix is straight-forward which is to use gen_int_mode rather than
simply GEN_INT.  This knows how to correctly sign-extend the negated
constant when this is needed.

gcc/
PR target/97534
* config/arm/arm.c (arm_split_atomic_op): Use gen_int_mode when
negating a const_int.
gcc/testsuite/
* gcc.dg/pr97534.c: New test.
---
 gcc/config/arm/arm.c   | 2 +-
 gcc/testsuite/gcc.dg/pr97534.c | 9 +
 2 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr97534.c

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 568e1530f24..56ed556b098 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -30824,7 +30824,7 @@ arm_split_atomic_op (enum rtx_code code, rtx old_out, 
rtx new_out, rtx mem,
 case MINUS:
   if (CONST_INT_P (value))
{
- value = GEN_INT (-INTVAL (value));
+ value = gen_int_mode (-INTVAL (value), wmode);
  code = PLUS;
}
   /* FALLTHRU */
diff --git a/gcc/testsuite/gcc.dg/pr97534.c b/gcc/testsuite/gcc.dg/pr97534.c
new file mode 100644
index 000..b363a322aa5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97534.c
@@ -0,0 +1,9 @@
+/* PR target/97534 - ICE in decompose on arm*-*-*.  */
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -O2 -g" } */
+
+int f (int a)
+{
+  int b;
+  __atomic_fetch_sub(, (int)(-__INT_MAX__ - 1), (int)0);
+}
-- 
2.17.1

Re: [AArch64] Add --with-tune configure flag

[Richard Earnshaw (lists) via Gcc-patches]

On 19/11/2020 14:40, Wilco Dijkstra via Gcc-patches wrote:
> Hi,
> 
     As for your second patch, --with-cpu-64 could be a simple alias indeed,
     but what is the exact definition/expected behaviour of a --with-cpu-32
     on a target that only supports 64-bit code? The AArch64 target cannot
     generate AArch32 code, so we shouldn't silently accept it.
>>>
>>> IMO allowing users to specify all the flags available on x86 is important.
>>>
>>
>> This isn't about general users though; it's about how you configure the
>> compiler and that's not all the same.  I don't mind the --with-cpu-64 as
>> a strict alias for --with-cpu, but --with-cpu-32 is both redundant and
>> misleading as it might give the impression that it does something useful.
> 
> We could make it do something useful, for example emit a warning, an error
> or default to -mabi=ilp32 (since that is similar to what other targets do).
> Anything is better than being the only target that doesn't support it...
> 
> Cheers,
> Wilco
> 

Having the same option have a completely different meaning would be even
worse than not having the option at all.  So no, that's a non-starter.

It's not like these configure options have wide-spread usage at present.

R.

Re: [AArch64] Add --with-tune configure flag

[Richard Earnshaw (lists) via Gcc-patches]

On 18/11/2020 17:16, Pop, Sebastian via Gcc-patches wrote:
> Hi,
> 
> On 11/18/20, 10:17 AM, "Wilco Dijkstra"  wrote:
>>I presume you're trying to unify the --with- options across most targets?
> 
> Yes, my intention was to provide the same configure options on arm64
> as on x86, such that projects that already use those options can change
> cpu name to "neoverse-n1" and that will build a compiler with the right
> tuning for Graviton2.
> 
> Allowing arm64 users to specify all the flags available on x86 is important.
> 
>>That would be very useful! However there are significant differences 
>> between
>>targets in how they interpret options like --with-arch=native (or 
>> -march). So
>>those differences also need to be looked at and fixed to avoid unexpected 
>> results.
>>
>>As for the first patch, I think support for --witch-tune requires more 
>> changes.
>>Without proper processing of a --with-tune, you get an incorrect 
>> architecture
>>version (if say the CPU you tune for is newer than the --with-cpu/arch
>>or default).
>>
>>   I posted patches to add --with-tune and fix various issues a while back:
>>https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
>>https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html
> 
> Thanks for pointing me to your patches, I was not aware of these changes.
> I see that your patches enable more use cases and fix several bugs.
> These changes would definitely be good to have in trunk and branches.
> 
> My patch was the minimal change to enable --with-tune=neoverse-n1
> 
>>As for your second patch, --with-cpu-64 could be a simple alias indeed,
>>but what is the exact definition/expected behaviour of a --with-cpu-32
>>on a target that only supports 64-bit code? The AArch64 target cannot
>>generate AArch32 code, so we shouldn't silently accept it.
> 
> IMO allowing users to specify all the flags available on x86 is important.
> 

This isn't about general users though; it's about how you configure the
compiler and that's not all the same.  I don't mind the --with-cpu-64 as
a strict alias for --with-cpu, but --with-cpu-32 is both redundant and
misleading as it might give the impression that it does something useful.

R.

> Thanks,
> Sebastian
> 

Re: [PATCH] libgcc: Add a weak stub for __sync_synchronize

[Richard Earnshaw (lists) via Gcc-patches]

On 17/11/2020 15:18, Bernd Edlinger wrote:
> On 11/17/20 1:44 PM, Richard Earnshaw (lists) wrote:
>> On 03/11/2020 15:08, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> this fixes a problem with a missing symbol __sync_synchronize
>>> which happens when newlib is used together with libstdc++ for
>>> the non-threaded simulator target arm-none-eabi.
>>>
>>> There are several questions on stackoverflow about this issue.
>>>
>>> I would like to add a weak symbol for this target, since this
>>> is only a default implementation and not meant to override a
>>> possibly more sophisticated synchronization function from the
>>> c-runtime.
>>>
>>>
>>> Regression tested successfully on arm-none-eabi with newlib-3.3.0.
>>>
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>
>> I seem to recall that this was a deliberate decision - you can't guess
>> this correctly, at least when trying to build portable code - you just
>> have to know which runtime you will be using.
>>
> 
> Therefore I suggest to use the weak attribute.  It is on purpose not
> implementing all of the atomics.
> 
> The use case, is a C++ program which initializes a local static variable.
> 
> $ cat test.cc
> #include 
> main(int argc, char **argv)
> {
>   static std::string x = "test";
>   return 0;
> }
> 
> compiles to this:
> sub sp, sp, #20
> str r0, [fp, #-24]
> str r1, [fp, #-28]
> ldr r3, .L14
> ldrbr4, [r3]
> bl  __sync_synchronize
> and r3, r4, #255
> and r3, r3, #1
> cmp r3, #0
> moveq   r3, #1
> movne   r3, #0
> and r3, r3, #255
> cmp r3, #0
> beq .L8
> ldr r0, .L14
> bl  __cxa_guard_acquire
> mov r3, r0
> 
> so __sync_synchronize is not defined in newlib since the target (arm-sim)
> is known to be not multi-threaded,
> but __cxa_guard_acquire is also not a thread safe function,
> because __GTHREADS is not defined by libgcc, since it is known
> at configure time, that the target does not support threads.
> So libstdc++ does not try to use a mutex or any atomics either,
> because it is not compiled with __GTHREADS.
> 
> I can further narrow down the patch by only defining this function when
> __GTHREADS is not defined, to make it more clear.
> 
> 
>> I think Ramana had some changes in the works at one point to address
>> (some) of this, but I'm not sure what happened to them.  Ramana?
>>
>>
>> +#if defined (__ARM_ARCH_6__) || defined (__ARM_ARCH_6J__)   \
>> +|| defined (__ARM_ARCH_6K__) || defined (__ARM_ARCH_6T2__)  \
>> +|| defined (__ARM_ARCH_6Z__) || defined (__ARM_ARCH_6ZK__)  \
>> +|| defined (__ARM_ARCH_7__) || defined (__ARM_ARCH_7A__)
>> +#if defined (__ARM_ARCH_7__) || defined (__ARM_ARCH_7A__)
>>
>> Ug, no!  Use the ACLE macros to avoid this sort of mess.
>>
> 
> Ah, thanks, copy-paste from freebsd-atomic.c :)
> 
> 
> I've attached the updated patch.
> Is it OK?
> 
> 
> Thanks
> Bernd.
> 

libgcc is *still* the wrong place for this.  It belongs in the system
library (eg newlib, or glibc, or whatever), which knows about the system
it's running on.  (Sorry, I should have said this before, but I've
context-switched this out since it's been a long time since it came up).

This hack will just lead to silent code failure of the worst kind
(non-reproducable, racy) at runtime.

R.

Re: [PATCH] libgcc: Add a weak stub for __sync_synchronize

[Richard Earnshaw (lists) via Gcc-patches]

On 03/11/2020 15:08, Bernd Edlinger wrote:
> Hi,
> 
> this fixes a problem with a missing symbol __sync_synchronize
> which happens when newlib is used together with libstdc++ for
> the non-threaded simulator target arm-none-eabi.
> 
> There are several questions on stackoverflow about this issue.
> 
> I would like to add a weak symbol for this target, since this
> is only a default implementation and not meant to override a
> possibly more sophisticated synchronization function from the
> c-runtime.
> 
> 
> Regression tested successfully on arm-none-eabi with newlib-3.3.0.
> 
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

I seem to recall that this was a deliberate decision - you can't guess
this correctly, at least when trying to build portable code - you just
have to know which runtime you will be using.

I think Ramana had some changes in the works at one point to address
(some) of this, but I'm not sure what happened to them.  Ramana?


+#if defined (__ARM_ARCH_6__) || defined (__ARM_ARCH_6J__)   \
+|| defined (__ARM_ARCH_6K__) || defined (__ARM_ARCH_6T2__)  \
+|| defined (__ARM_ARCH_6Z__) || defined (__ARM_ARCH_6ZK__)  \
+|| defined (__ARM_ARCH_7__) || defined (__ARM_ARCH_7A__)
+#if defined (__ARM_ARCH_7__) || defined (__ARM_ARCH_7A__)

Ug, no!  Use the ACLE macros to avoid this sort of mess.

R.