The atttached patch corrects the rtems specific Ada run-time files. With this patch powerpc-rtems and sparc-rtems built Ada successfully. NOTE: sparc-rtems also requires PR14537 in order to complete the build.
Created attachment 5901 [details] *-rtems Ada patch Please review so I can commit this before 3.4.0 is cut.
If an Ada maintainer signs off on this patch, it is OK for 3.4.0.
Subject: Re: *-rtems broken for gnat > Please review so I can commit this before 3.4.0 is cut. I'd suggest you post your patch with the missing ChangeLog on gcc-patches@, following the usual patch submission rules. The init.c change seems to be a no-op, so I'd rather not integrate it. Arno
Subject: Re: *-rtems broken for gnat charlet at act-europe dot fr wrote: >------- Additional Comments From charlet at act-europe dot fr 2004-03-12 06:24 ------- >Subject: Re: *-rtems broken for gnat > > > >>Please review so I can commit this before 3.4.0 is cut. >> >> > >I'd suggest you post your patch with the missing ChangeLog on >gcc-patches@, following the usual patch submission rules. > >The init.c change seems to be a no-op, so I'd rather not integrate it. > > > Actually it isn't a nop. It puts the simple __rtems__ check ahead of a check that is something like the following that I believe indicates the cpp predefines for one of the vxworks targets is broken: if (AIX or unix) and !vxworks At least some of the embedded sparc targets were built using solaris config files and ended up with unix defined. I can see where this could happen on another embedded target cpu configuration and wanted to avoid it accidentally being tripped. In fact, I submitted a fix for sparc to address this for sparc-rtems. I simply moved a very straight forward check on an OS specific predefine that is guaranteed to be there ahead of one that is questionable and could be unintentally tripped. This same problem also resulted in sys/termios.h being included from the <ctrl>c code since it ended up thinking it was on a unix system when it wasn't. I will also post the patch to the bugs list once I get in. >Arno > > > >
Subject: Re: *-rtems broken for gnat > Actually it isn't a nop. It puts the simple __rtems__ check ahead of a That's why ChangeLogs are so important :-) So we'll wait for your complete submision. > check that > is something like the following that I believe indicates the cpp > predefines for one of the > vxworks targets is broken: > > if (AIX or unix) and !vxworks That's wrong, vxworks does not define unix, you are misinterpreting the above. Arno
Add a ChangeLog and note that this has been posted to gcc-patches as http://gcc.gnu.org/ml/gcc-patches/2004-03/msg01037.html 2004-03-12 Joel Sherrill <joel@OARcorp.com> * ada/5rosinte.adb, ada/5rosinte.ads, ada/5rtpopsp.adb, ada/5rosinte.adb: Update RTEMS specific files to account for modifications to shared code. * ada/init.c: Move RTEMS code ahead of others using conditionals which mix checks for embedded systems (particularly vxworks) and Unix hosts.
Subject: Re: *-rtems broken for gnat charlet at act-europe dot fr wrote: > ------- Additional Comments From charlet at act-europe dot fr 2004-03-12 13:35 ------- > Subject: Re: *-rtems broken for gnat > > >>Actually it isn't a nop. It puts the simple __rtems__ check ahead of a > > > That's why ChangeLogs are so important :-) So we'll wait for your > complete submision. Yep. And this one is pretty complicated to explain. It makes me wonder how many targets have questionable sets of cpp predefines and no one has noticed or cares. I know that rtems has tripped across a number of these type of cases over the years. >>check that >>is something like the following that I believe indicates the cpp >>predefines for one of the >>vxworks targets is broken: >> >>if (AIX or unix) and !vxworks > > > That's wrong, vxworks does not define unix, you are misinterpreting the above. I won't argue that it shouldn't define it but at least the embedded sparc ports have historically been based upon solaris config files with some tweaks so you end up unix predefined and weird checks like this from ada/init.c #elif defined (sun) && defined (__SVR4) && !defined (__vxworks) That pretty clearly tells me that the sparc-vxworks target suffered from the same problem at some point. And this one from the same file: #elif defined(__alpha__) && defined(__osf__) && ! defined(__alpha_vxworks) which tells me that osf was probably defined for the alpha-vxworks target. So the init.c is moving the check for rtems ahead of all these cases where weirdness exists. I suspect that if the vxworks code were moved up, then you could simplify those conditionals. --joel
Subject: Re: *-rtems broken for gnat > So the init.c is moving the check for rtems ahead of all these > cases where weirdness exists. I suspect that if the vxworks > code were moved up, then you could simplify those conditionals. Thaks for the hint. We'll try to have a look at it when when start building GCC 3.4 with VxWorks, since the VxWorks ports have been completely revamped in 3.4 anyway. Arno
Subject: Re: *-rtems broken for gnat charlet at act-europe dot fr wrote: >------- Additional Comments From charlet at act-europe dot fr 2004-03-13 12:35 ------- >Subject: Re: *-rtems broken for gnat > > > >>So the init.c is moving the check for rtems ahead of all these >>cases where weirdness exists. I suspect that if the vxworks >>code were moved up, then you could simplify those conditionals. >> >> > >Thaks for the hint. We'll try to have a look at it when when start >building GCC 3.4 with VxWorks, since the VxWorks ports have been completely >revamped in 3.4 anyway. > > > Does that mean you are OK with me moving the rtems check up or that you want it to stay where it is? >Arno > > > >
Note that the run time clean up in *taprop.adb has been committed. Arno
Created attachment 6024 [details] current version of patch Arnaud .. please review so this can make it in the 3.4 series.
Subject: Re: *-rtems broken for gnat > Arnaud .. please review so this can make it in the 3.4 series. I can't review patches without their corresponding changelog. A quick look shows problems, in particular you can't remove the ATCB_Key in 7staprop.adb In any case, you ca never comment out code without adding comments justifying it. In this case, the ATCB_Key is needed, since 7stpopsp.adb depends on it. Arno
Created attachment 6031 [details] revised version of previous patch Since this patch has not been reviewed, I wanted to be sure it was the latest version and had a nice ChangeLog entry. 2004-04-01 Joel Sherrill <joel@oarcorp.com> * ada/5rosinte.adb: Remove fake mprotect() body. * ada/5rosinte.ads: Add SA_SIGINFO. Make pthread_key_t a type which can be set since Finalize_TCB in 7staprop.adb does not go through the Set_Specific interface. * ada/5rtpopsp.adb: Rewrite to use new interface. * ada/init.c: Reorder so the simple single OS conditional __rtems__ is tested before more complex ones which mix UNIX and embedded systems in the conditional. The change to make pthread_key_t a type which is not private would not be needed if Finalize_TCB in 7staprop.adb called Specific.Set instead of pthread_setspecific.
I noticed that this PR has been outstanding for 3 weeks, unassigned, and even though Arnaud has commented, he wasn;t on the cc list. Hopefully we can the review of this wrapped up.
Patch is fine, OK for commit. Arno
I know that this patch was approved for the mainline, what about 3.4.0? I figure it is too late for 3.4.0 so I would say hold off until 3.4.1 to ask. But Joel please apply this patch to the mainline as it is already approved.
Subject: Re: *-rtems broken for gnat pinskia at gcc dot gnu dot org wrote: > ------- Additional Comments From pinskia at gcc dot gnu dot org 2004-04-07 02:59 ------- > I know that this patch was approved for the mainline, what about 3.4.0? I figure it is too > late for 3.4.0 so I would say hold off until 3.4.1 to ask. But Joel please apply this patch to > the mainline as it is already approved. > Arnaud only approved my 2 Ada PRs over the weekend. He made them conditional on successfully passing test run. It was only overnight that I got those test results. So I was going to commit them this morning. Mark has approved both PRs for 3.4.0 and I actually just updated my local 3.4 CVS tree to commit them. I will go into a holding pattern and let Mark comment on this and the gnatmake broken for cross one.
This patch is OK for 3.4.0.
Subject: Bug 14538 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: joel@gcc.gnu.org 2004-04-08 17:40:08 Modified files: gcc : ChangeLog gcc/ada : 5rosinte.adb 5rosinte.ads 5rtpopsp.adb init.c Log message: 2004-04-08 Joel Sherrill <joel@oarcorp.com> PR ada/14538 * ada/5rosinte.adb: Remove fake mprotect() body. * ada/5rosinte.ads: Add SA_SIGINFO. Make pthread_key_t a type which can be set since Finalize_TCB in 7staprop.adb does not go through the Set_Specific interface. * ada/5rtpopsp.adb: Rewrite to use new interface. * ada/init.c: Reorder so the simple single OS conditional __rtems__ is tested before more complex ones which mix UNIX and embedded systems in the conditional. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.386&r2=2.2326.2.387 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.adb.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3&r2=1.3.16.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.ads.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.5&r2=1.5.16.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rtpopsp.adb.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2&r2=1.2.32.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/init.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.22&r2=1.22.4.1
Created attachment 6054 [details] Same patch modified for the CVS Head Patch applied to 3.4 branch worked around a nit that Arnaud fixed on the head.
Subject: Bug 14538 CVSROOT: /cvs/gcc Module name: gcc Changes by: joel@gcc.gnu.org 2004-04-08 17:54:03 Modified files: gcc : ChangeLog gcc/ada : 5rosinte.adb 5rosinte.ads 5rtpopsp.adb init.c Log message: 2004-04-08 Joel Sherrill <joel@oarcorp.com> PR ada/14538 * ada/5rosinte.adb: Remove fake mprotect() body. * ada/5rosinte.ads: Add SA_SIGINFO. * ada/5rtpopsp.adb: Rewrite to use new interface. * ada/init.c: Reorder so the simple single OS conditional __rtems__ is tested before more complex ones which mix UNIX and embedded systems in the conditional. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3364&r2=2.3365 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.adb.diff?cvsroot=gcc&r1=1.3&r2=1.4 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.ads.diff?cvsroot=gcc&r1=1.5&r2=1.6 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rtpopsp.adb.diff?cvsroot=gcc&r1=1.2&r2=1.3 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/init.c.diff?cvsroot=gcc&r1=1.30&r2=1.31
Attached patches applied to both head and 3.4 branch. This is the ChangeLog for the HEAD. 2004-04-08 Joel Sherrill <joel@oarcorp.com> PR ada/14538 * ada/5rosinte.adb: Remove fake mprotect() body. * ada/5rosinte.ads: Add SA_SIGINFO. * ada/5rtpopsp.adb: Rewrite to use new interface. * ada/init.c: Reorder so the simple single OS conditional __rtems__ is tested before more complex ones which mix UNIX and embedded systems in the conditional.