svn commit: r332092 - in head/sys: amd64/amd64 sys x86/x86

[Bruce Evans]

On Fri, 6 Apr 2018, [UTF-8] Roger Pau Monné wrote:

> Log:
>  remove GiB/MiB macros from param.h
>
>  And instead define them in the files where they are used.
>
>  Requested by: bde

Thanks, but these files have a negative need for the macros.

> Modified: head/sys/amd64/amd64/mp_machdep.c
> ==============================================================================
> --- head/sys/amd64/amd64/mp_machdep.c	Fri Apr  6 09:25:08 2018	(r332091)
> +++ head/sys/amd64/amd64/mp_machdep.c	Fri Apr  6 11:20:06 2018	(r332092)
> @@ -83,6 +83,8 @@ __FBSDID("$FreeBSD$");
> #define BIOS_RESET		(0x0f)
> #define BIOS_WARM		(0x0a)
>
> +#define GiB(v)			(v ## ULL << 30)
> +

In this file, the macro is used only once.  It takes about 4 times as
much code to define and use the macro once as to write
(vm_paddr_t)4 << 30.  Much more than 4 times longer to read, since some
searching is needed to find the macro and some decoding is needed to
understand it.  More to see that the wrong type returned by the macro
is not a problem.  The value can be written more consisely as 4L << 30
after doing a similar type analysis.  1G is normally written as
1024 * 1024 * 1024 since this is a bit clearer than 1 << 30.  This depends
n a similar type analysis -- the multipliction and the shift don't overflow
32-bit ints.  But care must be taken with multiplication by another 4 or
even 2.

> Modified: head/sys/x86/x86/mp_x86.c
> ==============================================================================
> --- head/sys/x86/x86/mp_x86.c	Fri Apr  6 09:25:08 2018	(r332091)
> +++ head/sys/x86/x86/mp_x86.c	Fri Apr  6 11:20:06 2018	(r332092)
> @@ -160,6 +160,8 @@ struct cache_info {
>
> unsigned int boot_address;
>
> +#define MiB(v)	(v ## ULL << 20)
> +

In this file, the macro is used twice with v = 1.  Defining and using it
takes only about twice as much code and time to read as (vm_paddr_t)1 << 20.
Here it is more important to use vm_paddr_t since this code is shared by
amd64, i386 and i386-PAE so the size of vm_paddr_t is variable.  However,
since 1MB is far below INT_MAX, it doesn't take much type analysis to see
than the shorter 1 << 20 is safe.  2 copies of the clearer 1024 * 1024
is also shorter than the macro and 2 calls to it.

The macro name doesn't match the comment.  The comment still says 1MB.
The fix is not to break the comment.

Later in the file, basemem is converted from K to bytes by multiplying
by 1024.  Now 1024 is shorter and clearer than 1 << 10 or 0x400 or a
macro with many undocmented details.  The type analysis to show that
multiplying by 1024 doesn't overflow is slightly more complicated since
basemem is a variable.  It is only easy to see that this doesn't
overflow because basemem is an old real-mode value.  640K was large
enough for anyone, and basemem in bytes is less than that.  640K was
20 times INT_MAX, but is now 1/3276 of INT_MAX.

Bruce