Re: Re: typeof and operands in named address spaces

Re: Re: typeof and operands in named address spaces

[Peter Zijlstra]

( restoring at least linux-toolchains@vger.kernel.org, since that seems
  to have gone missing )

On Mon, Nov 16, 2020 at 10:11:50AM +0100, Richard Biener wrote:
> On Sun, Nov 15, 2020 at 11:53 AM Uecker, Martin
> <Martin.Uecker@med.uni-goettingen.de> wrote:
> > > On Wed, Nov 04, 2020 at 07:31:42PM +0100, Uros Bizjak wrote:
> > > > Hello!
> > > >
> > > > I was looking at the recent linux patch series [1] where segment
> > > > qualifiers (named address spaces) were introduced to handle percpu
> > > > variables. In the patch [2], the author mentions that:
> > > >
> > > > --q--
> > > > Unfortunately, gcc does not provide a way to remove segment
> > > > qualifiers, which is needed to use typeof() to create local instances
> > > > of the per-cpu variable. For this reason, do not use the segment
> > > > qualifier for per-cpu variables, and do casting using the segment
> > > > qualifier instead.
> > > > --/q--
> > >
> > > C in general does not provide means to strip qualifiers. We recently had
> > > a _lot_ of 'fun' trying to strip volatile from a type, see here:
> > >
> > >   https://lore.kernel.org/lkml/875zimp0ay.fsf@mpe.ellerman.id.au
> > >
> > > which resulted in the current __unqual_scalar_typeof() hack.
> > >
> > > If we're going to do compiler extentions here, can we pretty please have
> > > a sane means of modifying qualifiers in general?
> >
> > Another way to drop qualifiers is using a cast. So you
> > can use typeof twice:
> >
> > typeof((typeof(_var))_var) tmp__;
> >
> > This also works for non-scalars but this is a GCC extension.
> >
> >
> > WG14 plans to standardize typeof. I would like to hear opinion
> > whether we should have typeof drop qualifiers or not.
> >
> > Currently, it does not do this on all compilers I tested
> > (except _Atomic on GCC) and there are also use cases for
> > keeping qualifiers. This is an argument for keeping qualifiers
> > should we standardize it, but then we need a way to drop
> > qualifiers.
> >
> >
> > lvalue conversion drops qualifers in C.  In GCC, this is not
> > implemented correctly as it is unobvervable in standard C
> > (but it using typeof).
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97702
> >
> > A have a working patch in preparation to change this. Then you
> > could use
> >
> > typeof( ((void)0, x) )

Neat, that actually already works with clang. And I suppose we can use
the above GCC extention until such time as that GCC is fixed.

See below..

> > to drop qualifiers. But this would then
> > also do array-to-pointer conversion. I am not sure
> > whether this is a problem.

I don't _think_ so, but..

> > Of course, we could also introduce a new feature for
> > dropping qualifiers. Thoughts?

> Just add a new qualifier that un-qualifies?
> 
> _Unqual volatile T x;
> 
> is T with volatile (evenually) removed.  Or just a way to drop
> all using _Unqual?
> 
> _Unqual T x;
> 
> removing all qualifiers from T.  Or add a special _Unqual_all
> to achieve that.  I think removing a specific qualification is
> useful.  Leaves cases like
> 
> _Unqual volatile volatile T x;
> 
> to be specified (that is ordering and cancellation of the
> unqual and qual variants of qualifiers).

I rather like this, however I think I'd prefer the syntax be something
like:

	_Unqual T x;

for removing all qualifiers, and:

	_Unqual(volatile) volatile T X;

for stripping specific qualifiers. The syntax as proposed above seems
very error prone to me.


---
Subject: compiler: Improve __unqual_typeof()

Improve our __unqual_scalar_typeof() implementation by relying on C
dropping qualifiers for lvalue convesions. There is one small catch in
that GCC is currently known broken in this respect, however it happens
to have a C language extention that achieves the very same, it drops
qualifiers on casts.

This gets rid of the _Generic() usage and should improve compile times
(less preprocessor output) as well as increases the capabilities of the
macros.

XXX: I've only verified the below actually compiles, I've not verified
     the generated code is actually 'correct'.

Suggested-by: "Uecker, Martin" <Martin.Uecker@med.uni-goettingen.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 74c6c0486eed..3c5cb52c12f9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -156,3 +156,11 @@
 #else
 #define __diag_GCC_8(s)
 #endif
+
+/*
+ * GCC has a bug where lvalue conversion doesn't drop qualifiers, use a GCC
+ * extention instead. GCC drops qualifiers on a cast, so use a double typeof().
+ *
+ *  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97702
+ */
+#define __unqual_typeof(type)	typeof( (typeof(type))type )
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index ac3fa37a84f9..4a6e2caab17b 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -250,27 +250,14 @@ struct ftrace_likely_data {
 /* Are two types/vars the same type (ignoring qualifiers)? */
 #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
 
+#ifndef __unqual_typeof
 /*
- * __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving
- *			       non-scalar types unchanged.
+ * lvalue conversion drops qualifiers
  */
-/*
- * Prefer C11 _Generic for better compile-times and simpler code. Note: 'char'
- * is not type-compatible with 'signed char', and we define a separate case.
- */
-#define __scalar_type_to_expr_cases(type)				\
-		unsigned type:	(unsigned type)0,			\
-		signed type:	(signed type)0
-
-#define __unqual_scalar_typeof(x) typeof(				\
-		_Generic((x),						\
-			 char:	(char)0,				\
-			 __scalar_type_to_expr_cases(char),		\
-			 __scalar_type_to_expr_cases(short),		\
-			 __scalar_type_to_expr_cases(int),		\
-			 __scalar_type_to_expr_cases(long),		\
-			 __scalar_type_to_expr_cases(long long),	\
-			 default: (x)))
+#define __unqual_typeof(type)	typeof( ((void)0, type) )
+#endif
+
+#define __unqual_scalar_typeof(type) __unqual_typeof(type)
 
 /* Is this type a native word size -- useful for atomic operations */
 #define __native_word(t) \

Re: Re: typeof and operands in named address spaces

[Peter Zijlstra]

On Mon, Nov 16, 2020 at 12:10:56PM +0100, Peter Zijlstra wrote:

> > > Another way to drop qualifiers is using a cast. So you
> > > can use typeof twice:
> > >
> > > typeof((typeof(_var))_var) tmp__;
> > >
> > > This also works for non-scalars but this is a GCC extension.

FWIW, clang seems to support this extension as well..

Re: Re: typeof and operands in named address spaces

[Uecker, Martin]

Am Montag, den 16.11.2020, 12:10 +0100 schrieb Peter Zijlstra:
> ( restoring at least linux-toolchains@vger.kernel.org, since that
> seems
>   to have gone missing )
> 
> On Mon, Nov 16, 2020 at 10:11:50AM +0100, Richard Biener wrote:
> > On Sun, Nov 15, 2020 at 11:53 AM Uecker, Martin
> > <Martin.Uecker@med.uni-goettingen.de> wrote:
> > > > On Wed, Nov 04, 2020 at 07:31:42PM +0100, Uros Bizjak wrote:
> > > > > Hello!
> > > > > 
> > > > > I was looking at the recent linux patch series [1] where
> > > > > segment
> > > > > qualifiers (named address spaces) were introduced to handle
> > > > > percpu
> > > > > variables. In the patch [2], the author mentions that:
> > > > > 
> > > > > --q--
> > > > > Unfortunately, gcc does not provide a way to remove segment
> > > > > qualifiers, which is needed to use typeof() to create local
> > > > > instances
> > > > > of the per-cpu variable. For this reason, do not use the
> > > > > segment
> > > > > qualifier for per-cpu variables, and do casting using the
> > > > > segment
> > > > > qualifier instead.
> > > > > --/q--
> > > > 
> > > > C in general does not provide means to strip qualifiers. We
> > > > recently had
> > > > a _lot_ of 'fun' trying to strip volatile from a type, see
> > > > here:
> > > > 
> > > >   
> > > > https://lore.kernel.org/lkml/875zimp0ay.fsf@mpe.ellerman.id.au
> > > > 
> > > > which resulted in the current __unqual_scalar_typeof() hack.
> > > > 
> > > > If we're going to do compiler extentions here, can we pretty
> > > > please have
> > > > a sane means of modifying qualifiers in general?
> > > 
> > > Another way to drop qualifiers is using a cast. So you
> > > can use typeof twice:
> > > 
> > > typeof((typeof(_var))_var) tmp__;
> > > 
> > > This also works for non-scalars but this is a GCC extension.

(That casts drop qualifiers is standard C. The extensions
are 'typeof' and that casts can be used for non-scalar types.)

> > > 
> > > WG14 plans to standardize typeof. I would like to hear opinion
> > > whether we should have typeof drop qualifiers or not.
> > > 
> > > Currently, it does not do this on all compilers I tested
> > > (except _Atomic on GCC) and there are also use cases for
> > > keeping qualifiers. This is an argument for keeping qualifiers
> > > should we standardize it, but then we need a way to drop
> > > qualifiers.
> > > 
> > > 
> > > lvalue conversion drops qualifers in C.  In GCC, this is not
> > > implemented correctly as it is unobvervable in standard C
> > > (but it using typeof).
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97702
> > > 
> > > A have a working patch in preparation to change this. Then you
> > > could use
> > > 
> > > typeof( ((void)0, x) )
> 
> Neat, that actually already works with clang. And I suppose we can
> use
> the above GCC extention until such time as that GCC is fixed.
> 
> See below..
> 
> > > to drop qualifiers. But this would then
> > > also do array-to-pointer conversion. I am not sure
> > > whether this is a problem.
> 
> I don't _think_ so, but..
> 
> > > Of course, we could also introduce a new feature for
> > > dropping qualifiers. Thoughts?
> > Just add a new qualifier that un-qualifies?
> > 
> > _Unqual volatile T x;
> > 
> > is T with volatile (evenually) removed.  Or just a way to drop
> > all using _Unqual?
> > 
> > _Unqual T x;
> > 
> > removing all qualifiers from T.  Or add a special _Unqual_all
> > to achieve that.  I think removing a specific qualification is
> > useful.  Leaves cases like
> > 
> > _Unqual volatile volatile T x;
> > 
> > to be specified (that is ordering and cancellation of the
> > unqual and qual variants of qualifiers).
> 
> I rather like this, however I think I'd prefer the syntax be
> something
> like:
> 
> 	_Unqual T x;
> 
> for removing all qualifiers, and:
> 
> 	_Unqual(volatile) volatile T X;
> 
> for stripping specific qualifiers. The syntax as proposed above seems
> very error prone to me.


There is also the idea of adding 'auto' to C which
would drop
qualifiers. (__auto_type does not on GCC
but on some other compilers)

Best,
Martin



> 
> 
> ---
> Subject: compiler: Improve __unqual_typeof()
> 
> Improve our __unqual_scalar_typeof() implementation by relying on C
> dropping qualifiers for lvalue convesions. There is one small catch
> in
> that GCC is currently known broken in this respect, however it
> happens
> to have a C language extention that achieves the very same, it drops
> qualifiers on casts.
> 
> This gets rid of the _Generic() usage and should improve compile
> times
> (less preprocessor output) as well as increases the capabilities of
> the
> macros.
> 
> XXX: I've only verified the below actually compiles, I've not
> verified
>      the generated code is actually 'correct'.
> 
> Suggested-by: "Uecker, Martin" <Martin.Uecker@med.uni-goettingen.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-
> gcc.h
> index 74c6c0486eed..3c5cb52c12f9 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -156,3 +156,11 @@
>  #else
>  #define __diag_GCC_8(s)
>  #endif
> +
> +/*
> + * GCC has a bug where lvalue conversion doesn't drop qualifiers,
> use a GCC
> + * extention instead. GCC drops qualifiers on a cast, so use a
> double typeof().
> + *
> + *  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97702
> + */
> +#define __unqual_typeof(type)	typeof( (typeof(type))type )
> diff --git a/include/linux/compiler_types.h
> b/include/linux/compiler_types.h
> index ac3fa37a84f9..4a6e2caab17b 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -250,27 +250,14 @@ struct ftrace_likely_data {
>  /* Are two types/vars the same type (ignoring qualifiers)? */
>  #define __same_type(a, b) __builtin_types_compatible_p(typeof(a),
> typeof(b))
>  
> +#ifndef __unqual_typeof
>  /*
> - * __unqual_scalar_typeof(x) - Declare an unqualified scalar type,
> leaving
> - *			       non-scalar types unchanged.
> + * lvalue conversion drops qualifiers
>   */
> -/*
> - * Prefer C11 _Generic for better compile-times and simpler code.
> Note: 'char'
> - * is not type-compatible with 'signed char', and we define a
> separate case.
> - */
> -#define __scalar_type_to_expr_cases(type)				
> \
> -		unsigned type:	(unsigned type)0,			
> \
> -		signed type:	(signed type)0
> -
> -#define __unqual_scalar_typeof(x) typeof(				
> \
> -		_Generic((x),						
> \
> -			 char:	(char)0,				
> \
> -			 __scalar_type_to_expr_cases(char),		
> \
> -			 __scalar_type_to_expr_cases(short),		
> \
> -			 __scalar_type_to_expr_cases(int),		
> \
> -			 __scalar_type_to_expr_cases(long),		
> \
> -			 __scalar_type_to_expr_cases(long long),	\
> -			 default: (x)))
> +#define __unqual_typeof(type)	typeof( ((void)0, type) )
> +#endif
> +
> +#define __unqual_scalar_typeof(type) __unqual_typeof(type)
>  
>  /* Is this type a native word size -- useful for atomic operations
> */
>  #define __native_word(t) \

Re: Re: typeof and operands in named address spaces

[Peter Zijlstra]

On Mon, Nov 16, 2020 at 12:23:17PM +0000, Uecker, Martin wrote:

> > > > Another way to drop qualifiers is using a cast. So you
> > > > can use typeof twice:
> > > > 
> > > > typeof((typeof(_var))_var) tmp__;
> > > > 
> > > > This also works for non-scalars but this is a GCC extension.
> 
> (That casts drop qualifiers is standard C. The extensions
> are 'typeof' and that casts can be used for non-scalar types.)

Ah, I'll clarify. Thanks!

Re: Re: typeof and operands in named address spaces

[Linus Torvalds]

On Mon, Nov 16, 2020 at 3:11 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> XXX: I've only verified the below actually compiles, I've not verified
>      the generated code is actually 'correct'.

Well, it was mainly the arm64 code generation for load-acquire and
store-release that wanted this - so it's really the generated code
that matters.

Will, can you check?

Because:

> +#define __unqual_typeof(type)  typeof( (typeof(type))type )

that's certainly a much nicer version than the existing pre-processor
expansion from hell.

             Linus

Re: Re: typeof and operands in named address spaces

[Jakub Jelinek]

On Tue, Nov 17, 2020 at 11:13:52AM -0800, Linus Torvalds wrote:
> > +#define __unqual_typeof(type)  typeof( (typeof(type))type )

> that's certainly a much nicer version than the existing pre-processor
> expansion from hell.

It would need to be typeof( (typeof(type)) (type) ) to not be that
constrained on what kind of expressions it accepts as arguments.
Anyway, it won't work with array types at least,
  int a[10];
  typeof ((typeof (a)) (a)) b;
is an error (in both gcc and clang), while typeof (a) b; will work
(but not drop the qualifiers).  Don't know if the kernel cares or not.

	Jakub

Re: Re: typeof and operands in named address spaces

[Linus Torvalds]

On Tue, Nov 17, 2020 at 11:25 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> It would need to be typeof( (typeof(type)) (type) ) to not be that
> constrained on what kind of expressions it accepts as arguments.

Yup.

> Anyway, it won't work with array types at least,
>   int a[10];
>   typeof ((typeof (a)) (a)) b;
> is an error (in both gcc and clang), while typeof (a) b; will work
> (but not drop the qualifiers).  Don't know if the kernel cares or not.

Well, the kernel already doesn't allow that, because our existing
horror only handles simple integer scalar types.

So that macro is a clear improvement - if it actually works (local
testing says it does, but who knows about random compiler versions
etc)

          Linus

Re: Re: typeof and operands in named address spaces

[Will Deacon]

On Tue, Nov 17, 2020 at 11:31:57AM -0800, Linus Torvalds wrote:
> On Tue, Nov 17, 2020 at 11:25 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > It would need to be typeof( (typeof(type)) (type) ) to not be that
> > constrained on what kind of expressions it accepts as arguments.
> 
> Yup.
> 
> > Anyway, it won't work with array types at least,
> >   int a[10];
> >   typeof ((typeof (a)) (a)) b;
> > is an error (in both gcc and clang), while typeof (a) b; will work
> > (but not drop the qualifiers).  Don't know if the kernel cares or not.
> 
> Well, the kernel already doesn't allow that, because our existing
> horror only handles simple integer scalar types.
> 
> So that macro is a clear improvement - if it actually works (local
> testing says it does, but who knows about random compiler versions
> etc)

I'll give it a go now, although if it works I honestly won't know whether
to laugh or cry.

Will

Re: Re: typeof and operands in named address spaces

[Will Deacon]

On Tue, Nov 17, 2020 at 09:10:53PM +0000, Will Deacon wrote:
> On Tue, Nov 17, 2020 at 11:31:57AM -0800, Linus Torvalds wrote:
> > On Tue, Nov 17, 2020 at 11:25 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > >
> > > It would need to be typeof( (typeof(type)) (type) ) to not be that
> > > constrained on what kind of expressions it accepts as arguments.
> > 
> > Yup.
> > 
> > > Anyway, it won't work with array types at least,
> > >   int a[10];
> > >   typeof ((typeof (a)) (a)) b;
> > > is an error (in both gcc and clang), while typeof (a) b; will work
> > > (but not drop the qualifiers).  Don't know if the kernel cares or not.
> > 
> > Well, the kernel already doesn't allow that, because our existing
> > horror only handles simple integer scalar types.
> > 
> > So that macro is a clear improvement - if it actually works (local
> > testing says it does, but who knows about random compiler versions
> > etc)
> 
> I'll give it a go now, although if it works I honestly won't know whether
> to laugh or cry.

GCC 9 and Clang 11 both seem to generate decent code for aarch64 defconfig
with:

#define __unqual_scalar_typeof(x)      typeof( (typeof(x)) (x))

replacing the current monstrosity. allnoconfig and allmodconfig build fine
too.

However, GCC 4.9.0 goes mad and starts spilling to the stack when dealing
with a pointer to volatile, as though we were just using typeof(). I tried
GCC 5.4.0 and that looks ok, so I think if anybody cares about the potential
performance regression with 4.9 then perhaps they should consider upgrading
their toolchain.

In other words, let's do it.

Will

Re: Re: typeof and operands in named address spaces

[Uecker, Martin]

Am Dienstag, den 17.11.2020, 11:31 -0800 schrieb Linus Torvalds:
> On Tue, Nov 17, 2020 at 11:25 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > 
> > It would need to be typeof( (typeof(type)) (type) ) to not be that
> > constrained on what kind of expressions it accepts as arguments.
> 
> Yup.
> 
> > Anyway, it won't work with array types at least,
> >   int a[10];
> >   typeof ((typeof (a)) (a)) b;
> > is an error (in both gcc and clang), while typeof (a) b; will work
> > (but not drop the qualifiers).  Don't know if the kernel cares or not.
> 
> Well, the kernel already doesn't allow that, because our existing
> horror only handles simple integer scalar types.
> 
> So that macro is a clear improvement - if it actually works (local
> testing says it does, but who knows about random compiler versions
> etc)

Well, the version that also works for other types (except
multi-dim arrays) is slightly more complicated ;-)

https://godbolt.org/z/KTKqon

Best,
Martin

Re: Re: typeof and operands in named address spaces

[Linus Torvalds]

On Tue, Nov 17, 2020 at 11:13 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> > +#define __unqual_typeof(type)  typeof( (typeof(type))type )
>
> that's certainly a much nicer version than the existing pre-processor
> expansion from hell.

Oh, and sparse doesn't handle this, and doesn't remove any qualifiers
for the above. And so this horror-test-case fails:

    #define __unqual_typeof(type)  typeof( (typeof(type))(type) )

    int *function(volatile int x)
    {
        extern __unqual_typeof(x) y;
        return &y;
    }

with

  t.c:6:17: warning: incorrect type in return expression (different modifiers)
  t.c:6:17:    expected int *
  t.c:6:17:    got int volatile *
  t.c:3:5: warning: symbol 'function' was not declared. Should it be static?

adding Luc and the sparse mailing list to the participants list.

But it does work with both gcc and clang for me.

For Luc, quoting some earlier emails:

> > lvalue conversion drops qualifers in C.  In GCC, this is not
> > implemented correctly as it is unobservable in standard C
> > (but it using typeof).
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97702
> >
> > A have a working patch in preparation to change this. Then you
> > could use
> >
> > typeof( ((void)0, x) )

on another thing that fails with sparse. But since gcc gets that wrong
too and it only works with clang, that's not so relevant for the
kernel.

Luc - same test-case as above, just

    #define __unqual_typeof(x) typeof( ((void)0, (x)) )

instead.

           Linus