From owner-svn-src-all@FreeBSD.ORG Fri Jan 6 19:59:28 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5CCC9106564A; Fri, 6 Jan 2012 19:59:28 +0000 (UTC) (envelope-from andreast@FreeBSD.org) Received: from smtp.fgznet.ch (mail.fgznet.ch [81.92.96.47]) by mx1.freebsd.org (Postfix) with ESMTP id 02DA98FC08; Fri, 6 Jan 2012 19:59:27 +0000 (UTC) Received: from deuterium.andreas.nets (dhclient-91-190-14-19.flashcable.ch [91.190.14.19]) by smtp.fgznet.ch (8.13.8/8.13.8/Submit_SMTPAUTH) with ESMTP id q06Jt8KJ028608; Fri, 6 Jan 2012 20:55:10 +0100 (CET) (envelope-from andreast@FreeBSD.org) Message-ID: <4F07529B.2060608@FreeBSD.org> Date: Fri, 06 Jan 2012 20:59:23 +0100 From: Andreas Tobler User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:8.0) Gecko/20111105 Thunderbird/8.0 MIME-Version: 1.0 To: Bruce Evans References: <201201060921.q069Lfi8081051@svn.freebsd.org> <20120106225728.G9027@besplex.bde.org> In-Reply-To: <20120106225728.G9027@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.64 on 81.92.96.47 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r229693 - in head/lib/libc: powerpc powerpc64 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Jan 2012 19:59:28 -0000 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. On 06.01.12 14:12, Bruce Evans wrote: > On Fri, 6 Jan 2012, Andreas Tobler wrote: > >> Log: >> Use the macro WEAK_ALIAS. Tested on 32 and 64-bit. > > This API should be fixed before using it extensively. It has the args > reversed relative to the C API __weak_reference(). Also, its name is > different, and the spelling of "=" in its implementation is different. > Perhaps the arg order makes sense for both, since for WEAK_ALIAS() the > alias is the first arg, while for __weak_reference() the symbol being > referred to to create the alias is the first arg. But this is still > confusing. > > 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 ..... If we find a final version I'll be able to delete the WEAK_ALIAS. > 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. > > More details on current existence and use of these: > > In the kernel, WEAK_ALIAS is not defined for amd64 or sparc64, and is > never used. > > In libc, WEAK_ALIAS was only used for arm, ia64 and mips. Now it is used > for some i386 string functions. > > In the kernel, STRONG_ALIAS was only defined for mips, and was never used. > Now it is also defined for i386, and is never used. > > In libc, STRONG_ALIAS was not used. It was used for a few days in some > i386 string functions. This was a bug. Now WEAK_ALIAS is used instead, > and STRONG_ALIAS is not used again. It is another bug that these > "optimized" i386 string functions (strchr/index and strrchr/rindex) even > exist. amd64 doesn't have them. The MI versions are not very optimal, > but neither are the i386 ones. > >> Modified: head/lib/libc/powerpc/SYS.h >> ============================================================================== >> --- head/lib/libc/powerpc/SYS.h Fri Jan 6 09:17:34 2012 (r229692) >> +++ head/lib/libc/powerpc/SYS.h Fri Jan 6 09:21:40 2012 (r229693) >> @@ -44,10 +44,8 @@ >> .align 2; \ >> 2: b PIC_PLT(CNAME(HIDENAME(cerror))); \ >> 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)); \ >> + WEAK_ALIAS(x,__CONCAT(__sys_,x)); \ >> + WEAK_ALIAS(__CONCAT(_,x),__CONCAT(__sys_,x)); \ >> _SYSCALL(x); \ >> bso 2b >> > > The style bugs in this should be fixed someday. This is still messed up > to support K&R. With ancient cpp's, you had to write __CONCAT(x,y) instead > of __CONCAT(x, y) to avoid getting a space between x and y. This was fixed > in Standard C 22 years ago, but all SYS.h files in libc except ia64's one > still use the ugly __CONCAT(x,y) in most places. ia64 hard-codes > __CONCAT(x, y) as x ## y instead. > > The missing space after the comma for the WEAK_ALIAS() parameters is even > less necessary. For ancient cpp's, it allowed WEAK_ALIAS to format the > asm directives without a space. With STDC cpp's, the formatting is > controlled by the macro, but it is still hard to produce nice formatting > because cpp may change whitespace. 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 > > __weak_reference() also difference from WEAK_ALIAS() in the spelling > of "=". It uses ".equ". ".set" in the all the SYS.h's except ia64 and > mips (*) seems to be yet another spelling. > > (*) ia64 avoids the hard coded directive using WEAK_ALIAS(). mips > hard-codes "=" instead of ".set". > > Some other gratuitous differences in the SYS.h's: > - arm and mips use _C_LABEL() instead of CNAME() > - ia64 doesn't use either _C_LABEL() or CNAME() > - arm, ia64 and mips don't use HIDENAME() for cerror: > - arm uses CERROR and hard-codes this as _C_LABEL(cerror) > - ia64 hard-codes it as .cerror > - mips hard-codes it as __cerror. > - sparc64 doesn't use ENTRY() or END() and has to repeat things like .type > and .size found in those when it defines _SYSENTRY() and _SYSEND(). > It doesn't use CNAME() in these definitions. Underscores in these > names are bogus, since these names are even less public than ENTRY() > and END() which don't have them. > > Some of these differences may be due to FreeBSD's cleaning up or down > of the decomposition of SYS.h and only for older arches. > Both of these files are MD so you can hard-code things in either, but > I think it is best put as much of the details as possible in the lowest > level, which is. Using WEAK_ALIAS() from there is a > step in this direction. If we want to stay with WEAK_ALIAS I can at least fix the style bugs. Thanks, Andreas