configure.in support for FreeBSD ia64/sparc64/powerpc

configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

The following change makes Emacs buildable on FreeBSD running on
sparc64, ia64 and powerpc too.  It has been tested for some time as a
local patch in the editors/emacs-devel port of FreeBSD, so it would be
nice if it became part of Emacs 22.  We won't have to maintain local
diffs for this part then:

%%%
diff -r e9c18eba38ea -r 355e6a7d786b configure.in
--- a/configure.in	Fri Oct 27 15:45:51 2006 +0000
+++ b/configure.in	Sat Oct 28 01:49:45 2006 +0300
@@ -237,6 +237,9 @@ case "${canonical}" in
     opsys=freebsd
     case "${canonical}" in
       alpha*-*-freebsd*)	machine=alpha ;;
+      ia64-*-freebsd*)		machine=ia64 ;;
+      sparc64-*-freebsd*)	machine=sparc ;;
+      powerpc-*-freebsd*)	machine=macppc ;;
       i[3456]86-*-freebsd*)	machine=intel386 ;;
       amd64-*-freebsd*|x86_64-*-freebsd*) machine=amdx86-64 ;;
     esac
diff -r e9c18eba38ea -r 355e6a7d786b src/alloc.c
--- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
+++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
@@ -4560,7 +4560,11 @@ mark_stack ()
      needed on ia64 too.  See mach_dep.c, where it also says inline
      assembler doesn't work with relevant proprietary compilers.  */
 #ifdef sparc
+#ifdef __sparc64__
+  asm ("flushw");
+#else
   asm ("ta 3");
+#endif
 #endif
 
   /* Save registers that we need to see on the stack.  We need to see
%%%

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Chong Yidong]

Giorgos Keramidas <keramida@ceid.upatras.gr> writes:

> The following change makes Emacs buildable on FreeBSD running on
> sparc64, ia64 and powerpc too.  It has been tested for some time as a
> local patch in the editors/emacs-devel port of FreeBSD, so it would be
> nice if it became part of Emacs 22.  We won't have to maintain local
> diffs for this part then:
>
> --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
> +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
> @@ -4560,7 +4560,11 @@ mark_stack ()
>       needed on ia64 too.  See mach_dep.c, where it also says inline
>       assembler doesn't work with relevant proprietary compilers.  */
>  #ifdef sparc
> +#ifdef __sparc64__
> +  asm ("flushw");
> +#else
>    asm ("ta 3");
> +#endif
>  #endif

Is this change correct for non-FreeBSD sparc64 systems too?  (I don't
know anything about this part of the code.)

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On 2006-11-03 22:28, Chong Yidong <cyd@stupidchicken.com> wrote:
>Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>> The following change makes Emacs buildable on FreeBSD running on
>> sparc64, ia64 and powerpc too.  It has been tested for some time as a
>> local patch in the editors/emacs-devel port of FreeBSD, so it would be
>> nice if it became part of Emacs 22.  We won't have to maintain local
>> diffs for this part then:
>>
>> --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
>> +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
>> @@ -4560,7 +4560,11 @@ mark_stack ()
>>       needed on ia64 too.  See mach_dep.c, where it also says inline
>>       assembler doesn't work with relevant proprietary compilers.  */
>>  #ifdef sparc
>> +#ifdef __sparc64__
>> +  asm ("flushw");
>> +#else
>>    asm ("ta 3");
>> +#endif
>>  #endif
> 
> Is this change correct for non-FreeBSD sparc64 systems too?  (I don't
> know anything about this part of the code.)

Sorry for taking so long to follow up about this.  I tried looking at
the sparc v9 specification for hints about why this change is needed on
64-bit SPARC platforms, but I couldn't come up with anything in a
reasonable amount of time.  I am not that proficient with SPARC64
assembly, but I will ask our FreeBSD/sparc64 people for details.

I'll follow up again when I get a reply from the people who know more
about sparc64 assembly :)

In the mean time, I can maintain this as a local FreeBSD patch, so it's
not really something we should push hard for getting into the release.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Mon, 6 Nov 2006, Giorgos Keramidas wrote:

>On 2006-11-03 22:28, Chong Yidong <cyd@stupidchicken.com> wrote:
>>Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>>> The following change makes Emacs buildable on FreeBSD running on
>>> sparc64, ia64 and powerpc too.  It has been tested for some time
>>> as a local patch in the editors/emacs-devel port of FreeBSD, so it
>>> would be nice if it became part of Emacs 22.  We won't have to
>>> maintain local diffs for this part then:
>>>
>>> --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
>>> +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
>>> @@ -4560,7 +4560,11 @@ mark_stack ()
>>>       needed on ia64 too.  See mach_dep.c, where it also says inline
>>>       assembler doesn't work with relevant proprietary compilers.
>>> */
>>>  #ifdef sparc
>>> +#ifdef __sparc64__
>>> +  asm ("flushw");
>>> +#else
>>>    asm ("ta 3");
>>> +#endif
>>>  #endif
>>
>> Is this change correct for non-FreeBSD sparc64 systems too?
>> (I don't know anything about this part of the code.)

> Sorry for taking so long to follow up about this.  I tried looking
> at the sparc v9 specification for hints about why this change is
> needed on 64-bit SPARC platforms, but I couldn't come up with
> anything in a reasonable amount of time.  I am not that proficient
> with SPARC64 assembly, but I will ask our FreeBSD/sparc64 people for
> details.

> I'll follow up again when I get a reply from the people who know more
> about sparc64 assembly :)

Following up to this message from two years ago. ;-)

Gentoo also maintains the above as local patch, both for FreeBSD and
GNU/Linux. Our Sparc team says that the patch is sane, and without it
Emacs cannot be built on Sparc/FreeBSD. See Gentoo Bug 159584 for
further details: <http://bugs.gentoo.org/159584>

Below is a slightly updated patch that will apply to the currect CVS
trunk. It would be nice if it could make it into Emacs 23.

Ulrich

--- emacs-orig/src/alloc.c
+++ emacs/src/alloc.c
@@ -4573,7 +4573,11 @@
      needed on ia64 too.  See mach_dep.c, where it also says inline
      assembler doesn't work with relevant proprietary compilers.  */
 #ifdef __sparc__
+#ifdef __sparc64__
+  asm ("flushw");
+#else
   asm ("ta 3");
+#endif
 #endif
 
   /* Save registers that we need to see on the stack.  We need to see

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Ulrich Mueller <ulm@gentoo.org> writes:

  > >>>>> On Mon, 6 Nov 2006, Giorgos Keramidas wrote:
  > 
  > >On 2006-11-03 22:28, Chong Yidong <cyd@stupidchicken.com> wrote:
  > >>Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
  > >>> The following change makes Emacs buildable on FreeBSD running on
  > >>> sparc64, ia64 and powerpc too.  It has been tested for some time
  > >>> as a local patch in the editors/emacs-devel port of FreeBSD, so it
  > >>> would be nice if it became part of Emacs 22.  We won't have to
  > >>> maintain local diffs for this part then:
  > >>>
  > >>> --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
  > >>> +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
  > >>> @@ -4560,7 +4560,11 @@ mark_stack ()
  > >>>       needed on ia64 too.  See mach_dep.c, where it also says inline
  > >>>       assembler doesn't work with relevant proprietary compilers.
  > >>> */
  > >>>  #ifdef sparc
  > >>> +#ifdef __sparc64__
  > >>> +  asm ("flushw");
  > >>> +#else
  > >>>    asm ("ta 3");
  > >>> +#endif
  > >>>  #endif
  > >>
  > >> Is this change correct for non-FreeBSD sparc64 systems too?
  > >> (I don't know anything about this part of the code.)
  > 
  > > Sorry for taking so long to follow up about this.  I tried looking
  > > at the sparc v9 specification for hints about why this change is
  > > needed on 64-bit SPARC platforms, but I couldn't come up with
  > > anything in a reasonable amount of time.  I am not that proficient
  > > with SPARC64 assembly, but I will ask our FreeBSD/sparc64 people for
  > > details.
  > 
  > > I'll follow up again when I get a reply from the people who know more
  > > about sparc64 assembly :)
  > 
  > Following up to this message from two years ago. ;-)
  > 
  > Gentoo also maintains the above as local patch, both for FreeBSD and
  > GNU/Linux. Our Sparc team says that the patch is sane, and without it
  > Emacs cannot be built on Sparc/FreeBSD. See Gentoo Bug 159584 for
  > further details: <http://bugs.gentoo.org/159584>
  > 
  > Below is a slightly updated patch that will apply to the currect CVS
  > trunk. It would be nice if it could make it into Emacs 23.
  > 
  > Ulrich
  > 
  > --- emacs-orig/src/alloc.c
  > +++ emacs/src/alloc.c
  > @@ -4573,7 +4573,11 @@
  >       needed on ia64 too.  See mach_dep.c, where it also says inline
  >       assembler doesn't work with relevant proprietary compilers.  */
  >  #ifdef __sparc__
  > +#ifdef __sparc64__
  > +  asm ("flushw");
  > +#else

Was this tested on GNU/Linux and Solaris (and all other sparc64
platforms)?  Otherwise better make that #ifdef depend on FreeBSD.


  >    asm ("ta 3");
  > +#endif
  >  #endif
  >  
  >    /* Save registers that we need to see on the stack.  We need to see

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Fri, 19 Dec 2008, Dan Nicolaescu wrote:

> Was this tested on GNU/Linux

Yes.

> and Solaris (and all other sparc64 platforms)?

No.

> Otherwise better make that #ifdef depend on FreeBSD.

I've nothing against that.

--- emacs-orig/src/alloc.c
+++ emacs/src/alloc.c
@@ -4573,7 +4573,11 @@
      needed on ia64 too.  See mach_dep.c, where it also says inline
      assembler doesn't work with relevant proprietary compilers.  */
 #ifdef __sparc__
+#if defined (__sparc64__) && defined(__FreeBSD__)
+  asm ("flushw");
+#else
   asm ("ta 3");
+#endif
 #endif
 
   /* Save registers that we need to see on the stack.  We need to see

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On Fri, 19 Dec 2008 19:10:26 +0100, Ulrich Mueller <ulm@gentoo.org> wrote:
>>>>>> On Fri, 19 Dec 2008, Dan Nicolaescu wrote:
>
>> Was this tested on GNU/Linux
>
> Yes.
>
>> and Solaris (and all other sparc64 platforms)?
>
> No.
>
>> Otherwise better make that #ifdef depend on FreeBSD.
>
> I've nothing against that.
>
> --- emacs-orig/src/alloc.c
> +++ emacs/src/alloc.c
> @@ -4573,7 +4573,11 @@
>       needed on ia64 too.  See mach_dep.c, where it also says inline
>       assembler doesn't work with relevant proprietary compilers.  */
>  #ifdef __sparc__
> +#if defined (__sparc64__) && defined(__FreeBSD__)
> +  asm ("flushw");
> +#else
>    asm ("ta 3");
> +#endif
>  #endif
>
>    /* Save registers that we need to see on the stack.  We need to see

No objection from me either.   Adding __FreeBSD__ seems fine.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Fri, 19 Dec 2008, Ulrich Mueller wrote:

>>>>> On Fri, 19 Dec 2008, Dan Nicolaescu wrote:
>> Was this tested on GNU/Linux

> Yes.

>> and Solaris (and all other sparc64 platforms)?

> No.

I've now asked our Prefix team to test this on Sparc/Solaris too, see
Gentoo bug 251667, <http://bugs.gentoo.org/251667>.

Any suggestion how to specifically test mark_stack and the garbage
collector?

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Sun, 21 Dec 2008, Ulrich Mueller wrote:

>>> Was this tested on GNU/Linux

>> Yes.

>>> and Solaris (and all other sparc64 platforms)?

>> No.

> I've now asked our Prefix team to test this on Sparc/Solaris too,
> see Gentoo bug 251667, <http://bugs.gentoo.org/251667>.

I don't know if it's sufficient as a test, but basic editing, as well
as byte-compiling a large package like CEDET seems to work flawlessly
on Solaris.

So maybe the original patch should be committed, without the
additional check for FreeBSD.

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Ulrich Mueller <ulm@gentoo.org> writes:

  > >>>>> On Sun, 21 Dec 2008, Ulrich Mueller wrote:
  > 
  > >>> Was this tested on GNU/Linux
  > 
  > >> Yes.
  > 
  > >>> and Solaris (and all other sparc64 platforms)?
  > 
  > >> No.
  > 
  > > I've now asked our Prefix team to test this on Sparc/Solaris too,
  > > see Gentoo bug 251667, <http://bugs.gentoo.org/251667>.
  > 
  > I don't know if it's sufficient as a test, but basic editing, as well
  > as byte-compiling a large package like CEDET seems to work flawlessly
  > on Solaris.
  > 
  > So maybe the original patch should be committed, without the
  > additional check for FreeBSD.

If this has not been tested on all versions of Solaris that we currently
support, that won't be a good idea at this point in the release
process.  We know for sure that the original code works.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Mon, 29 Dec 2008, Dan Nicolaescu wrote:

>> I don't know if it's sufficient as a test, but basic editing, as
>> well as byte-compiling a large package like CEDET seems to work
>> flawlessly on Solaris.
>> 
>> So maybe the original patch should be committed, without the
>> additional check for FreeBSD.

> If this has not been tested on all versions of Solaris that we
> currently support, that won't be a good idea at this point in the
> release process.

Oh well. "flushw" is a hardware instruction of Sparc-V9, and the code
was now tested on Linux, FreeBSD, and Solaris. Do you really expect
that there is a difference between different Solaris versions?

> We know for sure that the original code works.

It doesn't work on FreeBSD.

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Ulrich Mueller <ulm@gentoo.org> writes:

  > >>>>> On Mon, 29 Dec 2008, Dan Nicolaescu wrote:
  > 
  > >> I don't know if it's sufficient as a test, but basic editing, as
  > >> well as byte-compiling a large package like CEDET seems to work
  > >> flawlessly on Solaris.
  > >> 
  > >> So maybe the original patch should be committed, without the
  > >> additional check for FreeBSD.
  > 
  > > If this has not been tested on all versions of Solaris that we
  > > currently support, that won't be a good idea at this point in the
  > > release process.
  > 
  > Oh well. "flushw" is a hardware instruction of Sparc-V9, and the code
  > was now tested on Linux, FreeBSD, and Solaris. Do you really expect
  > that there is a difference between different Solaris versions?

As long as it has not been tested, it's not prudent to change.

  > > We know for sure that the original code works.
  > 
  > It doesn't work on FreeBSD.

You were proposing to change the Solaris code.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Mon, 29 Dec 2008, Dan Nicolaescu wrote:

>> > If this has not been tested on all versions of Solaris that we
>> > currently support, that won't be a good idea at this point in the
>> > release process.
>> 
>> Oh well. "flushw" is a hardware instruction of Sparc-V9, and the code
>> was now tested on Linux, FreeBSD, and Solaris. Do you really expect
>> that there is a difference between different Solaris versions?

> As long as it has not been tested, it's not prudent to change.

Why did you then ask for a test on Solaris at all, if you are not
interested in the answer? It means some effort to do such a test,
which could have been saved.

And how should it get testing on all Solaris versions, unless the
change is committed? I'd guess that no single person has all Solaris
versions at her disposal.

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]

On Mon, 29 Dec 2008 12:39:36 -0800 (PST), Dan Nicolaescu <dann@ics.uci.edu> wrote:
>Ulrich Mueller <ulm@gentoo.org> writes:
>>> We know for sure that the original code works.
>>
>> It doesn't work on FreeBSD.
>
> You were proposing to change the Solaris code.

Err, I think there's a misunderstanding here.  The src/alloc.c change
is related to sparc64 not Solaris (not _only_ Solaris, that is):

    --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
    +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
    @@ -4560,7 +4560,11 @@ mark_stack ()
          needed on ia64 too.  See mach_dep.c, where it also says inline
          assembler doesn't work with relevant proprietary compilers.
    */
     #ifdef sparc
    +#ifdef __sparc64__
    +  asm ("flushw");
    +#else
       asm ("ta 3");
    +#endif
     #endif

Now, I don't think anybody would argue that there are probably far more
Solaris installations of sparc64 than FreeBSD ones.

If the change is considered too risky for the 23.X release then I can
maintain it as a local FreeBSD patch for 23.X too.  We have been running
with this patch in the editors/emacs (based on 22.X) and the CVS-based
editors/emacs-devel port for several months now, so it's not a big
problem to keep it as a local patch in the FreeBSD Ports.

Having said that, if we *do* commit it to the CVS repository, we will
have one less patch to maintain locally.  That's usually a good idea :)

- Giorgos


[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Giorgos Keramidas <keramida@freebsd.org> writes:

  > On Mon, 29 Dec 2008 12:39:36 -0800 (PST), Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > >Ulrich Mueller <ulm@gentoo.org> writes:
  > >>> We know for sure that the original code works.
  > >>
  > >> It doesn't work on FreeBSD.
  > >
  > > You were proposing to change the Solaris code.
  > 
  > Err, I think there's a misunderstanding here.  The src/alloc.c change
  > is related to sparc64 not Solaris (not _only_ Solaris, that is):

  >     --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
  >     +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
  >     @@ -4560,7 +4560,11 @@ mark_stack ()
  >           needed on ia64 too.  See mach_dep.c, where it also says inline
  >           assembler doesn't work with relevant proprietary compilers.
  >     */
  >      #ifdef sparc
  >     +#ifdef __sparc64__
  >     +  asm ("flushw");
  >     +#else
  >        asm ("ta 3");
  >     +#endif
  >      #endif
  > 
  > Now, I don't think anybody would argue that there are probably far more
  > Solaris installations of sparc64 than FreeBSD ones.

Exactly, that code has been running for all other systems just fine for
many years.  Why the insistence to change without proper testing? 
Why not just make the conditional
 defined (__sparc64__) && defined (__FreeBSD__) ?

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On Mon, 29 Dec 2008 14:06:04 -0800 (PST), Dan Nicolaescu <dann@ics.uci.edu> wrote:
> Giorgos Keramidas <keramida@freebsd.org> writes:
>
>   > On Mon, 29 Dec 2008 12:39:36 -0800 (PST), Dan Nicolaescu <dann@ics.uci.edu> wrote:
>   > >Ulrich Mueller <ulm@gentoo.org> writes:
>   > >>> We know for sure that the original code works.
>   > >>
>   > >> It doesn't work on FreeBSD.
>   > >
>   > > You were proposing to change the Solaris code.
>   >
>   > Err, I think there's a misunderstanding here.  The src/alloc.c change
>   > is related to sparc64 not Solaris (not _only_ Solaris, that is):
>
>   >     --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
>   >     +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
>   >     @@ -4560,7 +4560,11 @@ mark_stack ()
>   >           needed on ia64 too.  See mach_dep.c, where it also says inline
>   >           assembler doesn't work with relevant proprietary compilers.
>   >     */
>   >      #ifdef sparc
>   >     +#ifdef __sparc64__
>   >     +  asm ("flushw");
>   >     +#else
>   >        asm ("ta 3");
>   >     +#endif
>   >      #endif
>   >
>   > Now, I don't think anybody would argue that there are probably far more
>   > Solaris installations of sparc64 than FreeBSD ones.
>
> Exactly, that code has been running for all other systems just fine for
> many years.  Why the insistence to change without proper testing?
> Why not just make the conditional
>  defined (__sparc64__) && defined (__FreeBSD__) ?

My impression from the original Gentoo bug <http://bugs.gentoo.org/159584>
is that this affects Linux too, so making the code depend on __FreeBSD__
doesn't solve Ulrich's original problem.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Tue, 30 Dec 2008, Giorgos Keramidas wrote:

> On Mon, 29 Dec 2008 14:06:04 -0800 (PST), Dan Nicolaescu <dann@ics.uci.edu> wrote:
>> > Err, I think there's a misunderstanding here.  The src/alloc.c change
>> > is related to sparc64 not Solaris (not _only_ Solaris, that is):
>> 
>> >     --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
>> >     +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
>> >     @@ -4560,7 +4560,11 @@ mark_stack ()
>> >           needed on ia64 too.  See mach_dep.c, where it also says inline
>> >           assembler doesn't work with relevant proprietary compilers.
>> >     */
>> >      #ifdef sparc
>> >     +#ifdef __sparc64__
>> >     +  asm ("flushw");
>> >     +#else
>> >        asm ("ta 3");
>> >     +#endif
>> >      #endif
>> >
>> > Now, I don't think anybody would argue that there are probably far more
>> > Solaris installations of sparc64 than FreeBSD ones.
>> 
>> Exactly, that code has been running for all other systems just fine for
>> many years.  Why the insistence to change without proper testing?
>> Why not just make the conditional
>> defined (__sparc64__) && defined (__FreeBSD__) ?

> My impression from the original Gentoo bug <http://bugs.gentoo.org/159584>
> is that this affects Linux too,

Sparc/Linux is fine, with or without the patch. Looks like both
"flushw" and "ta 3" work there.

> so making the code depend on __FreeBSD__ doesn't solve Ulrich's
> original problem.

It does, in fact. But testing for the OS still looks wrong to me,
since it is a question of a machine instruction implemented for
SPARC-V9, but not for earlier CPUs.

Could the patch be committed with the "&& defined (__FreeBSD__)", and
we keep in mind to remove this piece after the 23.1 release?

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Ulrich Mueller <ulm@gentoo.org> writes:

  > >>>>> On Tue, 30 Dec 2008, Giorgos Keramidas wrote:
  > 
  > > On Mon, 29 Dec 2008 14:06:04 -0800 (PST), Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > >> > Err, I think there's a misunderstanding here.  The src/alloc.c change
  > >> > is related to sparc64 not Solaris (not _only_ Solaris, that is):
  > >> 
  > >> >     --- a/src/alloc.c	Fri Oct 27 15:45:51 2006 +0000
  > >> >     +++ b/src/alloc.c	Sat Oct 28 01:49:45 2006 +0300
  > >> >     @@ -4560,7 +4560,11 @@ mark_stack ()
  > >> >           needed on ia64 too.  See mach_dep.c, where it also says inline
  > >> >           assembler doesn't work with relevant proprietary compilers.
  > >> >     */
  > >> >      #ifdef sparc
  > >> >     +#ifdef __sparc64__
  > >> >     +  asm ("flushw");
  > >> >     +#else
  > >> >        asm ("ta 3");
  > >> >     +#endif
  > >> >      #endif
  > >> >
  > >> > Now, I don't think anybody would argue that there are probably far more
  > >> > Solaris installations of sparc64 than FreeBSD ones.
  > >> 
  > >> Exactly, that code has been running for all other systems just fine for
  > >> many years.  Why the insistence to change without proper testing?
  > >> Why not just make the conditional
  > >> defined (__sparc64__) && defined (__FreeBSD__) ?
  > 
  > > My impression from the original Gentoo bug <http://bugs.gentoo.org/159584>
  > > is that this affects Linux too,
  > 
  > Sparc/Linux is fine, with or without the patch. Looks like both
  > "flushw" and "ta 3" work there.
  > 
  > > so making the code depend on __FreeBSD__ doesn't solve Ulrich's
  > > original problem.
  > 
  > It does, in fact. But testing for the OS still looks wrong to me,
  > since it is a question of a machine instruction implemented for
  > SPARC-V9, but not for earlier CPUs.

Or it's a question of FreeBSD not providing an implementation for the
"ta 3" trap?

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Mon, 29 Dec 2008, Dan Nicolaescu wrote:

> Ulrich Mueller <ulm@gentoo.org> writes:
>> >>>>> On Tue, 30 Dec 2008, Giorgos Keramidas wrote:
>> > so making the code depend on __FreeBSD__ doesn't solve Ulrich's
>> > original problem.
>> 
>> It does, in fact. But testing for the OS still looks wrong to me,
>> since it is a question of a machine instruction implemented for
>> SPARC-V9, but not for earlier CPUs.

> Or it's a question of FreeBSD not providing an implementation for
> the "ta 3" trap?

Probably _because_ it is available in hardware and therefore not
needed? Or, look what other programs like GCC do (in sparc.md):

   ;; Special trap insn to flush register windows.
   (define_insn "flush_register_windows"
     [(unspec_volatile [(const_int 0)] UNSPECV_FLUSHW)]
     ""
     { return TARGET_V9 ? "flushw" : "ta\t3"; }
     [(set_attr "type" "flushw")])

No test for the OS here. Similar code can be found in the Boehm-GC or
in libjava.

But this discussion is moot. Could you please commit the patch with
the test in the form "defined (__sparc64__) && defined (__FreeBSD__)"
at least, so that FreeBSD and Gentoo can get rid of their local patch?

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On Tue, 30 Dec 2008 13:44:14 +0100, Ulrich Mueller <ulm@gentoo.org> wrote:
>>>>>> On Mon, 29 Dec 2008, Dan Nicolaescu wrote:
>> Or it's a question of FreeBSD not providing an implementation for
>> the "ta 3" trap?
>
> Probably _because_ it is available in hardware and therefore not
> needed? Or, look what other programs like GCC do (in sparc.md):
>
>    ;; Special trap insn to flush register windows.
>    (define_insn "flush_register_windows"
>      [(unspec_volatile [(const_int 0)] UNSPECV_FLUSHW)]
>      ""
>      { return TARGET_V9 ? "flushw" : "ta\t3"; }
>      [(set_attr "type" "flushw")])
>
> No test for the OS here. Similar code can be found in the Boehm-GC or
> in libjava.
>
> But this discussion is moot. Could you please commit the patch with
> the test in the form "defined (__sparc64__) && defined (__FreeBSD__)"
> at least, so that FreeBSD and Gentoo can get rid of their local patch?

That would be fine by me.  One less local patch to maintain :)

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Fri, 19 Dec 2008, Giorgos Keramidas wrote:

>>> Was this tested on GNU/Linux
>> 
>> Yes.
>> 
>>> and Solaris (and all other sparc64 platforms)?
>> 
>> No.
>> 
>>> Otherwise better make that #ifdef depend on FreeBSD.
>> 
>> I've nothing against that.
>> 
>> --- emacs-orig/src/alloc.c
>> +++ emacs/src/alloc.c
>> @@ -4573,7 +4573,11 @@
>>       needed on ia64 too.  See mach_dep.c, where it also says inline
>>       assembler doesn't work with relevant proprietary compilers.  */
>>  #ifdef __sparc__
>> +#if defined (__sparc64__) && defined(__FreeBSD__)
>> +  asm ("flushw");
>> +#else
>>    asm ("ta 3");
>> +#endif
>>  #endif
>>  
>>    /* Save registers that we need to see on the stack.  We need to see

> No objection from me either.   Adding __FreeBSD__ seems fine.

So, another month has passed ... any reason why this patch cannot be
applied? Otherwise, it would be nice if someone could commit it before
pretest starts.

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On Thu, 22 Jan 2009 12:10:13 +0100, Ulrich Mueller <ulm@gentoo.org> wrote:
>>> I've nothing against that.
>>>
>>> --- emacs-orig/src/alloc.c
>>> +++ emacs/src/alloc.c
>>> @@ -4573,7 +4573,11 @@
>>>       needed on ia64 too.  See mach_dep.c, where it also says inline
>>>       assembler doesn't work with relevant proprietary compilers.  */
>>>  #ifdef __sparc__
>>> +#if defined (__sparc64__) && defined(__FreeBSD__)
>>> +  asm ("flushw");
>>> +#else
>>>    asm ("ta 3");
>>> +#endif
>>>  #endif
>>>
>>>    /* Save registers that we need to see on the stack.  We need to see
>
>> No objection from me either.   Adding __FreeBSD__ seems fine.
>
> So, another month has passed ... any reason why this patch cannot be
> applied? Otherwise, it would be nice if someone could commit it before
> pretest starts.

It would be nice if this can go in before the pretest.  When the pretest
is out, I will update the editors/emacs-devel port to use the tarball of
the pretest instead of the current old CVS snapshot.  If the patch goes
into CVS we can finally remove it from the FreeBSD-specific patches :-)

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Chong Yidong]

Giorgos Keramidas <keramida@freebsd.org> writes:

>> So, another month has passed ... any reason why this patch cannot be
>> applied? Otherwise, it would be nice if someone could commit it before
>> pretest starts.
>
> It would be nice if this can go in before the pretest.  When the pretest
> is out, I will update the editors/emacs-devel port to use the tarball of
> the pretest instead of the current old CVS snapshot.  If the patch goes
> into CVS we can finally remove it from the FreeBSD-specific patches :-)

Checked in, thanks.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On Thu, 22 Jan 2009 23:39:23 -0500, Chong Yidong <cyd@stupidchicken.com> wrote:
>Giorgos Keramidas <keramida@freebsd.org> writes:
>>> So, another month has passed ... any reason why this patch cannot be
>>> applied? Otherwise, it would be nice if someone could commit it before
>>> pretest starts.
>>
>> It would be nice if this can go in before the pretest.  When the pretest
>> is out, I will update the editors/emacs-devel port to use the tarball of
>> the pretest instead of the current old CVS snapshot.  If the patch goes
>> into CVS we can finally remove it from the FreeBSD-specific patches :-)
>
> Checked in, thanks.

Thank you :)

I've updated my local snapshot and will rebuild shortly.

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Thu, 22 Jan 2009, Chong Yidong wrote:

>> It would be nice if this can go in before the pretest. When the
>> pretest is out, I will update the editors/emacs-devel port to use
>> the tarball of the pretest instead of the current old CVS snapshot.
>> If the patch goes into CVS we can finally remove it from the
>> FreeBSD-specific patches :-)

> Checked in, thanks.

As I see, with a conditional for __sparc64__ only (which is the right
thing in my opinion).

However, one week later there is the following commit:
<http://cvs.savannah.gnu.org/viewvc/emacs/src/alloc.c?root=emacs&r1=1.446&r2=1.447>

So was there really a problem with this on non-BSD platforms?
(I'm just curious.)

Ulrich

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Chong Yidong]

Ulrich Mueller <ulm@gentoo.org> writes:

> As I see, with a conditional for __sparc64__ only (which is the right
> thing in my opinion).
>
> However, one week later there is the following commit:
> <http://cvs.savannah.gnu.org/viewvc/emacs/src/alloc.c?root=emacs&r1=1.446&r2=1.447>
>
> So was there really a problem with this on non-BSD platforms?
> (I'm just curious.)

Dan, did you see any problem with this patch on non-BSD platforms?

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Giorgos Keramidas]

On Thu, 05 Feb 2009 08:55:14 -0500, Chong Yidong <cyd@stupidchicken.com> wrote:
>Ulrich Mueller <ulm@gentoo.org> writes:
>> As I see, with a conditional for __sparc64__ only (which is the right
>> thing in my opinion).
>>
>> However, one week later there is the following commit:
>> <http://cvs.savannah.gnu.org/viewvc/emacs/src/alloc.c?root=emacs&r1=1.446&r2=1.447>
>>
>> So was there really a problem with this on non-BSD platforms?
>> (I'm just curious.)
>
> Dan, did you see any problem with this patch on non-BSD platforms?

FWIW, I've only tried the original patch on FreeBSD, but Ulrich and
other Gentoo people have contacted me about this.  I don't know how well
the patch works on GNU/Linux though :/

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Ulrich Mueller <ulm@gentoo.org> writes:
  > 
  > > As I see, with a conditional for __sparc64__ only (which is the right
  > > thing in my opinion).
  > >
  > > However, one week later there is the following commit:
  > > <http://cvs.savannah.gnu.org/viewvc/emacs/src/alloc.c?root=emacs&r1=1.446&r2=1.447>
  > >
  > > So was there really a problem with this on non-BSD platforms?
  > > (I'm just curious.)
  > 
  > Dan, did you see any problem with this patch on non-BSD platforms?

Didn't we have a long discussion about this and the last version of the
patch from Ulrich is cited here:

http://permalink.gmane.org/gmane.emacs.devel/108082

anything happened to change the results of that discussion, do we want
to start it again?

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> Didn't we have a long discussion about this and the last version of the
> patch from Ulrich is cited here:
>
> http://permalink.gmane.org/gmane.emacs.devel/108082
>
> anything happened to change the results of that discussion, do we want
> to start it again?

According to the archives, you did not respond to Ulrich's message
arguing that using flushw is not OS-specific, and that similar code is
found in other programs:

http://lists.gnu.org/archive/html/emacs-devel/2008-12/msg01055.html

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > > Didn't we have a long discussion about this and the last version of the
  > > patch from Ulrich is cited here:
  > >
  > > http://permalink.gmane.org/gmane.emacs.devel/108082
  > >
  > > anything happened to change the results of that discussion, do we want
  > > to start it again?
  > 
  > According to the archives, you did not respond to Ulrich's message
  > arguing that using flushw is not OS-specific, and that similar code is
  > found in other programs:
  > 
  > http://lists.gnu.org/archive/html/emacs-devel/2008-12/msg01055.html

From that message: 
   But this discussion is moot. Could you please commit the patch with
   the test in the form "defined (__sparc64__) && defined (__FreeBSD__)"
   at least, so that FreeBSD and Gentoo can get rid of their local patch?

Re: configure.in support for FreeBSD ia64/sparc64/powerpc

[Ulrich Mueller]

>>>>> On Thu, 5 Feb 2009, Dan Nicolaescu wrote:

>> According to the archives, you did not respond to Ulrich's message
>> arguing that using flushw is not OS-specific, and that similar code is
>> found in other programs:
>> 
>> http://lists.gnu.org/archive/html/emacs-devel/2008-12/msg01055.html

>> From that message: 
>    But this discussion is moot. Could you please commit the patch with
>    the test in the form "defined (__sparc64__) && defined (__FreeBSD__)"
>    at least, so that FreeBSD and Gentoo can get rid of their local patch?

Right, I wrote that. But it is taken a bit out of context. From the
rest of the message it should be clear that I was much in favour of
the solution with the __sparc64__ test only. (Which the Gentoo Sparc
team had tested under FreeBSD, under GNU/Linux, and under one Solaris
variant.)

Whereas the patch with the additional test for FreeBSD was a second
choice only, but of course preferred against not applying the patch at
all.

Ulrich