Re: CVS commit: src/sys/compat/sys
On Fri, Jun 15, 2018 at 17:11:49 +0300, Valery Ushakov wrote: > It seems that with restrict we can just memset the whole struct and > the compiler will elide the memset completely when there's no pad to > scrub. Alas, with the real code the compiler cannot elide the memset. :( -uwe
Re: CVS commit: src/sys/compat/sys
On Fri, Jun 15, 2018 at 19:55:28 +0700, Robert Elz wrote:
> Date:Fri, 15 Jun 2018 12:41:56 +0300
> From:Valery Ushakov
> Message-ID: <20180615094156.gd3...@pony.stderr.spb.ru>
>
> | Re memset - I now wonder if
> | compiler is even allowed to be smart here b/c strict aliasing
>
> Another way would be
>
> static const struct timespec50 zts = { 0 };
> and
> *ts50 = zts;
>
> which is unlikely to have that problem, and which the compiler might
> be smart enough to optimise into a single assignment where there is
> padding, and perhaps even nothing where there is none.
The compiler does field-by-field assignment in that case and uses
"narrow" movl that doesn't scrub the pad.
With memset you seem to also need to add "restrict" so that the
compiler can elide the dead moves.
The example I'm playing with is:
#include
struct s { int i; long l; };
struct s64 { long i; long l; };
//#define restrict
void foo(struct s * restrict l, const struct s64 * restrict r)
{
#if 0
static const struct s s0 = { 0, 0 };
*l = s0;
#else
memset(l, 0, sizeof(*l));
#endif
l->i = r->i;
l->l = r->l;
}
> I'm also not really a fan of the #if that tests the number of bits in
> int and long (by testing their max values) - that is assuming that
> when long is wider than int, it also has wider alignment, which is
> not necessarily so - a system with 4 byte ints and 8 byte long,
> could have long with 4 byte alignment (and that struct be 12 bytes
> long, alogned 4). The code as it is isn't wrong, ijust the attempt
> to avoid the memset() when it isn't needed might fail, and do the
> memset() when there is nothing to clear - and if the compiler doesn't
> or even can't, optimise it away then it is wasteful.
Yes, the #if test feels icky...
It seems that with restrict we can just memset the whole struct and
the compiler will elide the memset completely when there's no pad to
scrub.
-uwe
Re: CVS commit: src/sys/compat/sys
Date:Fri, 15 Jun 2018 12:41:56 +0300
From:Valery Ushakov
Message-ID: <20180615094156.gd3...@pony.stderr.spb.ru>
| Re memset - I now wonder if
| compiler is even allowed to be smart here b/c strict aliasing
Another way would be
static const struct timespec50 zts = { 0 };
and
*ts50 = zts;
which is unlikely to have that problem, and which the compiler might
be smart enough to optimise into a single assignment where there is
padding, and perhaps even nothing where there is none.
I'm also not really a fan of the #if that tests the number of bits in
int and long (by testing their max values) - that is assuming that
when long is wider than int, it also has wider alignment, which is
not necessarily so - a system with 4 byte ints and 8 byte long,
could have long with 4 byte alignment (and that struct be 12 bytes
long, alogned 4). The code as it is isn't wrong, ijust the attempt
to avoid the memset() when it isn't needed might fail, and do the
memset() when there is nothing to clear - and if the compiler doesn't
or even can't, optimise it away then it is wasteful.
kre
Re: CVS commit: src/sys/compat/sys
On Fri, Jun 15, 2018 at 18:05:29 +1000, matthew green wrote: > "Robert Elz" writes: > > Module Name:src > > Committed By: kre > > Date: Fri Jun 15 07:46:59 UTC 2018 > > > > Modified Files: > > src/sys/compat/sys: time_types.h > > > > Log Message: > > If we are going to use offsetof() we'd need to include to > > get it defined. Rather than deal with potential namespace issues > > with that, just clear the entire struct, rather than attempting to > > stop after the potential padding field. If the compiler is good enough > > it should make no difference (there are just 3 fields, 2 named ones > > are assigned to, immediately after the memset() - the compiler can > > detect that, and not bother assigning (via memset()) to the unmamed > > 3rd padding field). If the compiler is not smart enough to deal > > with this, then I doubt writing 8 more zero bytes will make enough > > difference to matter. > > the compiler isn't smart apparently, and the previous change > should have fixed the problem. did we really need this too? Oh, thanks for fixing it, my bad. Re memset - I now wonder if compiler is even allowed to be smart here b/c strict aliasing (but I'm not re^n-reading that part of the standard just to satisfy my idle curiosity :) -uwe
re: CVS commit: src/sys/compat/sys
"Robert Elz" writes: > Module Name: src > Committed By: kre > Date: Fri Jun 15 07:46:59 UTC 2018 > > Modified Files: > src/sys/compat/sys: time_types.h > > Log Message: > If we are going to use offsetof() we'd need to include to > get it defined. Rather than deal with potential namespace issues > with that, just clear the entire struct, rather than attempting to > stop after the potential padding field. If the compiler is good enough > it should make no difference (there are just 3 fields, 2 named ones > are assigned to, immediately after the memset() - the compiler can > detect that, and not bother assigning (via memset()) to the unmamed > 3rd padding field). If the compiler is not smart enough to deal > with this, then I doubt writing 8 more zero bytes will make enough > difference to matter. the compiler isn't smart apparently, and the previous change should have fixed the problem. did we really need this too? .mrg.
