From owner-freebsd-current@FreeBSD.ORG Sun Aug 2 16:55:38 2009 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 38F0B10656BC for ; Sun, 2 Aug 2009 16:55:38 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outV.internet-mail-service.net (outv.internet-mail-service.net [216.240.47.245]) by mx1.freebsd.org (Postfix) with ESMTP id 1B6558FC1C for ; Sun, 2 Aug 2009 16:55:37 +0000 (UTC) (envelope-from julian@elischer.org) Received: from idiom.com (mx0.idiom.com [216.240.32.160]) by out.internet-mail-service.net (Postfix) with ESMTP id ADDB6ADA67; Sun, 2 Aug 2009 09:55:37 -0700 (PDT) X-Client-Authorized: MaGic Cook1e X-Client-Authorized: MaGic Cook1e X-Client-Authorized: MaGic Cook1e Received: from julian-mac.elischer.org (home.elischer.org [216.240.48.38]) by idiom.com (Postfix) with ESMTP id CCDDE2D6017; Sun, 2 Aug 2009 09:55:36 -0700 (PDT) Message-ID: <4A75C50E.5020203@elischer.org> Date: Sun, 02 Aug 2009 09:55:42 -0700 From: Julian Elischer User-Agent: Thunderbird 2.0.0.22 (Macintosh/20090605) MIME-Version: 1.0 To: Alban Hertroys References: <4A756214.8010002@elischer.org> <4A756BA1.90002@gmx.de> <9A2BA686-016B-4B60-A247-7321C1E7F51A@solfertje.student.utwente.nl> In-Reply-To: <9A2BA686-016B-4B60-A247-7321C1E7F51A@solfertje.student.utwente.nl> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Christoph Mallon , FreeBSD Current Subject: Re: puzzling code in pcpu stuff X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Aug 2009 16:55:38 -0000 Alban Hertroys wrote: > On 2 Aug 2009, at 12:34, Christoph Mallon wrote: > >> Julian Elischer schrieb: >>> I simplified the output of the preprocessor for a PCPU_SET(xx, newval) >>> (to look at it). >>> I came down to: (after formatting) for i386.. >>> { >>> __typeof(((struct pcpu *)0)->pc_xx) __val; >>> struct __s >>> { >>> u_char __b[(((sizeof(__val)) < (4)) ? >>> (sizeof(__val)) : (4))]; >>> } __s; >>> __val = (newval); /* aligned */ >>> if (sizeof(__val) == 1 >>> || sizeof(__val) == 2 >>> || sizeof(__val) == 4) { >>> __s = *(struct __s *)(void *)&__val; >>> __asm volatile("mov %1,%%fs:%0" : "=m" >>> (*(struct __s *)(__builtin_offsetof( >>> struct pcpu, pc_xx))) : "r" (__s)); >>> } else { >>> *__extension__ ( >>> { >>> __typeof(__val) *__p; >>> __asm volatile("movl %%fs:%1,%0; >>> addl %2,%0" : "=r" (__p) : "m" >>> (*(struct pcpu *)(__builtin_offsetof(struct pcpu, pc_prvspace))), >>> "i" >>> (__builtin_offsetof(struct pcpu, pc_xx))); >>> __p; >>> }) = __val; >>> } >>> } >>> having had my brain explode on this several times, >>> I can't figure out exactly what teh clause after the else is doing. >>> anyone better at reading __asm better than me care to explain it in >>> simple words? >> >> First, ({}) is a statement expression - a GCC extension (not to be >> confused with expression statement, which is an expression followed by >> a semicolon). It wraps a compound statement, i.e. {}, and turns it >> into an expression. The value of the last statement is the value of >> the expression. In this case it's __p;. > > Speaking as an outsider I'd better be careful with any criticism, but > the first thing I noticed here was the lack of comments. From Julian's > question it seems obvious that this function could do with some. I > wonder what people would make of this in a couple of years when none of > the (then) active developers has any intimate knowledge of the workings > of functions like this one? there are no comments in this cut-n-paste because it is the output of the C preprocessor.. of course the source doesn't have many comments either.. (in i386/include/pcpu.h) > > I got from the posted code sample that __extension__ is probably a no-op > meant for documentation purposes only. I think it would help to add what > extension is being used though, maybe as a comment (but what would be > the use of defining __extension__ then) or as a parameter. That's up to > you though. > > Not being familiar with x86 assembly doesn't help me here of course, the > last time I touched assembly was on a Z80 a couple of decades back... > >> Let's have a closer look at the else clause: The asm reads the pointer >> to per-cpu information into __p and the statement expression returns >> it. This pointer gets dereferenced (mind the * before __extension__ - >> __extension__ does nothing, it just marks that the following is a GCC >> extension) and __val is assigned. > > > As I read it the value of __val is read into the memory address > calculated by the expression in the statement expression (__p > internally), am I right? > > Just chiming in with my opinion. I'm sure you'll do fine without it, but > still. > > Alban Hertroys > > -- > If you can't see the forest for the trees, > cut the trees and you'll see there is no forest. > > > !DSPAM:929,4a75a40b10131060118257! >