Re: svn commit: r253636 - head/sys/vm

2013-07-26 Thread David Chisnall
On 25 Jul 2013, at 16:10, m...@freebsd.org wrote:

 Isn't that a compiler bug?  memset(p, 0, n) is the same as bzero(p, n).  Why 
 would the compiler warn on one and not the other?

They are different.  memcpy is defined by the C standard.  bzero is defined by 
POSIX.  When you are compiling C code, the compiler is free to assume behaviour 
of any C standard functions but not of POSIX functions because it does not know 
that you are compiling for a POSIX target (possibly it should do this if the 
relevant POSIX macros are set).  

As Bruce says, however, the C standard excludes memset() and memcpy() in 
freestanding environments (which is a shame, because a lot of optimisations 
depend on their existence, and something I had thought was fixed in C11), so 
this is not relevant in the kernel.

 Does clang have a similar bias for memcpy versus bcopy?

Almost certainly.

David



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r253636 - head/sys/vm

2013-07-25 Thread Bruce Evans

On Thu, 25 Jul 2013, Tim Kientzle wrote:


Log:
 Clear entire map structure including locks so that the
 locks don't accidentally appear to have been already
 initialized.

 In particular, this fixes a consistent kernel crash on
 armv6 with:
   panic: lock vm map (user) 0xc09cc050 already initialized
 that appeared with r251709.

 PR: arm/180820

Modified:
 head/sys/vm/vm_map.c

Modified: head/sys/vm/vm_map.c
==
--- head/sys/vm/vm_map.cThu Jul 25 03:44:12 2013(r253635)
+++ head/sys/vm/vm_map.cThu Jul 25 03:48:37 2013(r253636)
@@ -239,8 +239,7 @@ vm_map_zinit(void *mem, int size, int fl
vm_map_t map;

map = (vm_map_t)mem;
-   map-nentries = 0;
-   map-size = 0;
+   memset(map, 0, sizeof(*map));
mtx_init(map-system_mtx, vm map (system), NULL, MTX_DEF | 
MTX_DUPOK);
sx_init(map-lock, vm map (user));
return (0);


memset() to value 0 is spelled bzero() in the kernel.

Before this commit, vm used normal style in 21 of 28 instances.

The style regression is even smaller in kern -- 18 of 169 instances
(3 in comments, and 1 of these in a loop doing a memset to a nonzero
value, which was required before about FreeBSD-4 since memset() didn't
exist in the kernel.  memset() is still a slightly pessimized wrapper
round bzero() when the value is nonzero, and a more pessimized loop
otherwise).

In FreeBSD-4, this style bug was not present in vm (0 of 16 instances)
and was smaller in kern (6 of 121 instances (3 in comments, and 1 to
#define a buggy memset() in terms of bzero() for the other 2)).

Bruce
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r253636 - head/sys/vm

2013-07-25 Thread Hans Petter Selasky

On 07/25/13 09:26, Bruce Evans wrote:

On Thu, 25 Jul 2013, Tim Kientzle wrote:


Log:
 Clear entire map structure including locks so that the
 locks don't accidentally appear to have been already
 initialized.

 In particular, this fixes a consistent kernel crash on
 armv6 with:
   panic: lock vm map (user) 0xc09cc050 already initialized
 that appeared with r251709.

 PR: arm/180820

Modified:
 head/sys/vm/vm_map.c

Modified: head/sys/vm/vm_map.c
==

--- head/sys/vm/vm_map.cThu Jul 25 03:44:12 2013(r253635)
+++ head/sys/vm/vm_map.cThu Jul 25 03:48:37 2013(r253636)
@@ -239,8 +239,7 @@ vm_map_zinit(void *mem, int size, int fl
vm_map_t map;

map = (vm_map_t)mem;
-map-nentries = 0;
-map-size = 0;
+memset(map, 0, sizeof(*map));
mtx_init(map-system_mtx, vm map (system), NULL, MTX_DEF |
MTX_DUPOK);
sx_init(map-lock, vm map (user));
return (0);


memset() to value 0 is spelled bzero() in the kernel.



Hi,

Is this the memset vs bzero or batman vs superman discussion again ;-)

The structure looks like some size, so bzero() might run faster than 
memset() depending on the compiler settings. Should be profiled before 
changed!


--HPS

___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r253636 - head/sys/vm

2013-07-25 Thread David Chisnall
On 25 Jul 2013, at 09:11, Hans Petter Selasky h...@bitfrost.no wrote:

 The structure looks like some size, so bzero() might run faster than memset() 
 depending on the compiler settings. Should be profiled before changed!

They will generate identical code for small structures with known sizes.  Both 
clang and gcc have a simplify libcalls pass that recognises both functions and 
will elide the call in preference to a small set of inline stores.

However(), memset is to be preferred in this idiom because the compiler 
provides better diagnostics in the case of error:

bzero.c:9:22: warning: 'memset' call operates on objects of type 'struct foo'
  while the size is based on a different type 'struct foo *'
  [-Wsizeof-pointer-memaccess]
memset(f, 0, sizeof(f));
   ~^
bzero.c:9:22: note: did you mean to dereference the argument to 'sizeof' (and
  multiply it by the number of elements)?
memset(f, 0, sizeof(f));
^

The same line with bzero(f, sizeof(f)) generates no error.

David



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r253636 - head/sys/vm

2013-07-25 Thread mdf
On Thu, Jul 25, 2013 at 4:43 AM, David Chisnall thera...@freebsd.orgwrote:

 However(), memset is to be preferred in this idiom because the compiler
 provides better diagnostics in the case of error:

 bzero.c:9:22: warning: 'memset' call operates on objects of type 'struct
 foo'
   while the size is based on a different type 'struct foo *'
   [-Wsizeof-pointer-memaccess]
 memset(f, 0, sizeof(f));
~^
 bzero.c:9:22: note: did you mean to dereference the argument to 'sizeof'
 (and
   multiply it by the number of elements)?
 memset(f, 0, sizeof(f));
 ^

 The same line with bzero(f, sizeof(f)) generates no error.


Isn't that a compiler bug?  memset(p, 0, n) is the same as bzero(p, n).
 Why would the compiler warn on one and not the other?

Does clang have a similar bias for memcpy versus bcopy?

Thanks,
matthew
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r253636 - head/sys/vm

2013-07-25 Thread Bruce Evans

On Thu, 25 Jul 2013, David Chisnall wrote:


On 25 Jul 2013, at 09:11, Hans Petter Selasky h...@bitfrost.no wrote:


The structure looks like some size, so bzero() might run faster than memset() 
depending on the compiler settings. Should be profiled before changed!


They will generate identical code for small structures with known sizes.  Both 
clang and gcc have a simplify libcalls pass that recognises both functions and 
will elide the call in preference to a small set of inline stores.


In the kernel, compilers are prevented from inlining memset() and many
other things by -ffreestanding in CFLAGS.  This rarely matters, and no
one except me cares.  In my version, memset() doesn't exist, but
bzero() has the micro-optimization of turning itself into __builtin_memset()
if the size is small (= 32).  My memcpy() has the micro-optimization
of turning itself into __builtin_memcpy() unconditionally.  __builtin_memcpy()
then turns itself back into extern memcpy() according to the inverse of
a similar size check.  I think extern memcmp() still doesn't exist in
the FreeBSD kernel, so simply using __builtin_memset() would give linkage
errors when __builtin_memcmp() turns itself back into memcmp().

Determining whether the size is small is difficult.  It is very
CPU-dependent, and also depends on how efficient the extern function
is.  Compilers once used a very large limits for inlining, but changed
to fairly small limits when they realized that they didn't understand
memory.  Extern functions are hard to optimize, since the correct
optimization depends on the CPU including its cache organization.
FreeBSD's x86 bcopy() and bzero() are still optimized for generic
CPUs.  Generic means approximately the original i386, but since these
are important operations, all CPUs run the old i386 code for them not
to badly (perhaps only twice as slow as possible), with newer Intel
systems doing it better than most.

Use of memcpy() in the kernel is the result of previous
micro-optimizations.  It was supposed to be used only for small
fixed-size copies.  This could have been done better by making bcopy()
inline and calling __builtin_memcpy() in this case.  The extern
memcpy() should never have been used, but was needed for cases where
__builtin_memcpy() turns itself into memcpy(), which happened mainly
when compiling with -O0.  Other uses of memcpy() were style bugs.  No
one cared when this optimization was turned into a style bug in all
cases by -ffreestanding.


However(), memset is to be preferred in this idiom because the compiler 
provides better diagnostics in the case of error:

bzero.c:9:22: warning: 'memset' call operates on objects of type 'struct foo'
 while the size is based on a different type 'struct foo *'
 [-Wsizeof-pointer-memaccess]
   memset(f, 0, sizeof(f));
  ~^
bzero.c:9:22: note: did you mean to dereference the argument to 'sizeof' (and
 multiply it by the number of elements)?
   memset(f, 0, sizeof(f));
   ^

The same line with bzero(f, sizeof(f)) generates no error.


This is compiler bug with -ffreestanding.  Then memset() is not special.
clang allows me to declare my own memset but still prints this warning
if the API is not too different: no warning for void memset(int *, int),
but warning for int memset(int *, int, int).  The warning seems to
be based on the function name, since it is not claimed that the function
is standard (even without -ffreestanding).

While testing this, I mistyped an f as foo, where foo is a function
name.  clang doesn't warn about this.  clang warned when memset was
my home made int memset(int *, int, int), because the function pointer
isn't compatible with int *.  But it also isn't compatible with void *.
I think casting NULL to a function pointer must work even if NULL is
spelled with a void *, but that is the only case where a void * object
pointer can safely be converted to a function pointer.

Bruce
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org