puzzling code in pcpu stuff
Julian Elischer
julian at elischer.org
Sun Aug 2 16:55:38 UTC 2009
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!
>
More information about the freebsd-current
mailing list