On Thu, Nov 22, 2018 at 10:28 AM Mateusz Guzik <[email protected]> wrote:
> On 11/20/18, Warner Losh <[email protected]> wrote: > > On Tue, Nov 20, 2018 at 8:28 AM Mateusz Guzik <[email protected]> wrote: > > > >> On 11/20/18, Konstantin Belousov <[email protected]> wrote: > >> >> +#if defined(__mips__) || defined(__powerpc__) > >> > Please note what I asked about this #ifdefs in the review. mips > >> > and powerpc machine/atomic.h should define some symbol like > >> > __ATOMIC_NO_64_OPS and this case should be handled in less > >> > arch-explicit > >> > manner. > >> > > >> > >> Right, should have mentioned that in the commit message. > >> > >> Anyhow, mips has some degree of 64 bit ops even for 32 bits so > >> this becomes more iffy. In particular it does have atomic_add_64. > >> I don't have a good way to test mips atomics and since non-atomic > >> version for powerpc was needed anyway I decided not to try to > >> add one. > >> > > > > I thought the 64-bit stuff was not present in true 32-bit mips at all. > You > > need the lld/scd instructions to do 64-bit atomics which are only > available > > in a 64-bit environment (eg n32 or n64 execution). They throw a fatal > > machine error if you execute them in o32 land. > > > > And I think the proper ifdef for this is defined(mips) && > > !defined(__mips_n64) && !defined(__mips_n32) or something horrible like > > that to not pessimize 64-bit executions. > > > > And powerpc will require something of similar sort (as reported by Mark > MIllard). It gets quite ugly indeed. > > I don't have strong opinion how to express the ifdefs, I think this is > least > bad if sticking to a mi header: > > diff --git a/sys/sys/systm.h b/sys/sys/systm.h > index a1b98c5660c..fab94ee7979 100644 > --- a/sys/sys/systm.h > +++ b/sys/sys/systm.h > @@ -523,7 +523,11 @@ int alloc_unr_specific(struct unrhdr *uh, u_int item); > int alloc_unrl(struct unrhdr *uh); > void free_unr(struct unrhdr *uh, u_int item); > > -#if defined(__mips__) || defined(__powerpc__) > +#if defined(mips) && !defined(__mips_n64) && !defined(__mips_n32) > +#define UNR64_LOCKED > +#endif > + > +#if defined(__powerpc__) && !defined(__powerpc64__) > #define UNR64_LOCKED > #endif > We should move the defining of this to machine/atomic.h as suggested elsewhere. Once it's more than one phrase long, it makes no sense to pollute the MI header with MD stuff like this. Warner _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "[email protected]"
