Re: [PATCH v2 3/5] target/s390x: Fix cc_calc_sla_64() missing overflows

[Ilya Leoshkevich]

On Wed, 2022-01-12 at 16:45 +0100, David Hildenbrand wrote:
> > > If the sign is false, the shifted bits (mask) have to be 0.
> > > If the sign bit is true, the shifted bits (mask) have to be set.
> > 
> > IIUC this logic handles sign bit + "shift - 1" bits. So if the last
> > shifted bit is different, the overflow is not detected.
> 
> Ah, right, because of the - 1ULL ...
> 
> [...]
> 
> > > This looks like some black magic :)
> > 
> > Yeah, I felt this way too, but didn't come up with anything better
> > and
> > just left a comment warning not to simplify.
> > 
> 
> I wonder if all we want is
> 
> const uint64_t sign = 1ULL << 63;
> uint64_t mask = (-1ULL << (63 - shift)) & ~sign;
> 
> For shift =
> *  0: 0000000...0b
> *  1: 0100000...0b
> *  2: 0110000...0b
> * 63: 0111111...1b
> 
> Seems to survive your tests.

-1ULL does indeed help a lot to simplify this.
I don't think we even need to mask out the sign, since it should be
the same as the other bits anyway. So just

    uint64_t mask = -1ULL << (63 - shift);

appears to be enough.