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<machine/asm.h>  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<machine.asm.h>.  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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to