Date: Sun, 2 Aug 2009 16:34:49 +0200 From: Alban Hertroys <dalroi@solfertje.student.utwente.nl> To: Christoph Mallon <christoph.mallon@gmx.de> Cc: Julian Elischer <julian@elischer.org>, FreeBSD Current <current@freebsd.org> Subject: Re: puzzling code in pcpu stuff Message-ID: <9A2BA686-016B-4B60-A247-7321C1E7F51A@solfertje.student.utwente.nl> In-Reply-To: <4A756BA1.90002@gmx.de> References: <4A756214.8010002@elischer.org> <4A756BA1.90002@gmx.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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? 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:930,4a75a40e10131514347140!
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9A2BA686-016B-4B60-A247-7321C1E7F51A>