Date: Fri, 21 Jun 2013 07:43:32 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kib@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r252032 - head/sys/amd64/include Message-ID: <20130621065839.J916@besplex.bde.org> In-Reply-To: <201306201430.r5KEU4G5049115@svn.freebsd.org> References: <201306201430.r5KEU4G5049115@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 20 Jun 2013, Konstantin Belousov wrote:
> Log:
> Allow immediate operand.
> ..
> Modified: head/sys/amd64/include/counter.h
> ==============================================================================
> --- head/sys/amd64/include/counter.h Thu Jun 20 14:20:03 2013 (r252031)
> +++ head/sys/amd64/include/counter.h Thu Jun 20 14:30:04 2013 (r252032)
> @@ -44,7 +44,7 @@ counter_u64_add(counter_u64_t c, int64_t
>
> __asm __volatile("addq\t%1,%%gs:(%0)"
> :
> - : "r" ((char *)c - (char *)&__pcpu[0]), "r" (inc)
> + : "r" ((char *)c - (char *)&__pcpu[0]), "ri" (inc)
> : "memory", "cc");
> }
I don't like the quality of these asms. pcpu.h on both amd64 and i386
uses a hackish output operand and no memory clobber, and no cc clobber.
The output operand is hackish because the variable is in a different
address space. But since the variable is in a different address space,
the memory clobber is very unlikely to protect anything -- it could
only protect accesses to the variable without using %gs, and it is
practically impossible to have such accesses in scope, since we are
using %gs because it is the only way that we have in scope to access
the variable.
The i386 version is much larger and uglier. It has mounds of style bugs.
See {amd64.i386}/include/atomic.h for normal style of multi-line asms.
I don't like that either. I like a style with backslash-newlines, hard
tabs and soft newlines ('... \n\$') used in i386/isa/npx.c. The
backslash-newlines are and hard tabs less ugly and more readable than
double quotes and soft tabs ('"...\n\t$"'). Escapes in string constants
are unfortunately necessary since gcc broke its extension of allowing
hard newlines in string constants.
The old good way, using the gcc extension, allowed copying extern asm:
directly into inline asm (code for a dummy loop):
asm("
movl $100,%%ecx
1:
decl %%ecx
jne 1b
");
Without the extension, this has to be uglified and requires a lot of editing
to convert. Using backslash-newline and the same indentation for the source
and generated asm file, but with trailing tabs to line up:
asm(" \n\
movl $100,%%ecx \n\
1: \n\
decl %%ecx \n\
jne 1b \n\
"); /* clobbers omitted */
The style in machine/atomic.h adds lots of quotes and breaks the formatting
of the generated asm file by omitting the newlines, and has to add another
ugliness to recover from that -- now it needs semicolons instead of newlines:
asm(
" movl $100,%%ecx; "
"1: " /* should use newline/semi */
" decl %%ecx ; "
" jne 1b "
); /* might be on previous line */
This requires much more editing. Finally, the style in
i386/include/counter.h changes the semicolons to soft newlines to
improve the output and removes most of the formatting in the source but
adds soft tabs to give the indentation in the output in another way:
asm(
"movl $100,%%ecx\n\t"
"1:\n\t"
"decl %%ecx\n\t"
"jne 1b\n\t"
);
The i386 version of the counter asm doesn't support the immediate
constraint for technical reasons. 64 bit counters are too large and
slow to use on i386, especially when they are implemented as they are
without races.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130621065839.J916>