From owner-svn-src-all@FreeBSD.ORG Thu Jun 20 22:05:52 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id ECAF071E; Thu, 20 Jun 2013 22:05:52 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 8099E199E; Thu, 20 Jun 2013 22:05:52 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 5BAE84216E9; Fri, 21 Jun 2013 07:43:35 +1000 (EST) Date: Fri, 21 Jun 2013 07:43:32 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov Subject: Re: svn commit: r252032 - head/sys/amd64/include In-Reply-To: <201306201430.r5KEU4G5049115@svn.freebsd.org> Message-ID: <20130621065839.J916@besplex.bde.org> References: <201306201430.r5KEU4G5049115@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=Q6eKePKa c=1 sm=1 a=0l9hOOMEwYoA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=gvJhbHXk4isA:10 a=qNzoWfBIm-v1GVZZY0YA:9 a=CjuIK1q_8ugA:10 a=lBjYRrsFGgrJhvrT:21 a=yrjkO6AgVjvk9TFD:21 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 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: Thu, 20 Jun 2013 22:05:53 -0000 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