Date: Sat, 28 Jun 2008 13:44:17 +0200 From: Marius Strobl <marius@alchemy.franken.de> To: Christoph Mallon <christoph.mallon@gmx.de> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/sparc64/include in_cksum.h Message-ID: <20080628114417.GL1215@alchemy.franken.de> In-Reply-To: <48657058.6020102@gmx.de> References: <200806252105.m5PL5AUp064418@repoman.freebsd.org> <48654667.1040401@gmx.de> <20080627222404.GJ1215@alchemy.franken.de> <48657058.6020102@gmx.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jun 28, 2008 at 12:57:28AM +0200, Christoph Mallon wrote:
> Marius Strobl wrote:
> >I wasn't aware that the clobber list allows to explicitly specify
> >the condition codes, thanks for the hint. Though it unfortunately
> >took me longer than two days to verify it's effect on the generated
> >code; sparc64 could still have been one of the archs where "cc" has
> >no effect. Besides I don't think using "__volatile" for this is
> >that wrong, given that the sparc64 code generated by using "cc"
> >and "__volatile" is nearly identical and given that at least i386
> >relies on "__volatile" telling GCC that the inline assembler uses
> >the condition codes since quite some time. So the condition codes
> >are probably part of what GCC treats as "important side-effects".
>
> If this is true and GCC only handles the eflags on x86 correctly, when
> __volatile is used, but not if "cc" is marked as clobbered, then this is
> clearly a bug.
>
> >Regarding the MFC, they don't happen automatically and the change
> >was not wrong in general so there was no need to hurry :)
>
> I still think, using __volatile only works by accident. volatile for an
> assembler block mostly means "this asm statement has an effect, even
> though the register specification looks otherwise, so do not optimise
> this away (i.e. no CSE, do not remove if result is unused etc.).
>
>
> On a related note: Is inline assembler really necessary here? For
> example couldn't in_addword() be written as
> static __inline u_short
> in_addword(u_short const sum, u_short const b)
> {
> u_int const t = sum + b;
> return t + (t >> 16);
> } ?
> This should at least produce equally good code and because the compiler
> has more knowledge about it than an assembler block, it potentially
> leads to better code. I have no SPARC compiler at hand, though.
With GCC 4.2.1 at -O2 the code generated for the above C version
takes on more instruction than the inline assembler so if one
wants to go for micro-optimizing one should certainly prefer the
inline assembler version.
>
> In fact the in/out specification for this asm block looks rather bad:
> "=&r" (__ret), "=&r" (__tmp) : "r" (sum), "r" (b) : "cc");
> The "&"-modifiers (do not use the same registers as for any input
> operand value) force the compiler to use 4 (!) register in total for
> this asm block. It could be done with 2 registers if a proper in/out
> specification was used. At the very least the in/out specification can
> be improved, but I suspect using plain C is the better choice.
>
The "&"-modifiers are necessary as the inline assembler in
question consumes output operands before all input operands
are consumed. Omitting them caused GCC to generate broken
code in the past.
Marius
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080628114417.GL1215>