RE: [PATCH] x86: Implement Linear Address Masking support
From: David Laight
Date: Thu May 12 2022 - 11:34:00 EST
From: Thomas Gleixner
> Sent: 12 May 2022 16:07
>
> On Thu, May 12 2022 at 15:07, Matthew Wilcox wrote:
> > On Thu, May 12, 2022 at 01:01:07PM +0000, David Laight wrote:
> >> > +static inline int64_t sign_extend64(uint64_t value, int index)
> >> > +{
> >> > + int shift = 63 - index;
> >> > + return (int64_t)(value << shift) >> shift;
> >> > +}
> >>
> >> Shift of signed integers are UB.
> >
> > Citation needed.
>
> I'll bite :)
>
> C11/19: 6.5.7 Bitwise shift operators
>
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
> bits are filled with zeros. If E1 has an unsigned type, the value of
> the result is E1 × 2E2, reduced modulo one more than the maximum
> value representable in the result type. If E1 has a signed type and
> nonnegative value, and E1 × 2E2 is representable in the result type,
> then that is the resulting value; otherwise, the behavior is
> undefined.
>
> This is irrelevant for the case above because the left shift is on an
> unsigned integer. The interesting part is this:
>
> 5 The result of E1 >> E2 is E1 right-shifted E2 bit positions. If E1
> has an unsigned type or if E1 has a signed type and a nonnegative
> value, the value of the result is the integral part of the quotient
> of E1/2E2. If E1 has a signed type and a negative value, the
> resulting value is implementation-defined.
>
> So it's not UB, it's implementation defined. The obvious choice is to
> keep LSB set, i.e. arithmetic shift, what both GCC and clang do.
I'm sure someone recently said one of the standards had made them UB.
In any case, given the caller seems to know whether the top bit is set
(and does a different call) using |= or &= is distinctly better.
Especially since the required constant can be computed in a slow path.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)