[PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Uros Bizjak]

The workaround for 'asm goto' miscompilation introduces a compiler
barrier quirk that inhibits many useful compiler optimizations. For
example, __try_cmpxchg_user compiles to:

   11375:	41 8b 4d 00          	mov    0x0(%r13),%ecx
   11379:	41 8b 02             	mov    (%r10),%eax
   1137c:	f0 0f b1 0a          	lock cmpxchg %ecx,(%rdx)
   11380:	0f 94 c2             	sete   %dl
   11383:	84 d2                	test   %dl,%dl
   11385:	75 c4                	jne    1134b <...>
   11387:	41 89 02             	mov    %eax,(%r10)

where the barrier inhibits flags propagation from asm when
compiled with gcc-12.

When the mentioned quirk is removed, the following code is generated:

   11553:	41 8b 4d 00          	mov    0x0(%r13),%ecx
   11557:	41 8b 02             	mov    (%r10),%eax
   1155a:	f0 0f b1 0a          	lock cmpxchg %ecx,(%rdx)
   1155e:	74 c9                	je     11529 <...>
   11560:	41 89 02             	mov    %eax,(%r10)

The refered compiler bug:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670

was fixed for gcc-4.8.2.

Current minimum required version of GCC is version 5.1 which has
the above 'asm goto' miscompilation fixed, so remove the workaround.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/compiler-gcc.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index a0c55eeaeaf1..9b157b71036f 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -66,17 +66,6 @@
 		__builtin_unreachable();	\
 	} while (0)
 
-/*
- * GCC 'asm goto' miscompiles certain code sequences:
- *
- *   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
- *
- * Work it around via a compiler barrier quirk suggested by Jakub Jelinek.
- *
- * (asm goto is automatically volatile - the naming reflects this.)
- */
-#define asm_volatile_goto(x...)	do { asm goto(x); asm (""); } while (0)
-
 #if defined(CONFIG_ARCH_USE_BUILTIN_BSWAP)
 #define __HAVE_BUILTIN_BSWAP32__
 #define __HAVE_BUILTIN_BSWAP64__
-- 
2.35.3

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Naveen N. Rao]

Uros Bizjak wrote:
> The workaround for 'asm goto' miscompilation introduces a compiler
> barrier quirk that inhibits many useful compiler optimizations. For
> example, __try_cmpxchg_user compiles to:
> 
>    11375:	41 8b 4d 00          	mov    0x0(%r13),%ecx
>    11379:	41 8b 02             	mov    (%r10),%eax
>    1137c:	f0 0f b1 0a          	lock cmpxchg %ecx,(%rdx)
>    11380:	0f 94 c2             	sete   %dl
>    11383:	84 d2                	test   %dl,%dl
>    11385:	75 c4                	jne    1134b <...>
>    11387:	41 89 02             	mov    %eax,(%r10)
> 
> where the barrier inhibits flags propagation from asm when
> compiled with gcc-12.
> 
> When the mentioned quirk is removed, the following code is generated:
> 
>    11553:	41 8b 4d 00          	mov    0x0(%r13),%ecx
>    11557:	41 8b 02             	mov    (%r10),%eax
>    1155a:	f0 0f b1 0a          	lock cmpxchg %ecx,(%rdx)
>    1155e:	74 c9                	je     11529 <...>
>    11560:	41 89 02             	mov    %eax,(%r10)
> 
> The refered compiler bug:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
> 
> was fixed for gcc-4.8.2.
> 
> Current minimum required version of GCC is version 5.1 which has
> the above 'asm goto' miscompilation fixed, so remove the workaround.
> 
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  include/linux/compiler-gcc.h | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index a0c55eeaeaf1..9b157b71036f 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -66,17 +66,6 @@
>  		__builtin_unreachable();	\
>  	} while (0)
>  
> -/*
> - * GCC 'asm goto' miscompiles certain code sequences:
> - *
> - *   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
> - *
> - * Work it around via a compiler barrier quirk suggested by Jakub Jelinek.
> - *
> - * (asm goto is automatically volatile - the naming reflects this.)
> - */
> -#define asm_volatile_goto(x...)	do { asm goto(x); asm (""); } while (0)
> -
>  #if defined(CONFIG_ARCH_USE_BUILTIN_BSWAP)
>  #define __HAVE_BUILTIN_BSWAP32__
>  #define __HAVE_BUILTIN_BSWAP64__

This is causing a build issue on ppc64le with a new patch replacing use 
of unreachable() with __builtin_unreachable() in __WARN_FLAGS():
https://lore.kernel.org/linuxppc-dev/20220808114908.240813-2-sv@linux.ibm.com/

during RTL pass: combine
In file included from /linux/kernel/locking/rtmutex_api.c:9:
/linux/kernel/locking/rtmutex.c: In function '__rt_mutex_slowlock.constprop':
/linux/kernel/locking/rtmutex.c:1612:1: internal compiler error: in purge_dead_edges, at cfgrtl.c:3369
 1612 | }
      | ^
0x142817c internal_error(char const*, ...)
	???:0
0x5c8a1b fancy_abort(char const*, int, char const*)
	???:0
0x72017f purge_all_dead_edges()
	???:0
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.


So, it looks like gcc still has issues with certain uses of asm goto.


- Naveen

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Christophe Leroy]

Le 25/08/2022 à 12:30, Naveen N. Rao a écrit :
> Uros Bizjak wrote:
>> The workaround for 'asm goto' miscompilation introduces a compiler
>> barrier quirk that inhibits many useful compiler optimizations. For
>> example, __try_cmpxchg_user compiles to:
>>
>>    11375:    41 8b 4d 00              mov    0x0(%r13),%ecx
>>    11379:    41 8b 02                 mov    (%r10),%eax
>>    1137c:    f0 0f b1 0a              lock cmpxchg %ecx,(%rdx)
>>    11380:    0f 94 c2                 sete   %dl
>>    11383:    84 d2                    test   %dl,%dl
>>    11385:    75 c4                    jne    1134b <...>
>>    11387:    41 89 02                 mov    %eax,(%r10)
>>
>> where the barrier inhibits flags propagation from asm when
>> compiled with gcc-12.
>>
>> When the mentioned quirk is removed, the following code is generated:
>>
>>    11553:    41 8b 4d 00              mov    0x0(%r13),%ecx
>>    11557:    41 8b 02                 mov    (%r10),%eax
>>    1155a:    f0 0f b1 0a              lock cmpxchg %ecx,(%rdx)
>>    1155e:    74 c9                    je     11529 <...>
>>    11560:    41 89 02                 mov    %eax,(%r10)
>>
>> The refered compiler bug:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
>>
>> was fixed for gcc-4.8.2.
>>
>> Current minimum required version of GCC is version 5.1 which has
>> the above 'asm goto' miscompilation fixed, so remove the workaround.
>>
>> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>

It's unlikely that asm goto bug was completely fixed in 4.8 because it's 
been encountered with gcc 4.9.3 and 4.9.4, see commit 1344a232016d 
("powerpc: Use asm_goto_volatile for put_user()")

Christophe

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Uros Bizjak]

On Thu, Aug 25, 2022 at 12:30 PM Naveen N. Rao
<naveen.n.rao@linux.ibm.com> wrote:
>
> Uros Bizjak wrote:
> > The workaround for 'asm goto' miscompilation introduces a compiler
> > barrier quirk that inhibits many useful compiler optimizations. For
> > example, __try_cmpxchg_user compiles to:
> >
> >    11375:     41 8b 4d 00             mov    0x0(%r13),%ecx
> >    11379:     41 8b 02                mov    (%r10),%eax
> >    1137c:     f0 0f b1 0a             lock cmpxchg %ecx,(%rdx)
> >    11380:     0f 94 c2                sete   %dl
> >    11383:     84 d2                   test   %dl,%dl
> >    11385:     75 c4                   jne    1134b <...>
> >    11387:     41 89 02                mov    %eax,(%r10)
> >
> > where the barrier inhibits flags propagation from asm when
> > compiled with gcc-12.
> >
> > When the mentioned quirk is removed, the following code is generated:
> >
> >    11553:     41 8b 4d 00             mov    0x0(%r13),%ecx
> >    11557:     41 8b 02                mov    (%r10),%eax
> >    1155a:     f0 0f b1 0a             lock cmpxchg %ecx,(%rdx)
> >    1155e:     74 c9                   je     11529 <...>
> >    11560:     41 89 02                mov    %eax,(%r10)
> >
> > The refered compiler bug:
> >
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
> >
> > was fixed for gcc-4.8.2.
> >
> > Current minimum required version of GCC is version 5.1 which has
> > the above 'asm goto' miscompilation fixed, so remove the workaround.
> >
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  include/linux/compiler-gcc.h | 11 -----------
> >  1 file changed, 11 deletions(-)
> >
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index a0c55eeaeaf1..9b157b71036f 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -66,17 +66,6 @@
> >               __builtin_unreachable();        \
> >       } while (0)
> >
> > -/*
> > - * GCC 'asm goto' miscompiles certain code sequences:
> > - *
> > - *   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
> > - *
> > - * Work it around via a compiler barrier quirk suggested by Jakub Jelinek.
> > - *
> > - * (asm goto is automatically volatile - the naming reflects this.)
> > - */
> > -#define asm_volatile_goto(x...)      do { asm goto(x); asm (""); } while (0)
> > -
> >  #if defined(CONFIG_ARCH_USE_BUILTIN_BSWAP)
> >  #define __HAVE_BUILTIN_BSWAP32__
> >  #define __HAVE_BUILTIN_BSWAP64__
>
> This is causing a build issue on ppc64le with a new patch replacing use
> of unreachable() with __builtin_unreachable() in __WARN_FLAGS():
> https://lore.kernel.org/linuxppc-dev/20220808114908.240813-2-sv@linux.ibm.com/
>
> during RTL pass: combine
> In file included from /linux/kernel/locking/rtmutex_api.c:9:
> /linux/kernel/locking/rtmutex.c: In function '__rt_mutex_slowlock.constprop':
> /linux/kernel/locking/rtmutex.c:1612:1: internal compiler error: in purge_dead_edges, at cfgrtl.c:3369
>  1612 | }
>       | ^
> 0x142817c internal_error(char const*, ...)
>         ???:0
> 0x5c8a1b fancy_abort(char const*, int, char const*)
>         ???:0
> 0x72017f purge_all_dead_edges()
>         ???:0
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.
>
>
> So, it looks like gcc still has issues with certain uses of asm goto.

This looks like a powerpc specific bug, and has nothing to do with
PR58560 (asm goto miscompilation). If this is indeed a target specific
bug, it should be worked around in
arch/powerpc/include/asm/compiler.h, but please also report it to the
GCC bugzilla.

Uros.

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Segher Boessenkool]

Hi!

On Thu, Aug 25, 2022 at 04:00:52PM +0530, Naveen N. Rao wrote:
> This is causing a build issue on ppc64le with a new patch replacing use 
> of unreachable() with __builtin_unreachable() in __WARN_FLAGS():
> https://lore.kernel.org/linuxppc-dev/20220808114908.240813-2-sv@linux.ibm.com/

What is the compiler version?  If this is a GCC version that is still
supported, could you please open a PR?  <https://gcc.gnu.org/bugs.html>

> during RTL pass: combine
> In file included from /linux/kernel/locking/rtmutex_api.c:9:
> /linux/kernel/locking/rtmutex.c: In function 
> '__rt_mutex_slowlock.constprop':
> /linux/kernel/locking/rtmutex.c:1612:1: internal compiler error: in 
> purge_dead_edges, at cfgrtl.c:3369
> 1612 | }
>      | ^
> 0x142817c internal_error(char const*, ...)
> 	???:0
> 0x5c8a1b fancy_abort(char const*, int, char const*)
> 	???:0
> 0x72017f purge_all_dead_edges()
> 	???:0
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.

(For some reason your compiler does not show compiler source code file
names or line numbers.)

So it is GCC 11...  is it 11.3?  If not, please try with that.

> So, it looks like gcc still has issues with certain uses of asm goto.

Could be.  Please attach preprocessed code (or reduced code that shows
the same problem if you have that / can make that).  Thanks!


Segher

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Christophe Leroy]

Le 25/08/2022 à 20:08, Segher Boessenkool a écrit :
> Hi!
> 
> On Thu, Aug 25, 2022 at 04:00:52PM +0530, Naveen N. Rao wrote:
>> This is causing a build issue on ppc64le with a new patch replacing use
>> of unreachable() with __builtin_unreachable() in __WARN_FLAGS():
>> https://lore.kernel.org/linuxppc-dev/20220808114908.240813-2-sv@linux.ibm.com/
> 
> What is the compiler version?  If this is a GCC version that is still
> supported, could you please open a PR?  <https://gcc.gnu.org/bugs.html>
> 
>> during RTL pass: combine
>> In file included from /linux/kernel/locking/rtmutex_api.c:9:
>> /linux/kernel/locking/rtmutex.c: In function
>> '__rt_mutex_slowlock.constprop':
>> /linux/kernel/locking/rtmutex.c:1612:1: internal compiler error: in
>> purge_dead_edges, at cfgrtl.c:3369
>> 1612 | }
>>       | ^
>> 0x142817c internal_error(char const*, ...)
>> 	???:0
>> 0x5c8a1b fancy_abort(char const*, int, char const*)
>> 	???:0
>> 0x72017f purge_all_dead_edges()
>> 	???:0
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> Please include the complete backtrace with any bug report.
>> See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.
> 
> (For some reason your compiler does not show compiler source code file
> names or line numbers.)
> 
> So it is GCC 11...  is it 11.3?  If not, please try with that.

With gcc 11.3 I get:

   CC      kernel/locking/rtmutex_api.o
during RTL pass: combine
In file included from kernel/locking/rtmutex_api.c:9:
kernel/locking/rtmutex.c: In function '__rt_mutex_slowlock.constprop':
kernel/locking/rtmutex.c:1612:1: internal compiler error: in 
purge_dead_edges, at cfgrtl.c:3369
  1612 | }
       | ^
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.
make[2]: *** [scripts/Makefile.build:249 : kernel/locking/rtmutex_api.o] 
Erreur 1
make[1]: *** [scripts/Makefile.build:465 : kernel/locking] Erreur 2
make: *** [Makefile:1857 : kernel] Erreur 2


With gcc 12.2 I get:

   CC      kernel/locking/rtmutex_api.o
during RTL pass: combine
In file included from kernel/locking/rtmutex_api.c:9:
kernel/locking/rtmutex.c: In function '__rt_mutex_slowlock.constprop':
kernel/locking/rtmutex.c:1612:1: internal compiler error: in 
purge_dead_edges, at cfgrtl.cc:3347
  1612 | }
       | ^
Please submit a full bug report, with preprocessed source (by using 
-freport-bug).
See <https://gcc.gnu.org/bugs/> for instructions.
make[2]: *** [scripts/Makefile.build:249 : kernel/locking/rtmutex_api.o] 
Erreur 1
make[1]: *** [scripts/Makefile.build:465 : kernel/locking] Erreur 2
make: *** [Makefile:1857 : kernel] Erreur 2


> 
>> So, it looks like gcc still has issues with certain uses of asm goto.
> 
> Could be.  Please attach preprocessed code (or reduced code that shows
> the same problem if you have that / can make that).  Thanks!
> 
> 
> Segher

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Christophe Leroy]

Le 26/08/2022 à 09:02, Christophe Leroy a écrit :
> 
> 
> Le 25/08/2022 à 20:08, Segher Boessenkool a écrit :
>> Hi!
>>
>> On Thu, Aug 25, 2022 at 04:00:52PM +0530, Naveen N. Rao wrote:
>>> This is causing a build issue on ppc64le with a new patch replacing use
>>> of unreachable() with __builtin_unreachable() in __WARN_FLAGS():
>>> https://lore.kernel.org/linuxppc-dev/20220808114908.240813-2-sv@linux.ibm.com/
>>
>> What is the compiler version?  If this is a GCC version that is still
>> supported, could you please open a PR?  <https://gcc.gnu.org/bugs.html>
>>
>>> during RTL pass: combine
>>> In file included from /linux/kernel/locking/rtmutex_api.c:9:
>>> /linux/kernel/locking/rtmutex.c: In function
>>> '__rt_mutex_slowlock.constprop':
>>> /linux/kernel/locking/rtmutex.c:1612:1: internal compiler error: in
>>> purge_dead_edges, at cfgrtl.c:3369
>>> 1612 | }
>>>        | ^
>>> 0x142817c internal_error(char const*, ...)
>>> 	???:0
>>> 0x5c8a1b fancy_abort(char const*, int, char const*)
>>> 	???:0
>>> 0x72017f purge_all_dead_edges()
>>> 	???:0
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> Please include the complete backtrace with any bug report.
>>> See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.
>>
>> (For some reason your compiler does not show compiler source code file
>> names or line numbers.)
>>
>> So it is GCC 11...  is it 11.3?  If not, please try with that.
> 
> With gcc 11.3 I get:
> 
>     CC      kernel/locking/rtmutex_api.o
> during RTL pass: combine
> In file included from kernel/locking/rtmutex_api.c:9:
> kernel/locking/rtmutex.c: In function '__rt_mutex_slowlock.constprop':
> kernel/locking/rtmutex.c:1612:1: internal compiler error: in
> purge_dead_edges, at cfgrtl.c:3369
>    1612 | }
>         | ^
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <https://gcc.gnu.org/bugs/> for instructions.
> make[2]: *** [scripts/Makefile.build:249 : kernel/locking/rtmutex_api.o]
> Erreur 1
> make[1]: *** [scripts/Makefile.build:465 : kernel/locking] Erreur 2
> make: *** [Makefile:1857 : kernel] Erreur 2
> 
> 
> With gcc 12.2 I get:
> 
>     CC      kernel/locking/rtmutex_api.o
> during RTL pass: combine
> In file included from kernel/locking/rtmutex_api.c:9:
> kernel/locking/rtmutex.c: In function '__rt_mutex_slowlock.constprop':
> kernel/locking/rtmutex.c:1612:1: internal compiler error: in
> purge_dead_edges, at cfgrtl.cc:3347
>    1612 | }
>         | ^
> Please submit a full bug report, with preprocessed source (by using
> -freport-bug).
> See <https://gcc.gnu.org/bugs/> for instructions.
> make[2]: *** [scripts/Makefile.build:249 : kernel/locking/rtmutex_api.o]
> Erreur 1
> make[1]: *** [scripts/Makefile.build:465 : kernel/locking] Erreur 2
> make: *** [Makefile:1857 : kernel] Erreur 2
> 


Same problem with 10.3, 9.5, 8.5 and 5.5

> 
>>
>>> So, it looks like gcc still has issues with certain uses of asm goto.
>>
>> Could be.  Please attach preprocessed code (or reduced code that shows
>> the same problem if you have that / can make that).  Thanks!
>>
>>
>> Segher

Re: [PATCH] compiler-gcc.h: Remove ancient workaround for gcc PR 58670

[Christophe Leroy]

Le 25/08/2022 à 20:08, Segher Boessenkool a écrit :
> Hi!
> 
> On Thu, Aug 25, 2022 at 04:00:52PM +0530, Naveen N. Rao wrote:
>> This is causing a build issue on ppc64le with a new patch replacing use
>> of unreachable() with __builtin_unreachable() in __WARN_FLAGS():
>> https://lore.kernel.org/linuxppc-dev/20220808114908.240813-2-sv@linux.ibm.com/
> 
> What is the compiler version?  If this is a GCC version that is still
> supported, could you please open a PR?  <https://gcc.gnu.org/bugs.html>
> 
>> during RTL pass: combine
>> In file included from /linux/kernel/locking/rtmutex_api.c:9:
>> /linux/kernel/locking/rtmutex.c: In function
>> '__rt_mutex_slowlock.constprop':
>> /linux/kernel/locking/rtmutex.c:1612:1: internal compiler error: in
>> purge_dead_edges, at cfgrtl.c:3369
>> 1612 | }
>>       | ^
>> 0x142817c internal_error(char const*, ...)
>> 	???:0
>> 0x5c8a1b fancy_abort(char const*, int, char const*)
>> 	???:0
>> 0x72017f purge_all_dead_edges()
>> 	???:0
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> Please include the complete backtrace with any bug report.
>> See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.
> 
> (For some reason your compiler does not show compiler source code file
> names or line numbers.)
> 
> So it is GCC 11...  is it 11.3?  If not, please try with that.
> 
>> So, it looks like gcc still has issues with certain uses of asm goto.
> 
> Could be.  Please attach preprocessed code (or reduced code that shows
> the same problem if you have that / can make that).  Thanks!
> 

Opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106751

Christophe