From: Linus Torvalds
> Sent: 10 January 2024 19:35
> 
> On Tue, 9 Jan 2024 at 22:17, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > > Can somebody else confirm similar time differences? Or is it just me?
> >
> > I was hopeful, but:
> 
> Yeah, my build times seem to be very unstable for some reason, and
> seem to fluctuate fairly widely. I'm not sure what triggers it.
> 
> The min/max simplification helps,

The first check in __types_ok() can go, the second one (with the '+ 0')
(added to promote char to int) includes the first one.

-Wsign-compare will need work from the compiler people.
I suspect that when min(unsigned_var, 4) effectively does
	int four = 4;
	if (unsigned_var > four)
it is going to trip the warning until gcc uses its value tracking
for that warning.

Have you looked at how much the compile-time string length
stuff costs? That might also be measurable.
And the compile-time usercopy tests.
The run-time costs of the latter can be horrid...

> but I think my "big change" thing
> was mostly due to other fluctuations.
> 
> It would be lovely to have some performance automation to find build
> time regressions, although at least for me, one source of regressions
> tends to be system updates with new compilers ;(

Can we go back to gcc 2.95 or 2.97 - that would be quick on a modern cpu :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[ Going through some pending issues now that I've mostly emptied my pull queue ]

On Wed, 10 Jan 2024 at 14:58, David Laight <David.Laight@aculab.com> wrote:
>
> The first check in __types_ok() can go, the second one (with the '+ 0')
> (added to promote char to int) includes the first one.

That turns out to not be true. An expression like

  min(u8, unsigned int)

is fine because the underlying types are compatible.

But the promotion to 'int' makes the first argument be a signed
integer, and is no longer compatible with the second argument.

             Linus

From: Linus Torvalds
> Sent: 20 January 2024 21:34
> 
> [ Going through some pending issues now that I've mostly emptied my pull queue ]
> 
> On Wed, 10 Jan 2024 at 14:58, David Laight <David.Laight@aculab.com> wrote:
> >
> > The first check in __types_ok() can go, the second one (with the '+ 0')
> > (added to promote char to int) includes the first one.
> 
> That turns out to not be true. An expression like
> 
>   min(u8, unsigned int)
> 
> is fine because the underlying types are compatible.
> 
> But the promotion to 'int' makes the first argument be a signed
> integer, and is no longer compatible with the second argument.

Yes, I realised that afterwards.

This version is much simpler though.

+/* Allow unsigned compares against non-negative signed constants. */
+#define __is_ok_unsigned(x) \
+       (!is_signed_type(typeof(x)) || (__is_constexpr(x) ? (x) >= 0 : 0))
+
+/* Check for signed after promoting unsigned char/short to int */
+#define __is_ok_signed(x) is_signed_type(typeof((x) + 0))
+
+/* Allow if both x and y are valid for either signed or unsigned compares. */
+#define __types_ok(x, y)                               \
+       ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
+        (__is_ok_unsigned(x) && __is_ok_unsigned(y)))

And _Statc_assert() only needs a compile-time constant, not
a constant expression - so no need for all the __builtin_choose_expr().

I'll post the actual patch series in a couple of days.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)