8 messages in org.freebsd.freebsd-currentRe: puzzling code in pcpu stuff| From | Sent On | Attachments |
|---|---|---|
| Julian Elischer | Aug 2, 2009 2:52 am | |
| Julian Elischer | Aug 2, 2009 3:25 am | |
| Christoph Mallon | Aug 2, 2009 3:33 am | |
| Alban Hertroys | Aug 2, 2009 7:34 am | |
| Christoph Mallon | Aug 2, 2009 8:09 am | |
| Alban Hertroys | Aug 2, 2009 8:37 am | |
| Julian Elischer | Aug 2, 2009 9:55 am | |
| Alban Hertroys | Aug 2, 2009 10:20 am |
| Subject: | Re: puzzling code in pcpu stuff | |
|---|---|---|
| From: | Alban Hertroys (dal...@solfertje.student.utwente.nl) | |
| Date: | Aug 2, 2009 7:34:26 am | |
| List: | org.freebsd.freebsd-current | |
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!
_______________________________________________ free...@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "free...@freebsd.org"