On Wed, Aug 01, 2018 at 11:36:03AM +0200, Peter Zijlstra wrote:

On Tue, Jun 26, 2018 at 11:54:35AM -0600, Jason Gunthorpe wrote:

quoted

What about more like this?

          check_shift_overflow(a, s, d) ({

Should that not be: check_shl_overflow() ? Just 'shift' lacks a
direction.

Yes, that makes sense.

quoted

	      // Shift is always performed on the machine's largest unsigned
              u64 _a = a;
	      typeof(s) _s = s;
              typeof(d) _d = d;

	      // Make s safe against UB
	      unsigned int _to_shift = _s >= 0 && _s < 8*sizeof(*d) : _s ? 0;

Should we not do a gcc-plugin or something that fixes that particular
UB? Shift acting all retarded like that is just annoying. I feel we
should eliminate UBs from the language where possible, like
-fno-strict-overflow mandates 2s complement.

No idea, if someone does this they can remove the above overhead..

quoted

              *_d = (_a << _to_shift);

	       // s is malformed
              (_to_shift != _s ||

Not strictly an overflow though, just not expected behaviour.

'overflow' here means the math didn't work, ie
   C = A << B
has a C that does not match A << B done on infinite precision. It is
not limited to checking overflow.

quoted

	       // d is a signed type and became negative
	       *_d < 0 ||

Only a problem if it wasn't negative to start out with.

quoted

	       // a is a signed type and was negative
	       _a < 0 ||

Why would that be a problem? You can shift left negative values just
fine. The only problem is when you run out of sign bits.

These are both a problem because of how the macro is setup, nobody had
an idea how to make this work with different types and allow for
negatives to work properly.

You could define this, but since there is no usecase..

quoted

	       // Not invertable means a was truncated during shifting
	       (*_d >> _to_shift) != a))
          })

And I'm not exactly seeing the use case for this macro. What's the point
of a shift-left if you cannot truncate bits. I suppose it's in the name
_overflow, but still.

It is basically a specialized case of check_mul_overflow where the
multiply is known to be a power of 2.

Jason