Re: arm-elf multilib issues
>>>>> "Paul" == Paul Brook writes: Paul> IMHO building lots of multilibs by default significantly increases Paul> toolchain size and build time for little actual benefit. ARM CPUs Paul> are generally backwards compatible and we only have one important Paul> ABI variant, so very few multilibs are required for a functional Paul> toolchain. I have had problem in the past with Cortex M3 based processors because libraries were not compiled with the Thumb-2 instruction set (which is the only one supported by those ARM CPUs) variant. Sam -- Samuel Tardieu -- s...@rfc1149.net -- http://www.rfc1149.net/
Submitting a patch for an architecture without a maintainer
Hi. GCC 4.5.0 will ship with support for lm32. This is an IP which can be configured in several ways (with or without barrel shifter, and with or without a hardware multiplier). To be usable in all cases, it has to support multilib. The support is present but broken: the "MULTILIB_OPTIONS" variable is located in the wrong file! I submitted a small patch from a colleague of mine (http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00307.html) and sent three PING already. But since there is no listed maintainer for lm32, it seems that it doesn't draw any attention from anyone who can approve it, so I cannot check it in. If we do not apply it, the multilib option for this new target will be broken in 4.5.0. I know we are late in the development stage, but lm32 is a new target and reading the patch makes it clear that it cannot break any other platform. I know you guys are all very busy but I am not able to reach anyone interested in lm32 on gcc-patches. Could someone with global commit rights have a look at this patch and approve/reject/delay it? http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00307.html Sam PS/ Note that I don't use this architecture myself and I am not interested in becoming its maintainer -- I just happen to know someone who uses it and noticed that GCC was broken. -- Samuel Tardieu -- s...@rfc1149.net -- http://www.rfc1149.net/
Re: ada.h _ECHO
>>>>> "andrew" == andrew m goth <[EMAIL PROTECTED]> writes: andrew> For the non-__STDC__ case, why is the macro called _ECHO on andrew> one line and ECHO on the next? Given that GNAT can only be compiled with GNAT, I bet the non __STDC__ case has not been exercized for years and can simply be removed. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: GCC 4.4.0 Status Report (2009-03-13)
>>>>> "Richard" == Richard Kenner writes: Richard> He's the President of the organization, so he has the Richard> ultimate authority over any FSF project. According to the FSF by-laws? Oh, wait, do you mean you actually had access to them? You're much more lucky than most of us then :) Sam -- Samuel Tardieu -- s...@rfc1149.net -- http://www.rfc1149.net/
Multilib for ARM in thumb2 mode
Right now, to be able to compile a mutilib-enabled ARM-targeted compiler supporting thumb2, one has to uncomment some code in gcc/config/arm/t-arm-elf. Given that thumb2 processors are more and more used (such as microcontrollers based on the Cortex-M3), wouldn't it make sense to unconditionnaly compile the thumb2 version of the library in 4.5.0 and 4.4.1? If people don't want it, could we add an option to enable thumb2 at configure time? Sam -- Samuel Tardieu -- s...@rfc1149.net -- http://www.rfc1149.net/
Re: Multilib for ARM in thumb2 mode
* Alexandre Pereira Nunes [2009-05-08 11:24:14 -0300] | > Don't you already get this if you created a toolchain with | > --with-arch=armv7-m and the multilib for thumb ? A toolchain configured for | > --with-arch=armv7-m and the multilib for thumb would automatically cover | > this configuration. By default we compile for armv4t and why should one | > have | > an additional multi-lib for thumb2 in such a case ? | | And how would someone generate code for an armv4t, or v5, or even v6, with | thumb1, and other scenarios, using a toolchain built the way you suggest? | Samuel's idea seems to increase the multilib range to cover most arm | configurations, and not to generate a build restricted to lesser | configurations. To cope with your idea and still get the other cases, one | would have to tweak the multilib configuration anyway, so Samuel's idea | seems the way to go from the start. Yes, this is exactly what I was thinking: cover more configurations. For example, in the engineering school where I teach, we have many ARM boards with different CPUs, and we would really like to have a single cross-compiler that our students can use or install on their own machines. Sam
Double constructors in C++?
Hi.
Why are constructors included twice in object code? This is with
GCC 4.3.0 20080214:
% cat t.cc
#include
class T{
T(const std::string&);
int foo();
int x;
};
T::T(const std::string&) {x=2;}
int T::foo()
{
x=7;
}
% g++ -c -o t.o t.cc -fomit-frame-pointer -O
% nm -S -C t.o
0018 000b T T::foo()
000c 000b T T::T(std::string const&)
000b T T::T(std::string const&)
% objdump -D t.o
Disassembly of section .text:
<_ZN1TC2ERKSs>:
0: 8b 44 24 04 mov0x4(%esp),%eax
4: c7 00 02 00 00 00 movl $0x2,(%eax)
a: c3 ret
b: 90 nop
000c <_ZN1TC1ERKSs>:
c: 8b 44 24 04 mov0x4(%esp),%eax
10: c7 00 02 00 00 00 movl $0x2,(%eax)
16: c3 ret
17: 90 nop
0018 <_ZN1T3fooEv>:
18: 8b 44 24 04 mov0x4(%esp),%eax
1c: c7 00 07 00 00 00 movl $0x7,(%eax)
22: c3 ret
The constructor seems to be included twice with the very same code. Is
that expected?
Sam
--
Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: When the R.I.P. of 4.1.x branch for?
>>>>> "JCP" == J C Pizarro <[EMAIL PROTECTED]> writes: JCP> I want to see a comparison of performances between 4.1.x, 4.2.x JCP> and 4.4.x to know how they have been evolved. What prevents you from doing the comparaison yourself using SVN? Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Fwd: Porting gcc to a new architecture
>>>>> "Schmave" == Schmave <[EMAIL PROTECTED]> writes: > i was wondering what steps i need to take to port gcc to a new > architecture Did you find something unclear in the GCC documentation? Do you have a specific question? Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Official GCC git repository
>>>>> "Bernardo" == Bernardo Innocenti <[EMAIL PROTECTED]> writes: Bernardo> If people think it would be useful, we could install a commit Bernardo> hook on gcc.gnu.org to mirror each commit immediately. It would be great! Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Obvious problem with Ada front-end patches handling
Samuel Tardieu wrote: > I cannot seem to find an answer to this PING message from last > December. > > > Subject: [PATCH] ada: Do not let a type in an aggregate be an ancestor > > of itself > > Date:Thu, 29 Nov 2007 11:43:44 +0100 > > URL: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01670.html > > > > Subject: [PATCH] ada: Do not optimize X*(2**Y) in a modular context > > Date:Tue, 20 Nov 2007 17:02:43 +0100 > > URL: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01215.html > > > > Subject: [PATCH] ada: Simplify redundant test > > Date:Sat, 17 Nov 2007 16:03:15 +0100 > > URL: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01216.html > > > > Subject: [PATCH] ada: Do not save and restore environment if generic > > instance is a top-level one > > Date:Tue, 20 Nov 2007 14:49:19 +0100 > > URL: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01214.html > > > > Subject: [PATCH] ada: Do not allow "use" of something which is not a > > package > > Date:Tue, 20 Nov 2007 15:56:08 +0100 > > URL: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01212.html Still no answer after two PING (December and February, for patches submitted in November, and there are others not mentionned here). Is there any Ada front-end maintainer handling patches proposals? Since 2007-12-19, which is the date of the latest AdaCore batch merge (three months ago), the Ada front end has only received the following patches: - platform-specific and build-related patches - gigi (Ada structures -> GCC structures) patches from Éric Botcazou in *.c files - typos and documentation patches - two patches from me: - one which happened to have been integrated in AdaCore version and announced as a new feature on their web site without me receiving a ACK (nor a NACK) to check it in the FSF tree (http://thread.gmane.org/gmane.comp.gcc.patches/153380/focus=157411) - one which is a 4.3 regression for which I never received a ACK (nor a NACK) to check it in the 4.3 branch (http://thread.gmane.org/gmane.comp.gcc.patches/157121/focus=157407) Most if not all other Ada front-end patches have received no answer. Maybe the patches are wrong and need some more work, but it is really frustrating to get no feedback at all. Not many people volunteer to work on Ada-related bug reports from the community against the FSF version, and when they do, it sometimes appears completely useless. >From MAINTAINERS: | Ada front end Geert Bosch [EMAIL PROTECTED] | Ada front end Robert Dewar[EMAIL PROTECTED] | Ada front end Arnaud Charlet [EMAIL PROTECTED] I am perfectly fine with all three Ada front end maintainers being from AdaCore, but at this time it looks like the FSF GNAT front end is totally stalled because of this. I want to know if I am wasting my time trying to get the FSF GNAT version in a better shape, in which case I can certainly use it on something more productive. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Obvious problem with Ada front-end patches handling
On 20/03, Arnaud Charlet wrote: | > Since 2007-12-19, which is the date of the latest AdaCore batch | > merge (three months ago), the Ada front end has only received the | > following patches: | > | > - platform-specific and build-related patches | > | > - gigi (Ada structures -> GCC structures) patches from Éric Botcazou | > in *.c files | | In addition, Eric's changes are definitely part of AdaCore's regular | synchronizations to the FSF tree, and these represent quite a few critical | changes. So your impression that "no changes have been made to the FSF tree by | AdaCore for 3 monthes" is incorrect. Éric patches hardly belong to what I call "AdaCore batch merges", that is: - large merges done in a very short period of time - no corresponding test cases or non-regression tests - no reference to existing PR - no closing of corresponding bugzilla entries As a consequence, the last two items require that other people retest the Ada bugs in bugzilla and close them if they happen to have been fixed by chance by the last jumbo merge. If people don't do that, the bugzilla is clogged with old useless entries and those volunteering to fix Ada bugs in GCC (as I do) lose a lot of time figuring out which bug they should work on and whether it is still relevant or not. Éric patches do reference PR and add corresponding tests, as is done in the rest of GCC and as yourself require before accepting contributed patches for the Ada front-end. Doing a egrep '(adacore.com|act-europe.fr|gnat.com)' gcc/testsuite/ChangeLog and comparing this to the gcc/ada/ChangeLog speaks for itself. So no, I was definitely not talking about Éric patches when writing about "AdaCore batch merges". Moreover, either you consider that Éric changes are part of the Ada front-end and this means that it was possible to get changes in, or you consider that they are not part of the front-end (but rather the middle-end) which means that the front-end is really stalled. | As I said, I am working on putting many changes at the FSF tree right now, | which should happen within the next few weeks, and for a week or so, since there | are many patches to test and incorporate. That is in itself good news. | I hope this clarifies things. This partially explains why external patches were not approved, but this doesn't explain why they didn't receive *any* feedback in a reasonable time. I certainly appreciate all the very high quality support I get from AdaCore when working on fixing a bug in GNAT or trying to get a new feature in, they have always been of tremendous help and it is always a real pleasure to work with them. What I am disatisfied with is the way Ada front-end maintainers for the FSF GNAT tree have been handling (or rather not handling) patches for a few months without any explanation and how large changes are pushed in without taking care of proper GCC procedures. Maybe you think that there is no problem here, but from a volunteering contributor point of view, there is. Sam
Re: Obvious problem with Ada front-end patches handling
On 20/03, Arnaud Charlet wrote: | Maybe that's partly because you are not following closely the development of | GCC and are not familiar with the GCC stages. Submitting patches during | stage3 (in particular end of stage3) is not really the most appropriate time, | so it's often better to wait for stage 1 or stage 2 for such submissions. I was under the impression that stage 3 was perfectly fine to submit bug fixes. Of the 5 patches I mentionned in the first mail of this thread, 4 were bug fixes fitting the description outlined in http://gcc.gnu.org/develop.html: "the only (non-documentation) changes that may be made are changes that fix bugs or new ports which do not require changes to other parts of the compiler." Asking contributors to wait for stage 1 or 2 to submit bug fixes to the Ada compiler is not a good way to encourage contributions. As a result, those four bugs are still present in GCC 4.3.0 while fixes were proposed in November, and since they were not regressions, are not likely to be fixed in GCC 4.3.x releases either and will have to wait for GCC 4.4.0. Sam
Re: Official GCC git repository
>>>>> "Paolo" == Paolo Bonzini <[EMAIL PROTECTED]> writes: >> I was only suggesting it as a nicity, if people are happy with the >> login name alone. Paolo> What about "Real Name <[EMAIL PROTECTED]>"? The overseers have Paolo> the mapping, or you can sort of guess it from the names in the Paolo> ChangeLog. This has to be decided before the first push, so Paolo> it's kind of urgent to decide it. I also think adding the Real Name is nice. Maybe we should also consider adding the login as an additional field in the MAINTAINERS file, as AFAIK every MAINTAINERS as a @gcc.gnu.org address. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Official GCC git repository
>>>>> "Daniel" == Daniel Berlin <[EMAIL PROTECTED]> writes: Daniel> No, nothing is wrong wit he import, if you want all the remote Daniel> branches, you have to ask git to get all the remote branches Daniel> git config --add remote.origin.fetch '+refs/remotes/*:refs/remotes/*' Daniel, how did you setup the git repository? The branch heads are supposed to be located under "/refs/heads/" on gcc.gnu.org, not under "/refs/remotes/", where you generally find the remote references: those branches aren't supposed to be remote on the master repository. If you populate this repository by pushing from another one, the push command should use "--mirror". Excerpt from the manual: --mirror Instead of naming each ref to push, specifies that all refs under $GIT_DIR/refs/heads/ and $GIT_DIR/refs/tags/ be mirrored to the remote repository. Newly created local refs will be pushed to the remote end, locally updated refs will be force updated on the remote end, and deleted refs will be removed from the remote end. This way, branches will be kept under /refs/heads/, not put under /refs/remotes/. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Official GCC git repository
>>>>> "Daniel" == Daniel Berlin <[EMAIL PROTECTED]> writes: Daniel> I put my version of the gcc conversion (which has all branches Daniel> but no tags) at git://gcc.gnu.org/git/gcc.git and set a script Daniel> up to update it appropriately. Daniel, how is the GIT repository synced? Is there a hook which updates it after a svn commit? I'm about to switch from infradead to gcc.gnu.org, and I want to make sure that the latter is synced at least as frequently as the former before I do that. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Official GCC git repository
On 18/04, Daniel Berlin wrote: | > how is the GIT repository synced? Is there a hook which updates it | > after a svn commit? | | No. | It is synced every 30 minutes. | | > | > I'm about to switch from infradead to gcc.gnu.org, and I want to make | > sure that the latter is synced at least as frequently as the former | > before I do that. | | I have strong doubts that whether it is synced every commit or every | 30 minutes seriously affects your development. Where did I wrote that it would "seriously affect my development"? However, having it synced periodically rather than after every commit is an annoyance. For example, when I see an interesting commit, I sometimes want to look at it once installed in the sources; the easiest solution is to refresh my git repository and look at it. If it's not there yet, it's an annoyance. When one of my patches get approved, before committing it, I rebase it on the latest sources and put it at the bottom of my patch stack and checks that it at least still compiles cleanly. If git is not in sync, it's an annoyance. When I commit one of my patches, I remove it from my git patch stack and rebase other patches on top of the upstream git repository, which should now include the patch I just checked in. If git is not in sync, I have to either leave for up to 30 minutes with one patch missing, or I have to wait for up to 30 minutes before I can remove the original from my tree. It's an annoyance. As you see, no, it doesn't "seriously affect my development", it just makes it less practical and, more importantly from my point of view, less fun. Especially when there are no obstacles to making it be more practical but a one-time setup. So having git refresh itself after a SVN commit would be great :)
Re: Official GCC git repository
On 18/04, Daniel Berlin wrote: | > However, having it synced periodically rather than after every commit is | > an annoyance. | | True, but it won't change anytime soon because it would place more | load, and require more locking (since there is no guarantee a git sync | will finish before the next commit occurs). More load? Come on, you are doing 48 "svn update" a day right now by syncing every 30 minutes without even knowing if a commit took place or not. What I am proposing would make one "svn update" per svn commit. Hardly more load, probably even less. As far as locking is concerned, I was thinking of a procmail script catching mail received from gcc-cvs and doing something like: :0:/tmp/svn2git * ^To: [EMAIL PROTECTED] | (cd /to/git/repo && git svn fetch) This is not hard to do, put an equivalent or a lighter strain on the svn server, gives a much better synchronized repository and takes care of the locking. And if you want to make sure that you don't have more than one svn update per minute in case something goes wrong, adding a "sleep 60" between the parentheses won't change the number of "svn update" done by this procmail script but will at least space them in time. | > So having git refresh itself after a SVN commit would be great :) | | Again, not gonna happen anytime soon. | Sorry. I know that you don't like git, but I was happy to see that you volunteered anyway to setup the GIT repository when the person who was supposed to do it became MIA. But now that you have done the hard part, I don't understand why you explicitly refuse to do the small part which is needed to make the GIT repository that you populated easy and pleasant to use. If it is because you don't have time to do it, I am volunteering to finish the setup, make it ready for wide use, and setup what is necessary to keep it up-to-date, just send me the required credentials so that I can take care of it. Sam
Re: Official GCC git repository
On 18/04, Daniel Berlin wrote: | I know you think this is personal, as per your email below, but it really isn't. | I had it set up to do it after every commit, and it drove our load | average up a noticeable amount. | As such, I stopped doing it and set it to run every 30 minutes. | | > I know that you don't like git, but I was happy to see that you volunteered | > anyway to setup the GIT repository when the person who was supposed to | > do it became MIA. But now that you have done the hard part, I don't | > understand why you explicitly refuse to do the small part which is | > needed to make the GIT repository that you populated easy and pleasant | > to use. | | I'm not sure why you think this is some personal vendetta I have. | I tried what you ask, it causes noticeably more load, so i stopped doing it. I don't think this is personal at all, on the contrary: you stated publicly that GIT is not your SCM of choice, which is of course perfectly right, and you set it up anyway, so I don't think you have a personal vendetta running against git :) | I can't see how only being updated every 30 minutes is somehow "less | easy and pleasant to use". | This is the price you pay for not being the main version control system. | The same is true of the mercurial mirror, it is only updated every 30 minutes. | I have not discriminated against git in any way, shape or form, and in | fact had tried what you suggest before moving to this model. | | FWIW, if the git repo or the mercurial repo started seriously driving | up the load average of the server, we would turn them off, because | neither is the main version control system of gcc. | This is just the price you pay for not being the official repo. I think the mistake is to have them (git & hg) hosted on the same machine as svn. Having them on "hg.gcc.gnu.org" and "git.gcc.gnu.org" would allow to split the load between machines (even if "hg.gcc.gnu.org" and "git.gcc.gnu.org" are the same machines originally).
Re: Official GCC git repository
On 18/04, Christopher Faylor wrote: | After consultation with Dan, I have set things up on gcc.gnu.org so that | the git repository is updated every time an email message is received | from the gcc-cvs mailing list. Great! Thanks to both of you.
ada: What is libgnalasup?
In Interfaces.Fortran.Blas, there is a reference to libgnalasup. It has been introduced by this commit: Author: charlet <[EMAIL PROTECTED]> Date: Tue Aug 14 08:50:51 2007 + 2007-08-14 Geert Bosch <[EMAIL PROTECTED]> * i-forbla.ads, i-forbla.adb, a-ngcoar.adb, a-ngcoar.ads, i-forlap.ads, s-gearop.adb, s-gecobl.adb, s-gecobl.ads, s-gerela.adb, s-gerela.ads: Add required linker pragmas for automatically linking with the gnalasup linear algebra support library, and the systems math library. Rename cdot to cdotu and zdot to zdotu. Update header comment to describe purpose of package. The only reference I can find about gnalasup is here: http://www.christ-usch-grein.homepage.t-online.de/Ada/SI.html Is this library something written by AdaCore? Will it be contributed in GCC? Otherwise, it looks like we can replace the reference in GCC i-forbla.adb by references to liblapack and libblas instead. Would that be functionally equivalent? Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: ada: What is libgnalasup?
>>>>> "Arnaud" == Arnaud Charlet <[EMAIL PROTECTED]> writes: Arnaud> Yes, libgnalasup is a bundled version of lapack/blas Arnaud> libraries. Thanks. I've submitted a patch to make lapack/blas the default version. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Official GCC git repository
>>>>> "Christopher" == Christopher Faylor <[EMAIL PROTECTED]> writes: Christopher> After consultation with Dan, I have set things up on Christopher> gcc.gnu.org so that the git repository is updated every Christopher> time an email message is received from the gcc-cvs Christopher> mailing list. Christopher> We'll be monitoring the system to see if there is a load Christopher> hit. If so, we'll probably drop back to Dan's original Christopher> method. It looks like the GIT repository hasn't been synced since yesterday. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Official GCC git repository
On 24/04, Christopher Faylor wrote: | In case it isn't obvious, I don't know how to fix this. Well, I guess it fixed itself then since it's now working again :)
Re: s-oscons technique does not work for RTEMS
>>>>> "Thomas" == Thomas Quinot <[EMAIL PROTECTED]> writes: Thomas> As an alternative to Arno's suggestion, maybe you could use Thomas> the --with-sysroot configure parameter to make the required Thomas> headers available to the build process. I know others have Thomas> used this method on some cross targets. Do you mean --with-build-sysroot? Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
-fno-tree-cselim not working?
With yesterday's SVN sources, the following source produces the same
result with -ftree-cselim and -fno-tree-cselim on x86:
#include
static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static int acquires_count = 0;
int
trylock()
{
int res;
res = pthread_mutex_trylock(&mutex);
if (res == 0)
++acquires_count;
return res;
}
trylock:
subl$12, %esp
movl$mutex, (%esp)
callpthread_mutex_trylock
movlacquires_count, %edx
cmpl$1, %eax
adcl$0, %edx
movl%edx, acquires_count
addl$12, %esp
ret
Shouldn't the conditional store be preserved when -fno-tree-cselim is
set?
Sam
--
Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: -fno-tree-cselim not working?
On 25/10, Andrew Pinski wrote: | On 10/25/07, Samuel Tardieu <[EMAIL PROTECTED]> wrote: | > With yesterday's SVN sources, the following source produces the same | > result with -ftree-cselim and -fno-tree-cselim on x86: | | Because another patch does this optimization, the RTL if conversion | pass which implements conditional moves. Is there any way to turn it off (other than using -O0) (see current thread-safeness discussion)? It looks like this particular optimization is duplicated, as the comment in tree-ssa-phiopt.c exactly describes what happens here: /* This pass tries to transform conditional stores into unconditional ones, enabling further simplifications with the simpler then and else blocks. In particular it replaces this: bb0: if (cond) goto bb2; else goto bb1; bb1: *p = RHS bb2: with bb0: if (cond) goto bb1; else goto bb2; bb1: condtmp' = *p; bb2: condtmp = PHI *p = condtmp This transformation can only be done under several constraints, documented below. */
Re: -fno-tree-cselim not working?
>>>>> "David" == David Miller <[EMAIL PROTECTED]> writes: David> I suspect that local.a++ will be represented as a frame relative David> access. Test compiles show that gcc does update the on-stack David> copy on Sparc, It does on i386 too. David> now just to check if the conditional memory operation cases David> trigger here too. It doesn't because you have the spin_unlock() in the "then" part so it is no longer a single assignment (the only case that gets "optimized" currently). Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Optimization of conditional access to globals: thread-unsafe?
>>>>> "Sam" == Samuel Tardieu <[EMAIL PROTECTED]> writes: Sam> In the following example, is the access to "Shared" considered Sam> unsynchronized even though what looks like a proper lock is used Sam> around it? Call to Always_Unlock was incorrect in the previous example, a fixed one exhibiting the bug is now at http://pastebin.com/f1bc7ba32 Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Optimization of conditional access to globals: thread-unsafe?
On 26/10, Robert Dewar wrote: | Of course in Ada there is a clear notion of threads semantic, and | a clear definition of what the meaning of code is in the presence | of threads, so the specific situation discussed here is easy to | deal with (though Ada takes the view that unsychronized shared access to | non-atomic or non-volatile data from separate threads has undefined | effects). In the following example, is the access to "Shared" considered unsynchronized even though what looks like a proper lock is used around it? package P is Shared : Natural := 0; procedure Maybe_Increment; end P; package body P is protected Lock is procedure Maybe_Lock (Locked : out Boolean); procedure Always_Unlock; private Is_Locked : Boolean := False; end Lock; protected body Lock is procedure Always_Unlock is begin Is_Locked := False; end Always_Unlock; procedure Maybe_Lock (Locked : out Boolean) is begin Locked:= not Is_Locked; Is_Locked := True; end Maybe_Lock; end Lock; procedure Maybe_Increment is L : Boolean; begin Lock.Maybe_Lock (L); if L then Shared := Shared + 1; end if; Lock.Always_Unlock; end Maybe_Increment; end P; By naively reading the code, I would assume that if two tasks were to call Maybe_Increment once, after completion of those tasks Shared would contain either 1 or 2, depending on whether they both got the lock in turn or if only one of them got it. However, if you look at the x86 code for Maybe_Increment (-O3 -fomit-frame-pointer -fno-inline), you'll see: 1 p__maybe_increment: 2 .LFB11: 3 subl$12, %esp 4 .LCFI6: 5 movl$p__lock, %eax 6 callp__lock__maybe_lockP 7 cmpb$1, %al 8 movlp__shared, %eax <=== unconditional load 9 sbbl$-1, %eax <=== conditional +1 10 movl%eax, p__shared <=== unconditional store 11 movl$p__lock, %eax 12 addl$12, %esp 13 jmp p__lock__always_unlockP Note lines 8 to 10: on a multiprocessor system with both tasks running at the same time on different processors, you can end up with Shared being zero after the two tasks have ended (for example if the task getting the lock runs one or two instructions ahead the one without the lock). Sam
Re: -fno-tree-cselim not working?
>>>>> "Dave" == Dave Korn <[EMAIL PROTECTED]> writes: Dave> I certainly won't object to any move to prohibit the Dave> read-conditional-add-write (and related) optimisation(s) when Dave> compiling with an option that explicitly specifies that we are Dave> compiling multi-threaded code. There are two spots I could find in ifcvt.c: /* Convert "if (test) foo++" into "foo += (test != 0)", and similarly for "foo--". */ and /* Convert "if (test) x = 0;" to "x &= -(test == 0);" */ Would a patch such as the following be acceptable? http://article.gmane.org/gmane.comp.gcc.patches/151261 Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Optimization of conditional access to globals: thread-unsafe?
On 27/10, Florian Weimer wrote: | (I can't reproduce the conditional store with my GCC 4.2 installation, | though.) You need "-O -fno-inline" to trigger it on this particular example (you don't need "-fno-inline" if you put "Lock" in a separate package).
Re: Optimization of conditional access to globals: thread-unsafe?
On 27/10, Robert Dewar wrote: > I don't understand, are we looking at the same example, the example > from Sam that I looked at did not have an entry body, so how could the > entry body rule apply? Let's look at the example (where I replaced protected procedures by entries with a guard which is always True). As per RM95 9.10, I understand that the calls to Maybe_Lock/Maybe_Unlock act as a synchronization point for two tasks executing Maybe_Increment: http://pastebin.com/f35d673e9 This looks to me like a proper way to "synchronize the actions of two or more tasks to allow, for example, meaningful communication by the direct updating and reading of variables shared between the tasks" (RM citation). However, despite the fact that the first task will get what looks like a lock (L is True), with the current "optimization" the other one will be able to modify the Shared variable even though the Ada code is not supposed to read or write in its memory location (as L will be False in the second task since the lock has not been acquired as it has been granted to the first task). I admit that this example may be contrived and not occur in real-life code, but I am surprised that Ada allows the Shared variable to be updated to its older value by a race condition in this case.
Re: Optimization of conditional access to globals: thread-unsafe?
>>>>> "skaller" == skaller <[EMAIL PROTECTED]> writes:
skaller> Since 'a' here is sharable, the function can assume it is not
skaller> aliased in a register, load and increment it and store it
skaller> back.
skaller> It doesn't matter then, whether there is a mutex or not. In
skaller> fact, it doesn't matter if locked is true or false.
skaller> I also can't see anything at all is lost here.
Back to the beginning of this thread. The problem is that when locked
is false, the compiler writes (the previous value) to it anyway.
Look at the generated code for this example with the current
compiler. It is compiled as if it had been written as:
int a;
void foo(bool locked)
{
a -= locked ? -1 : 0;
}
with (x86):
movla, %eax
cmpl$1, 4(%esp)
sbbl$-1, %eax
movl%eax, a
There is clearly a race condition if you have multiple threads
executing this code even if only one thread has "locked" being true.
Sam
--
Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: 64 bit assignment trouble on a 32 bit platform
[this is not a question for the GCC developers list]
Kumaresh> Is it necessary to type-cast both a32 and b32 to make above
Kumaresh> statements to work as expected?
Yes.
Kumaresh> Without casting, I find that addition works while the
Kumaresh> multiplication does not.
Addition is not supposed to work either. I guess that your addition
just doesn't overflow the capacity of an UINT32 while your
multiplication does.
The following program shows it clearly:
#include
typedef long long unsigned UINT64;
typedef long unsigned UINT32;
int main(void)
{
UINT32 a = 0x;
UINT32 b = a;
printf ("a+b = 0x%016llx\n", (UINT64) (a+b));
return 0;
}
The output will be:
a+b = 0xfffe
instead of your expected
a+b = 0x0001fffe
Sam
--
Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
volatile and R/M/W operations
When looking at an Ada PR, I stumbled upon the equivalent of the
following C code:
unsigned char x;
volatile unsigned char y;
void f ()
{
x |= 16;
y |= 32;
}
With trunk/i686, the following code is generated (-O3 -fomit-frame-pointer):
f:
movzbl y, %eax
orb $16, x
orl $32, %eax
movb%al, y
ret
I cannot see a reason not to use "orb $32,y" here instead of a three
steps read/modify/write operation. Is this only a missed optimization?
(in which case I will open a PR)
Sam
--
Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: volatile and R/M/W operations
On 1/12, Robert Dewar wrote: >> I cannot see a reason not to use "orb $32,y" here instead of a three >> steps read/modify/write operation. Is this only a missed optimization? >> (in which case I will open a PR) > > Are you sure it is an optimization, the timing on these things is > very subtle. What evidence do you have that there is a missed > optimization here? For this pattern (isolated setting of one bit in the middle of a byte at a random memory location), this is the best code on this platform AFAIK. As an evidence, if you mark neither variable as volatile GCC generates with -O3 -fomit-frame-pointer: f: orb $16, x orb $32, y ret And I sure expect that GCC didn't choose to generate worst code when I *removed* the volatile constraint :)
Rant about ChangeLog entries and commit messages
As a recent committer to GCC, I am always surprised to see the content of ChangeLog entries and commit messages. I tend to find GCC ChangeLog entries useless. For example, the more recent ChangeLog entry in gcc/ChangeLog is: | 2007-11-30 Jan Hubicka <[EMAIL PROTECTED]> | | * ggc-common.c (dump_ggc_loc_statistics): Reset ggc_force_collect | flag. How could a newcomer guess why the gcc_force_collect flag needs to be reset? Jan posted a useful explanation on gcc-patches, but finding it by searching the mailing-list is not practical and it is not coupled with the checkin. Let's look at the corresponding svn log message, which can be found with "svn blame" if a particular line needs to be pinpointed: | r130560 | hubicka | 2007-12-01 22:06:31 +0100 (Sat, 01 Dec 2007) | 4 lines | | * ggc-common.c (dump_ggc_loc_statistics): Reset ggc_force_collect | flag. Ok, same information is mirrored here, not useful. Let's look at the change itself then: | Index: gcc/ChangeLog | === | --- gcc/ChangeLog (revision 130559) | +++ gcc/ChangeLog (revision 130560) | @@ -1,3 +1,8 @@ | +2007-11-30 Jan Hubicka <[EMAIL PROTECTED]> | + | + * ggc-common.c (dump_ggc_loc_statistics): Reset ggc_force_collect | + flag. | + | 2007-11-30 Seongbae Park <[EMAIL PROTECTED]> | | PR rtl-optimization/34171 | Index: gcc/ggc-common.c | === | --- gcc/ggc-common.c(revision 130559) | +++ gcc/ggc-common.c(revision 130560) | @@ -1018,5 +1018,6 @@ |fprintf (stderr, "%-48s %10s %10s %10s %10s %10s\n", |"source location", "Garbage", "Freed", "Leak", "Overhead", "Times"); |fprintf (stderr, "---\n") | ; | + ggc_force_collect = false; | #endif | } Ok, still the same information because of the ChangeLog diff, and we can see that the change shows that... gcc_force_collect is reset. Wow! Now, compare that with Jan's message on the list: | pre-ipa-mem-reports force GGC to be done at each invokation in order to | collect data on live memory references, but forgets to reset the flag | after done. This means that compiler continues GGCcollecting and works | slowly. This *is* the information I would expect to be present somewhere in GCC history. A clear and detailed information on why the change was necessary. Sure, in some case the checkin references a PR, but the PR often contains information of what didn't work before the change and the same information which is already repeated three times (ChangeLog, svn log and svn diff). Compare this to a typical commit in the Linux kernel: | commit b1812582ba94b5f377d5d3cec7646cc17d84e733 | Author: Joachim Fenkes <[EMAIL PROTECTED]> | Date: Fri Nov 30 16:19:41 2007 -0800 | | IB/ehca: Fix static rate if path faster than link | | The formula would yield -1 if the path is faster than the link, which | is wrong in a bad way (max throttling). Clamp to 0, which is the | correct value. | | Signed-off-by: Joachim Fenkes <[EMAIL PROTECTED]> | Signed-off-by: Roland Dreier <[EMAIL PROTECTED]> | | diff --git a/drivers/infiniband/hw/ehca/ehca_av.c b/drivers/infiniband/hw/ehca/ehca_av.c | index 453eb99..f7782c8 100644 | --- a/drivers/infiniband/hw/ehca/ehca_av.c | +++ b/drivers/infiniband/hw/ehca/ehca_av.c | @@ -76,8 +76,12 @@ int ehca_calc_ipd(struct ehca_shca *shca, int port, | | link = ib_width_enum_to_int(pa.active_width) * pa.active_speed; | | - /* IPD = round((link / path) - 1) */ | - *ipd = ((link + (path >> 1)) / path) - 1; | + if (path >= link) | + /* no need to throttle if path faster than link */ | + *ipd = 0; | + else | + /* IPD = round((link / path) - 1) */ | + *ipd = ((link + (path >> 1)) / path) - 1; | | return 0; | } What do we have here? A one-line high-level description identifying what has been done, a synthetic analysis of the problem cause and its solution, and the audit trail with the Signed-off-by lines (which in the Linux case is more important than in GCC as copyrights are not assigned to one entity). Linux doesn't use ChangeLog, but its history is much more useful to developers and casual observers than GCC one. And it could be done for GCC (with SVN) as well. Maybe we should consider dropping ChangeLogs and using better checkin messages. Sam
Re: Rant about ChangeLog entries and commit messages
On 2/12, Eric Botcazou wrote: | He indeed cannot, but the ChangeLog is not meant to make it possible either. | See http://gcc.gnu.org/contribute.html, especially the GNU Coding Standards. I know this document and I think the part on ChangeLog doesn't achieve its purpose: http://www.gnu.org/prep/standards/standards.html#Change-Logs Keep a change log to describe all the changes made to program source files. The purpose of this is so that people investigating bugs in the future will know about the changes that might have introduced the bug. Often a new bug can be found by looking at what was recently changed. More importantly, change logs can help you eliminate conceptual inconsistencies between different parts of a program, by giving you a history of how the conflicting concepts arose and who they came from. This is precisely why I am proposing an evolution in the current process. Also, this document states: There's no need to describe the full purpose of the changes or how they work together. If you think that a change calls for explanation, you're probably right. Please do explain it—but please put the explanation in comments in the code, where people will see it whenever they see the code. When you fix a bug by changing a constant (for example if there has been an offset by one error or, as I did a few minutes ago in config/sh/sh.md, there was an error in the argument to consider), this doesn't always mandate a comment in the code. For example, I think a description such as the one I wrote when describing the problem cmpgeusi_t splitting code compares operand 0 to 0, while this constant value can only be in operand 1. When compiling the Ada runtime, this leads to a "cmp/hs #0,r7" instruction which is not valid as "cmp/hs" operands must be two registers. along with the above change would have been a better commit message than just gcc/ * config/sh/sh.md (cmpgeusi_t): Fix condition. which I used as suggested. | That's how it has always worked so it should be more or less practical. Sure, it works. But this is not a reason not to improve the process. | For PRs, there is a link (URL: field), maybe we should use PRs more often. This field is useful to look at the discussion that led to the change, but PRs often contain no synthetic information on the analysis of the problem unless when the PR submitter sends a patch himself (in which case he often includes his analysis to get a better chance to get his patch checked in).
Re: Rant about ChangeLog entries and commit messages
On 2/12, Andreas Schwab wrote: | That is supposed to be written in a comment. The change log entry | should describe _what_ is being changed, so that you can find out when a | particular change was made. This should be the job of the VCS, e.g. "svn log" and "svn blame". Moreover, ChangeLogs are organized by directories. You have to look at the "svn log" to see if a test corresponds to a code change and identify it.
Re: Rant about ChangeLog entries and commit messages
On 2/12, Eric Botcazou wrote: | > I know this document and I think the part on ChangeLog doesn't achieve | > its purpose: | > | > http://www.gnu.org/prep/standards/standards.html#Change-Logs | > | >Keep a change log to describe all the changes made to program source | >files. The purpose of this is so that people investigating bugs in the | >future will know about the changes that might have introduced the bug. | >Often a new bug can be found by looking at what was recently changed. | >More importantly, change logs can help you eliminate conceptual | >inconsistencies between different parts of a program, by giving you a | >history of how the conflicting concepts arose and who they came from. | | Could you elaborate? I'll take an example from one of your recent changes in gcc/ChangeLog: 2007-11-19 Eric Botcazou <[EMAIL PROTECTED]> * stor-layout.c (lang_adjust_rli): Delete. (set_lang_adjust_rli): Likewise. (layout_type): Do not call lang_adjust_rli hook. * tree.h (set_lang_adjust_rli): Delete. Without digging in the mailing-list archives to see why you made the change, if something new breaks on a STABS platform I will have no hint that this change was in any way related to STABS. If we didn't use ChangeLogs and if commit logs contained the information you gave on the mailing-list, this would be much easier: The compiler has been broken on STABS platform since mapped locations were enabled by default. The Ada front-end is emitting debug info too early. The patch also reorganizes a little the front-end's initialization and gets rid of dead code in the process, which in turn enables a further cleanup in the middle-end. Also note that the ChangeLog doesn't give any hint that changes in the ada directory have been made at the same time, only "svn log" reveals that. So far for the "The purpose of this is so that people investigating bugs in the future will know about the changes that might have introduced the bug." sentence. I would prefer that information which is deemed necessary for peer-review when a patch is sent to gcc-patches@ is also included in GCC log history. | > [sh.md fix] | | Not really in my opinion, it's a trivial fix and totally unrelated to Ada in | itself, "Fix typo" or "Fix obvious mistake" would have been just fine too. Well, I find it useful to know which part of the compiler has exercized this code path (as obviously there was no test associated with this optimization) and uncovered the bug, but I agree that this was an obvious typo fix.
Re: Rant about ChangeLog entries and commit messages
Sam> How could a newcomer guess why the gcc_force_collect flag needs to Sam> be reset? Andreas> That is supposed to be written in a comment. The change log Andreas> entry should describe _what_ is being changed, so that you Andreas> can find out when a particular change was made. Out of curiosity, I looked for an example with one of your own commits in the Linux kernel tree and I found one: I think http://tinyurl.com/2j7lt7 is a very helpful explanation of the corresponding change (http://tinyurl.com/2tpw8l). Someone trying to fix a similar bug in another driver will benefit from having this message in the VCS history. A comment in the code would probably have been much shorter than this explanation and would probably not contain the "headphone", "line out" and "muted" words. Once again, I agree that the current mechanism works for GCC developers, but I think it could be much better if: 1- commit messages didn't duplicate ChangeLog entries (maybe by getting rid of ChangeLogs) 2- commit messages contained a synthetic information such as the one provided for peer-review I'm not trying to launch a revolution in the GCC development process, I'm only comparing two ways of documenting changes as they are committed and explaining why I find the Linux way of doing it more useful. As a side note, I know several (sick?) people (including myself) who casually read the Linux kernel RSS feed in their RSS aggregator and find it very insightful (if you exclude the "Merge" messages) while lighter than reading the whole linux-kernel mailing-list. People reading this can look at http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=atom with RSS-capable software to see what I mean. The GCC ChangeLogs, even when aggregated, aren't as nice to read when you're having breakfast :) Sam
Re: Rant about ChangeLog entries and commit messages
>>>>> "Eric" == Eric Botcazou <[EMAIL PROTECTED]> writes: >> Without digging in the mailing-list archives to see why you made >> the change, if something new breaks on a STABS platform I will have >> no hint that this change was in any way related to STABS. Eric> But this change has nothing to do with STABS. :-) Sure, but as you explained yourself in the message I cited, the reason to do this change was because of a problem in STABS info generation :) Eric> Well, any changes in the compiler can potentially introduce bugs Eric> elsewhere and I suppose that you aren't proposing to mention all Eric> the known dependencies in the commit message. Of course not, but when such a dependency is the reason you make a change in the first place, I think it ought to be mentionned. If the chance caused a regression on an obscure platform using some barely used debugging format, this could give a hint of where to look (STABS for example) to see how it is done there and if bogus assumptions were made about the previous behaviour. Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
Re: Git and GCC
>>>>> "Daniel" == Daniel Berlin <[EMAIL PROTECTED]> writes: Daniel> So I tried a full history conversion using git-svn of the gcc Daniel> repository (IE every trunk revision from 1-HEAD as of Daniel> yesterday) The git-svn import was done using repacks every Daniel> 1000 revisions. After it finished, I used git-gc --aggressive Daniel> --prune. Two hours later, it finished. The final size after Daniel> this is 1.5 gig for all of the history of gcc for just trunk. Most of the space is probably taken by the SVN specific data. To get an idea of how GIT would handle GCC data, you should clone the GIT directory or checkout one from infradead.org: % git clone git://git.infradead.org/gcc.git On my machine, it takes 856M with a checkout copy of trunk and contains the trunk, autovect, fixed-point, 4.1 and 4.2 branches. In comparaison, my checked out copy of trunk using SVN requires 1.2G, and I don't have any history around... Sam -- Samuel Tardieu -- [EMAIL PROTECTED] -- http://www.rfc1149.net/
