Re: Want comments regarding patch

[Jan Engelhardt <jengelh@linux01.gwdg.de>]

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'
--