Date: Fri, 24 Feb 2017 14:53:35 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r314087 - head/sys/x86/x86 Message-ID: <20170224125335.GV2092@kib.kiev.ua> In-Reply-To: <20170223053954.J1044@besplex.bde.org> References: <201702220707.v1M7764i020598@repo.freebsd.org> <20170223053954.J1044@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Feb 23, 2017 at 06:33:43AM +1100, Bruce Evans wrote:
> On Wed, 22 Feb 2017, Konstantin Belousov wrote:
>
> > Log:
> > More fixes for regression in r313898 on i386.
> > Use long long constants where needed.
>
> The long long abomination is never needed, and is always a style bug.
I never saw any explanation behind this claim. Esp. the first part
of it, WRT 'never needed'.
> I don't like using explicit long constants either. Here the number of bits
> in the register is fixed by the hardware at 64. The number of bits in a
> long on amd64 and a long on i386 is only fixed by ABI because the ABI is
> broken for historical reasons.
I really cannot make any sense of this statement.
> Only very MD code can safely assume the
> size of long and long long. This code was MD enough before it was merged,
> but now it shouldn't use long since that varies between amd64 and i386,
> and it shouldn't use long long since that is a style bug.
Well, I do not see anything wrong with long long, at least until
explained.
Anyway, below is the patch to use uint64_t cast in important place,
and removal of LL suffix in unimportant expression.
diff --git a/sys/x86/x86/x86_mem.c b/sys/x86/x86/x86_mem.c
index d639224f840..8bc4d3917a0 100644
--- a/sys/x86/x86/x86_mem.c
+++ b/sys/x86/x86/x86_mem.c
@@ -260,7 +260,7 @@ x86_mrfetch(struct mem_range_softc *sc)
/* Compute the range from the mask. Ick. */
mrd->mr_len = (~(msrv & mtrr_physmask) &
- (mtrr_physmask | 0xfffLL)) + 1;
+ (mtrr_physmask | 0xfff)) + 1;
if (!mrvalid(mrd->mr_base, mrd->mr_len))
mrd->mr_flags |= MDF_BOGUS;
@@ -638,7 +638,8 @@ x86_mrinit(struct mem_range_softc *sc)
* Determine the size of the PhysMask and PhysBase fields in
* the variable range MTRRs.
*/
- mtrr_physmask = (((uint64_t)1 << cpu_maxphyaddr) - 1) & ~0xfffULL;
+ mtrr_physmask = (((uint64_t)1 << cpu_maxphyaddr) - 1) &
+ ~(uint64_t)0xfff;
/* If fixed MTRRs supported and enabled. */
if ((mtrrcap & MTRR_CAP_FIXED) && (mtrrdef & MTRR_DEF_FIXED_ENABLE)) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170224125335.GV2092>