[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [microblaze-uclinux] [patch] errno corruption (fwd) (fwd)





Hi Guys,

I ran this past Paul Dale,  our resident compiler guru,  and here are
his comments,

Cheers,
Davidm


----- Forwarded message from Pauli <pauli@bne.snapgear.com> -----

To: David McCullough <davidm@snapgear.com>
Subject: Re: [microblaze-uclinux] [patch] errno corruption (fwd) 
From: Pauli <pauli@bne.snapgear.com>
X-Spam-Level: 
X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on 
	dorfl.internal.moreton.com.au
X-Spam-Status: No, hits=-4.9 required=5.0 tests=AWL,BAYES_00 autolearn=ham 
	version=2.63


Some comments....


Original definition:

> > > > #define SYSCALL_CLOBBERS	"r3", "r4", "r11", "r12", "r17"

[...]

> #define _syscall2(type, name, type1, arg1, type2, arg2)                 \
> type name (type1 arg1, type2 arg2)                                      \
> {                                                                       \
>         long __ret;                                                     \
>         __asm__ __volatile__ ("addk	r5, r0, %2	\n\t"           \
>                               "addk	r6, r0, %3	\n\t"           \
>                               "bralid	r17, 0x8	\n\t"           \
>                               "addik	r12, r0, %1	\n\t"           \
>                               "addk	%0, r3, r0	\n\t"           \
>                               : "=r" (__ret)                            \
>                               : "i" (__NR_##name),                      \
>                                 "r" ((long)arg1),                       \
>                                 "r" ((long)arg2)                        \
>                               : SYSCALL_CLOBBERS);                      \
>         __syscall_return (type, __ret);                                 \
> }

The clobbers don't sync up with the code.  r5, r6, r12 & r17 look like being
clobbered.  r0 and r3 don't look properly initialised.  If r3 is a syscall
return, it should be a clobber too.  r0 == 0 perhaps?  Sorry, if this is
obvious, I'm not in the slightest bit up to speed on the microblaze :-)

Also, if you've got a condition codes register and you might alter it, you
must include "cc" as a clobber.  Even if you don't have such a register (e.g.
the alpha) it is harmless to include "cc" anyway.  Likewise, if memory is
modified, include "memory" as a clobber (probably not needed for syscalls
though).


You might also be able to loosen the constraints on the the asm args if the
microblaze has other addressing modes.  I.e. there isn't any need to put the
args and return in registers if the CPU supports rich enough third operand
addressing modes.  "g" is a good mode (general memory/immediate/register) if
possible.  I imagine "ri" would be safe.

A lot depends on the CPU.  I used "i" for the syscall number on m68k since
that pretty much guaranteed a space saving and lessened register usage.  I
used "ai" for most of the incoming arguments (all bar the first loaded which
is "g").  I vaguely remember some problems leaving them all as "g" but can't
remember what exactly.  If you know your calling convention and addressing
modes, you should be able to make a sensible decision as to what to do here.


All this said and done, very little code outside of libc uses this stuff.
Yes, one could create a select(2) function for which the last argument is
always NULL and thus trivially improve performance.  Luckily, almost nobody
does.

For libc, you can play additional tricks to improve syscall dispatch.  Here
you know you're in a C function call world and you know what you can and
cannot do and how to call the system call.  For example, in the m68k world,
all syscalls use a short branch after loading the syscall number into the
first register to common epilogue code for example.  I knew they'd be lumped
together in libc and thus this is safe and saved an amount of space
(especially before I figured out shared libraries).
See new-wave/lib/libc/sysdeps/m68k in our distribution for details of this
(and I just noticed a speed for space optimisation I'd previously missed!).
The user visible syscalls come from the include file:

	new-wave/lib/libc/include/m68k/syscall.h

which is significantly different to the libc internal version even though it
is performing the same task.


Finally, getting asm's working properly is a bit of an art form.  Just about
every architecture I've examined does it wrong and is open to compiler
optimisation problems.  Fortunately, these don't seem to bite currently
but when they do ....

And if anybody needs further assistance, I'll try to help.
(I'm not on the list, but Dave will probably pass messages along)

Regards,

Pauli

-- 
Dr Paul Dale, Software Grunt, SnapGear - A CyberGuard Company
Leaders in embedded Linux security   http://www.SnapGear.com/

----- End forwarded message -----

-- 
David McCullough, davidm@snapgear.com  Ph:+61 7 34352815 http://www.SnapGear.com
Custom Embedded Solutions + Security   Fx:+61 7 38913630 http://www.uCdot.org
___________________________
microblaze-uclinux mailing list
microblaze-uclinux@itee.uq.edu.au
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/