Re: Want comments regarding patch

From: Jan Engelhardt
Date: Thu Dec 28 2006 - 18:53:42 EST



On Dec 28 2006 19:53, Arjan van de Ven wrote:
>On Thu, 2006-12-28 at 19:41 +0100, Daniel MarjamÃki wrote:
>> Hello all!
>>
>> I sent a patch with this content:
>>
>> - for (i = 0; i < MAX_PIRQS; i++)
>> - pirq_entries[i] = -1;
>> + memset(pirq_entries, -1, sizeof(pirq_entries));
>>
>> I'd like to know if you have any comments to this change. It was
>> of course my intention to make the code shorter, simpler and
>> faster.
>
>personally I don't like the new code; memset only takes a byte as
>argument and while it probably is the same, that is now implicit
>behavior and no longer explicit. A reasonably good compiler will
>notice it's the same and generate the best code anyway, so I would
>really really suggest to go for the best readable code, which imo is
>the original code.

Then GCC is not a "reasonably good compiler". Considering

#define MAX 6400
struct foo {
int line[MAX];
};
void bar(struct foo *a) {
int i;
for(i = 0; i < MAX; ++i)
a->line[i] = -1;
}
void baz(struct foo *a) {
__builtin_memset(a->line, -1, sizeof(a->line));
}

`gcc -O3 -c test.c` will generate a classic loop rather than a repz
movsd for bar(). baz() will get a call to an extern memset(),
probably because gcc could not figure out how to make a repz for it
and hence thought it was better to use an external hand-crafted
memset.


-`J'
--