Re: [PATCH] fix sparse warning from include/linux/mmzone.h

From: Linus Torvalds
Date: Fri Feb 08 2008 - 17:44:37 EST




On Fri, 8 Feb 2008, Linus Torvalds wrote:
>
> It would probably make more sense to just write it as something like
>
> struct zone *base = zone->zone_pgdat->node_zones;
>
> if (zone == base + ZONE_HIGHMEM ||
> (zone == base + ZONE_MOVABLE && zone_movable_is_highmem());

Side note: while the above is more readable, gcc still isn't smart enough
to notice that the two compares could be done more efficiently as a
subtraction. But using an actual pointer subtraction does involve that
nasty divide (well, it's nasty only for certain sizes of "struct zone"),
so writing it as such is not very nice either, even if it's the most
obvious way from a source code standpoint.

Here's an example:

/* 12-byte (non-power-of-two) example struct */
struct example {
int a, b, c;
};

#define ptrcmp1(ptr, base, index) \
((ptr) == (base) + (index))
#define ptrcmp2(ptr, base, index) \
((ptr) - (base) == (index))
#define ptrcmp3(ptr, base, index) \
((char *)(ptr) - (char *)(base) == (index)*sizeof(*ptr))

#define test(cmp) \
if (cmp(ptr, base, 1) || cmp(ptr, base, 3)) \
printf("success\n")

int test1(struct example *ptr, struct example *base) { test(ptrcmp1); }
int test2(struct example *ptr, struct example *base) { test(ptrcmp2); }
int test3(struct example *ptr, struct example *base) { test(ptrcmp3); }

and the results for me are:

test1:
leaq 12(%rsi), %rax
cmpq %rdi, %rax
je .L15
leaq 36(%rsi), %rax
cmpq %rdi, %rax
je .L15

test2:
subq %rsi, %rdi
leaq -12(%rdi), %rax
cmpq $11, %rax
jbe .L11
leaq -36(%rdi), %rax
cmpq $11, %rax
jbe .L11

test3:
subq %rsi, %rdi
cmpq $12, %rdi
je .L4
cmpq $36, %rdi
je .L4

ie the only way to get the *nice* code generation is that ugly third
alternative.

YMMV depending on compiler, of course.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/