Re: [PATCH] longlong.h: Do no use asm input cast for clang
On Tue, 10 Jan 2023, Adhemerval Zanella Netto via Gcc-patches wrote: > That's my original intention [1], but Joseph stated that GCC is the upstream > source of this file. Joseph, would you be ok for a similar patch to glibc > since gcc is reluctant to accept it? I don't think it's a good idea for the copies to diverge. I also think the file is more heavily used in GCC (as part of the libgcc sources, effectively) than in glibc and so it's best to use GCC as the upstream for this shared file. Ideally maybe most of the macros in this file would be replaced by built-in functions (that are guaranteed to expand inline rather than possibly circularly calling a libgcc function defined using the same macro), so that the inline asm could be avoided (when building libgcc, or when building glibc with a new-enough compiler). But that would be a substantial project. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] longlong.h: Do no use asm input cast for clang
On Tue, Jan 10, 2023 at 03:35:37PM +0100, Andreas Schwab wrote: > On Jan 10 2023, Segher Boessenkool wrote: > > > The file starts with > > > > /* longlong.h -- definitions for mixed size 32/64 bit arithmetic. > >Copyright (C) 1991-2022 Free Software Foundation, Inc. > > > >This file is part of the GNU C Library. > > > > Please change that first then? > > GCC is the source of the original version of longlong.h (from 1991). It > has then been imported into GMP, from where it found its way into GLIBC. > After that, the file has been synchronized back and forth between GCC > and GLIBC. Then change the header to make that clear? The current state suggests that Glibc is the master copy. I don't care what way this is resolved, but it would be good if it was resolved *some* way :-) We have rules and policies only to make clear to everyone what to expect and what to do. To make live easier for everyone! Segher
Re: [PATCH] longlong.h: Do no use asm input cast for clang
On Jan 10 2023, Segher Boessenkool wrote: > The file starts with > > /* longlong.h -- definitions for mixed size 32/64 bit arithmetic. >Copyright (C) 1991-2022 Free Software Foundation, Inc. > >This file is part of the GNU C Library. > > Please change that first then? GCC is the source of the original version of longlong.h (from 1991). It has then been imported into GMP, from where it found its way into GLIBC. After that, the file has been synchronized back and forth between GCC and GLIBC. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH] longlong.h: Do no use asm input cast for clang
Hi! On Tue, Jan 10, 2023 at 09:26:13AM -0300, Adhemerval Zanella Netto wrote: > On 12/12/22 20:52, Segher Boessenkool wrote: > > On Mon, Dec 12, 2022 at 02:10:16PM -0300, Adhemerval Zanella Netto wrote: > > How do you intend to modify all the existing copies of the header that > > haven't been updated for over a decade already?> > > If you think changing all user code that uses longlong.h is a good idea, > > please change it to not use inline asm, use builtins in some cases but > > mostly just rewrite things in plain C. But GCC cannot rewrite user code > > (not preemptively anyway ;-) ) -- and longlong.h as encountered in the > > wild (not the one in our libgcc source code) is user code. > > > > If you think changing the copy in libgcc is a good idea, please change > > the original in glibc first? > > That's my original intention [1], but Joseph stated that GCC is the upstream > source of this file. Joseph, would you be ok for a similar patch to glibc > since gcc is reluctant to accept it? > > [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143050.html The file starts with /* longlong.h -- definitions for mixed size 32/64 bit arithmetic. Copyright (C) 1991-2022 Free Software Foundation, Inc. This file is part of the GNU C Library. Please change that first then? Segher
Re: [PATCH] longlong.h: Do no use asm input cast for clang
On 12/12/22 20:52, Segher Boessenkool wrote: > On Mon, Dec 12, 2022 at 02:10:16PM -0300, Adhemerval Zanella Netto wrote: >> On 30/11/22 20:24, Segher Boessenkool wrote: >>> I understand that the casts should be no-ops on the asm side (maybe they >>> change the sign) and they are present as type-checking. Can we implement >>> this type-checking in a different (portable) way? I think the macro you use >>> should be named like __asm_output_check_type (..) or so to indicate the >>> intended purpose. > > I didn't write that. Please quote correctly. Thanks! > >> I do not think trying to leverage it on clang side would yield much, it >> seems that it really does not want to support this extension. I am not >> sure we can really make it portable, best option I can think of would to >> add a mix of __builtin_classify_type and typeof prior asm call (we do >> something similar to powerp64 syscall code on glibc), although it would >> still require some gcc specific builtins. >> >> I am open for ideas, since to get this header to be clang-compatible on >> glibc it requires to get it first on gcc. > > How do you intend to modify all the existing copies of the header that > haven't been updated for over a decade already?> > If you think changing all user code that uses longlong.h is a good idea, > please change it to not use inline asm, use builtins in some cases but > mostly just rewrite things in plain C. But GCC cannot rewrite user code > (not preemptively anyway ;-) ) -- and longlong.h as encountered in the > wild (not the one in our libgcc source code) is user code. > > If you think changing the copy in libgcc is a good idea, please change > the original in glibc first? That's my original intention [1], but Joseph stated that GCC is the upstream source of this file. Joseph, would you be ok for a similar patch to glibc since gcc is reluctant to accept it? [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143050.html
Re: [PATCH] longlong.h: Do no use asm input cast for clang
On Mon, Dec 12, 2022 at 02:10:16PM -0300, Adhemerval Zanella Netto wrote: > On 30/11/22 20:24, Segher Boessenkool wrote: > > I understand that the casts should be no-ops on the asm side (maybe they > > change the sign) and they are present as type-checking. Can we implement > > this type-checking in a different (portable) way? I think the macro you use > > should be named like __asm_output_check_type (..) or so to indicate the > > intended purpose. I didn't write that. Please quote correctly. Thanks! > I do not think trying to leverage it on clang side would yield much, it > seems that it really does not want to support this extension. I am not > sure we can really make it portable, best option I can think of would to > add a mix of __builtin_classify_type and typeof prior asm call (we do > something similar to powerp64 syscall code on glibc), although it would > still require some gcc specific builtins. > > I am open for ideas, since to get this header to be clang-compatible on > glibc it requires to get it first on gcc. How do you intend to modify all the existing copies of the header that haven't been updated for over a decade already? If you think changing all user code that uses longlong.h is a good idea, please change it to not use inline asm, use builtins in some cases but mostly just rewrite things in plain C. But GCC cannot rewrite user code (not preemptively anyway ;-) ) -- and longlong.h as encountered in the wild (not the one in our libgcc source code) is user code. If you think changing the copy in libgcc is a good idea, please change the original in glibc first? Segher
Re: [PATCH] longlong.h: Do no use asm input cast for clang
Hi! On Thu, Dec 01, 2022 at 08:26:52AM +0100, Richard Biener wrote: > On Thu, Dec 1, 2022 at 12:26 AM Segher Boessenkool > wrote: > > On Wed, Nov 30, 2022 at 03:16:25PM -0300, Adhemerval Zanella via > > Gcc-patches wrote: > > > clang by default rejects the input casts with: > > > > > > error: invalid use of a cast in a inline asm context requiring an > > > lvalue: remove the cast or build with -fheinous-gnu-extensions > > > > > > And even with -fheinous-gnu-extensions clang still throws an warning > > > and also states that this option might be removed in the future. > > > For gcc the cast are still useful somewhat [1], so just remove it > > > clang is used. > > > > This is one of the things in inline asm that is tightly tied to GCC > > internals. You should emulate GCC's behaviour faithfully if you want > > to claim you implement the inline asm GNU C extension. > > I understand that the casts should be no-ops on the asm side (maybe they > change the sign) and they are present as type-checking. Can we implement > this type-checking in a different (portable) way? Portable? Portable between which things? Inline assembler is a GNU C extension, this is portable between any two compilers that implement that (correctly), already. This can be written some other way of course, but as I said before, most instances of longlong.h that are used "in the wild" are over ten years old, so we really cannot fix that "problem". If we would distribute this header with GCC, then we could start doing such things as soon as people start using the new header. But almost all of the functionality the header provides is legacy anyway! > I think the macro you use > should be named like __asm_output_check_type (..) or so to indicate the > intended purpose. I'm all for that, certainly. Or a better name preferably (check type for what? And do what with the result? Etc.) Segher
Re: [PATCH] longlong.h: Do no use asm input cast for clang
On 30/11/22 20:24, Segher Boessenkool wrote: > Hi! > > On Wed, Nov 30, 2022 at 03:16:25PM -0300, Adhemerval Zanella via Gcc-patches > wrote: >> clang by default rejects the input casts with: >> >> error: invalid use of a cast in a inline asm context requiring an >> lvalue: remove the cast or build with -fheinous-gnu-extensions >> >> And even with -fheinous-gnu-extensions clang still throws an warning >> and also states that this option might be removed in the future. >> For gcc the cast are still useful somewhat [1], so just remove it >> clang is used. > > This is one of the things in inline asm that is tightly tied to GCC > internals. You should emulate GCC's behaviour faithfully if you want > to claim you implement the inline asm GNU C extension. Agree, that's why I just make it a no-op for clang which indicates that it does not seem much use of this extension. > I understand that the casts should be no-ops on the asm side (maybe they > change the sign) and they are present as type-checking. Can we implement > this type-checking in a different (portable) way? I think the macro you use > should be named like __asm_output_check_type (..) or so to indicate the > intended purpose. I do not think trying to leverage it on clang side would yield much, it seems that it really does not want to support this extension. I am not sure we can really make it portable, best option I can think of would to add a mix of __builtin_classify_type and typeof prior asm call (we do something similar to powerp64 syscall code on glibc), although it would still require some gcc specific builtins. I am open for ideas, since to get this header to be clang-compatible on glibc it requires to get it first on gcc. > >> --- a/include/ChangeLog >> +++ b/include/ChangeLog > > That should not be part of the patch? Changelog entries should be > verbatim in the message you send. > > The size of this patch already makes clear this is a bad idea, imo. > This code is already hard enough to read. Indeed, I forgot that CL entries were not part of the commit.
Re: [PATCH] longlong.h: Do no use asm input cast for clang
On Thu, Dec 1, 2022 at 12:26 AM Segher Boessenkool wrote: > > Hi! > > On Wed, Nov 30, 2022 at 03:16:25PM -0300, Adhemerval Zanella via Gcc-patches > wrote: > > clang by default rejects the input casts with: > > > > error: invalid use of a cast in a inline asm context requiring an > > lvalue: remove the cast or build with -fheinous-gnu-extensions > > > > And even with -fheinous-gnu-extensions clang still throws an warning > > and also states that this option might be removed in the future. > > For gcc the cast are still useful somewhat [1], so just remove it > > clang is used. > > This is one of the things in inline asm that is tightly tied to GCC > internals. You should emulate GCC's behaviour faithfully if you want > to claim you implement the inline asm GNU C extension. I understand that the casts should be no-ops on the asm side (maybe they change the sign) and they are present as type-checking. Can we implement this type-checking in a different (portable) way? I think the macro you use should be named like __asm_output_check_type (..) or so to indicate the intended purpose. Richard. > > --- a/include/ChangeLog > > +++ b/include/ChangeLog > > That should not be part of the patch? Changelog entries should be > verbatim in the message you send. > > The size of this patch already makes clear this is a bad idea, imo. > This code is already hard enough to read. > > > Segher
Re: [PATCH] longlong.h: Do no use asm input cast for clang
Hi! On Wed, Nov 30, 2022 at 03:16:25PM -0300, Adhemerval Zanella via Gcc-patches wrote: > clang by default rejects the input casts with: > > error: invalid use of a cast in a inline asm context requiring an > lvalue: remove the cast or build with -fheinous-gnu-extensions > > And even with -fheinous-gnu-extensions clang still throws an warning > and also states that this option might be removed in the future. > For gcc the cast are still useful somewhat [1], so just remove it > clang is used. This is one of the things in inline asm that is tightly tied to GCC internals. You should emulate GCC's behaviour faithfully if you want to claim you implement the inline asm GNU C extension. > --- a/include/ChangeLog > +++ b/include/ChangeLog That should not be part of the patch? Changelog entries should be verbatim in the message you send. The size of this patch already makes clear this is a bad idea, imo. This code is already hard enough to read. Segher
[PATCH] longlong.h: Do no use asm input cast for clang
clang by default rejects the input casts with: error: invalid use of a cast in a inline asm context requiring an lvalue: remove the cast or build with -fheinous-gnu-extensions And even with -fheinous-gnu-extensions clang still throws an warning and also states that this option might be removed in the future. For gcc the cast are still useful somewhat [1], so just remove it clang is used. [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-October/581722.html --- include/ChangeLog | 60 ++ include/longlong.h | 524 +++-- 2 files changed, 325 insertions(+), 259 deletions(-) diff --git a/include/ChangeLog b/include/ChangeLog index dda005335c0..747fc923ef5 100644 --- a/include/ChangeLog +++ b/include/ChangeLog @@ -1,3 +1,63 @@ +2022-11-30 Adhemerval Zanella + + * include/longlong.h: Modified. + [(__GNUC__) && ! NO_ASM][( (__i386__) || (__i486__)) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][( (__i386__) || (__i486__)) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][( (__i386__) || (__i486__)) && W_TYPE_SIZE == 32](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][( (__i386__) || (__i486__)) && W_TYPE_SIZE == 32](udiv_qrnnd): Modified. + [(__GNUC__) && ! NO_ASM][(( (__sparc__) && (__arch64__)) || (__sparcv9)) && W_TYPE_SIZE == 64](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(( (__sparc__) && (__arch64__)) || (__sparcv9)) && W_TYPE_SIZE == 64](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(( (__sparc__) && (__arch64__)) || (__sparcv9)) && W_TYPE_SIZE == 64](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__M32R__) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__M32R__) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__arc__) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__arc__) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__arm__) && ( (__thumb2__) || ! __thumb__) && W_TYPE_SIZE == 32][(__ARM_ARCH_2__) || (__ARM_ARCH_2A__) || (__ARM_ARCH_3__)](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__arm__) && ( (__thumb2__) || ! __thumb__) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__arm__) && ( (__thumb2__) || ! __thumb__) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__hppa) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__hppa) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__i960__) && W_TYPE_SIZE == 32](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__i960__) && W_TYPE_SIZE == 32](__umulsidi3): Modified. + [(__GNUC__) && ! NO_ASM][(__ibm032__) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__ibm032__) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__ibm032__) && W_TYPE_SIZE == 32](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__ibm032__) && W_TYPE_SIZE == 32](count_leading_zeros): Modified. + [(__GNUC__) && ! NO_ASM][(__m88000__) && W_TYPE_SIZE == 32][(__mc88110__)](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__m88000__) && W_TYPE_SIZE == 32][(__mc88110__)](udiv_qrnnd): Modified. + [(__GNUC__) && ! NO_ASM][(__m88000__) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__m88000__) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__m88000__) && W_TYPE_SIZE == 32](count_leading_zeros): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32][!((__mcoldfire__))](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32][( (__mc68020__) && ! __mc68060__)](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32][( (__mc68020__) && ! __mc68060__)](udiv_qrnnd): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32][( (__mc68020__) && ! __mc68060__)](sdiv_qrnnd): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32][(__mcoldfire__)](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32](add_ss): Modified. + [(__GNUC__) && ! NO_ASM][(__mc68000__) && W_TYPE_SIZE == 32](sub_ddmmss): Modified. + [(__GNUC__) && ! NO_ASM][(__sh__) && W_TYPE_SIZE == 32][! __sh1__](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__sparc__) && ! __arch64__ && ! __sparcv9 && W_TYPE_SIZE == 32][!((__sparc_v9__))][!((__sparc_v8__))][!((__sparclite__))](umul_ppmm): Modified. + [(__GNUC__) && ! NO_ASM][(__sparc__) && ! __arch64__ && ! __sparcv9 && W_TYPE_SIZE == 32][!((__sparc_v9__))][!((__sparc_v8__))][!((__sparclite__))](udiv_qrnnd): Modified. + [(__GNUC__) && !
