Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 15 Jan 2012 08:43:17 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Andreas Tobler <andreast@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au>
Subject:   Re: svn commit: r229693 - in head/lib/libc: powerpc powerpc64
Message-ID:  <20120115080256.E843@besplex.bde.org>
In-Reply-To: <4F07529B.2060608@FreeBSD.org>
References:  <201201060921.q069Lfi8081051@svn.freebsd.org> <20120106225728.G9027@besplex.bde.org> <4F07529B.2060608@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 6 Jan 2012, Andreas Tobler wrote:

Sorry for the delay.

> Hi Bruce,
>
> thank you for the feedback. I wasn't aware about the fine details.
>
> I try to understand and implement what you suggest. It may take several 
> iterations until I match everything.

OK.

>> The easiest way to fix this is to remove WEAK_ALIAS() and add an asm
>> API WEAK_REFERENCE().  Unfortunately, ALIAS is a better name than
>> REFERENCE.  __weak_reference() is not so easy to change since it is
>> used extensively
>
> So, I started with a WEAK_REFERENCE macro in sys/powerpc/asm.h
> It is like the WEAK_ALIAS but with reversed arguments:
>
> +#define WEAK_REFERENCE(sym, alias)				\
> +	.weak alias;						\
> +	alias = sym
> +
>
> Here I do not have a preference for the "=" implementation, is it "=" or is 
> it .set .....

I prefer .equ, since it is what is ised for weak_reference() and it has
minor syntactial advantages.  Anyway, This is much easier to change than
uses of the macro.

>
> If we find a final version I'll be able to delete the WEAK_ALIAS.

Yes, delete it before it is used much.  The only thing to worry about
is use of it in ports.  I checked a 2005 version of NetBSD.  It has
separate sys/cdefs files for aout and elf, and spells things completely
backwards relative to FreeBSD there:
- __asm is spelled __asm__
- the order is reversed in all macros: (alias,sym).  The misformatting
   of no space after the comma in this to support aout is preserved in
   cdefs_elf.h.
- #alias and #sym are not used rawly, even for elf.  They are surrounded
   by _C_LABEL() or _C_LABEL_STRING().  For elf, this usually makes no
   difference, but there is an option to add an underscore for elf too.
   OTOH, K&R is not supported, so there are no additional files, ifdefs
   or macro obfuscations to support it (FreeBSD still has ifdefs for it,
   but doesn't support aout).
- it has __strong_alias() there, but no __strong_reference()
- it has __weak_alias() there, but no __weak_reference()
- it has __weak_extern() (expands to a plain .weak)
- it has __warn_references().  This is the only one that needs much magic
   (.stabs declarations).
- it "=" instead of ".equ" or ".set"
- it uses single-line asms instead of multi-line ones, except for the ones
   with .stabs.  This gives additional ugliness from string concatenation
   and explicit newlines or semicolons to separate the statements (newlines
   are used, which makes the output more readable, but tabs are not used).
   One feature of K&R is that this feature is unavailable, so FreeBSD
   couldn't use it in the K&R part of the ifdef and it may as well not use
   it in the STDC part.
In summary, unportability to NetBSD couldn't be implemented much better
if done intentionally.  To preserve this intentionally, we shouldn't
define __weak_alias(), but should keep __weak_reference() with its args
reversed.

>> Similarly for STRONG_ALIAS() and __strong_reference(), except
>> STRONG_REFERENCE() doesn't exist for most arches and __strong_reference()
>> didn't exist until this week, so neither is used extensively.

I didn't check what NetBSD does for asm macros.

> ...
> If I get the above right, the snippet from above should look like this, 
> right?
>
> @@ -51,20 +51,17 @@
> 	ld	%r0,16(%r1);					\
> 	mtlr	%r0;						\
> 	blr;							\
> -ENTRY(__CONCAT(__sys_,x));					\
> -	.weak	CNAME(x);					\
> -	.set	CNAME(x),CNAME(__CONCAT(__sys_,x));		\
> -	.weak	CNAME(__CONCAT(_,x));				\
> -	.set	CNAME(__CONCAT(_,x)),CNAME(__CONCAT(__sys_,x));	\
> +ENTRY(__CONCAT(__sys_, x));					\
> +	WEAK_REFERENCE(__CONCAT(__sys_, x), x);	     		\
> +	WEAK_REFERENCE(__CONCAT(__sys_, x), __CONCAT(_, x));	\
> 	_SYSCALL(x);						\
> 	bso	2b

Yes, assuming the args are in the right order.  I would also change
`x' to a meaningful name.  At least on i386, <machine/asm.h> uses
x in most places, but csym and asmsym in 2 places, while
<machine/asmacros.h> uses meaningul parameter names in most places.

> If we want to stay with WEAK_ALIAS I can at least fix the style bugs.

It is the arg order that I care about most, and with __weak_alias() or
WEAK_ALIAS() we should remain compatible with the opposite order in
NetBSD.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120115080256.E843>