| Message ID | 20190408212648.2407234-10-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | [01/12] s390: remove -fno-strength-reduce flag | expand |
On Mon, 8 Apr 2019 23:26:23 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > llvm on s390 has problems with __builtin_return_address(n), with n>0, > this results in a somewhat cryptic error message: > > fatal error: error in backend: Unsupported stack frame traversal count > > To work around it, use the direct return address directly. This > is probably not ideal here, but gets things to compile and should > only lead to inferior reporting, not to misbehavior of the generated > code. > > Link: https://bugs.llvm.org/show_bug.cgi?id=41424 > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/s390/include/asm/ftrace.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index 5a3c95b11952..7923c63946fb 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -13,7 +13,12 @@ > > #ifndef __ASSEMBLY__ > > +#ifdef CONFIG_CC_IS_CLANG > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */ > +#define ftrace_return_address(n) __builtin_return_address(0) > +#else > #define ftrace_return_address(n) __builtin_return_address(n) > +#endif > > void _mcount(void); > void ftrace_caller(void); I can say I like this one. If the compiler can not do __builtin_return_address(n) it feels wrong to just use __builtin_return_address(0). -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote: > > --- a/arch/s390/include/asm/ftrace.h > > +++ b/arch/s390/include/asm/ftrace.h > > @@ -13,7 +13,12 @@ > > > > #ifndef __ASSEMBLY__ > > > > +#ifdef CONFIG_CC_IS_CLANG > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */ > > +#define ftrace_return_address(n) __builtin_return_address(0) > > +#else > > #define ftrace_return_address(n) __builtin_return_address(n) > > +#endif > > > > void _mcount(void); > > void ftrace_caller(void); > > I can say I like this one. If the compiler can not do __builtin_return_address(n) > it feels wrong to just use __builtin_return_address(0). I agree. The proper return value is 0UL, see include/linux/ftrace.h /* Archs may use other ways for ADDR1 and beyond */ #ifndef ftrace_return_address # ifdef CONFIG_FRAME_POINTER # define ftrace_return_address(n) __builtin_return_address(n) # else # define ftrace_return_address(n) 0UL # endif #endif This is why we treat zero differently: #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0) #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1)) #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2)) #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3)) #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4)) #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5)) #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6)) -- Steve
On Wed, Apr 10, 2019 at 6:14 PM Steven Rostedt <rostedt@goodmis.org> wrote: > On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote: > > > > --- a/arch/s390/include/asm/ftrace.h > > > +++ b/arch/s390/include/asm/ftrace.h > > > @@ -13,7 +13,12 @@ > > > > > > #ifndef __ASSEMBLY__ > > > > > > +#ifdef CONFIG_CC_IS_CLANG > > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */ > > > +#define ftrace_return_address(n) __builtin_return_address(0) > > > +#else > > > #define ftrace_return_address(n) __builtin_return_address(n) > > > +#endif > > > > > > void _mcount(void); > > > void ftrace_caller(void); > > > > I can say I like this one. If the compiler can not do __builtin_return_address(n) > > it feels wrong to just use __builtin_return_address(0). > > I agree. The proper return value is 0UL, see include/linux/ftrace.h > > /* Archs may use other ways for ADDR1 and beyond */ > #ifndef ftrace_return_address > # ifdef CONFIG_FRAME_POINTER > # define ftrace_return_address(n) __builtin_return_address(n) > # else > # define ftrace_return_address(n) 0UL > # endif > #endif > > This is why we treat zero differently: > > #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0) > #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1)) > #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2)) > #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3)) > #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4)) > #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5)) > #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6)) Right, got it. Martin, do you want me to send a replacement patch, or can you commit the patch with #ifdef CONFIG_CC_IS_CLANG /* https://bugs.llvm.org/show_bug.cgi?id=41424 */ #define ftrace_return_address(n) 0UL #else #define ftrace_return_address(n) __builtin_return_address(n) #endif instead? Arnd
On Wed, 10 Apr 2019 21:07:56 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > On Wed, Apr 10, 2019 at 6:14 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 10 Apr 2019 18:03:57 +0200 Martin Schwidefsky <schwidefsky@de.ibm.com> wrote: > > > > > > --- a/arch/s390/include/asm/ftrace.h > > > > +++ b/arch/s390/include/asm/ftrace.h > > > > @@ -13,7 +13,12 @@ > > > > > > > > #ifndef __ASSEMBLY__ > > > > > > > > +#ifdef CONFIG_CC_IS_CLANG > > > > +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */ > > > > +#define ftrace_return_address(n) __builtin_return_address(0) > > > > +#else > > > > #define ftrace_return_address(n) __builtin_return_address(n) > > > > +#endif > > > > > > > > void _mcount(void); > > > > void ftrace_caller(void); > > > > > > I can say I like this one. If the compiler can not do __builtin_return_address(n) > > > it feels wrong to just use __builtin_return_address(0). > > > > I agree. The proper return value is 0UL, see include/linux/ftrace.h > > > > /* Archs may use other ways for ADDR1 and beyond */ > > #ifndef ftrace_return_address > > # ifdef CONFIG_FRAME_POINTER > > # define ftrace_return_address(n) __builtin_return_address(n) > > # else > > # define ftrace_return_address(n) 0UL > > # endif > > #endif > > > > This is why we treat zero differently: > > > > #define CALLER_ADDR0 ((unsigned long)ftrace_return_address0) > > #define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1)) > > #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2)) > > #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3)) > > #define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4)) > > #define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5)) > > #define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6)) > > Right, got it. > > Martin, do you want me to send a replacement patch, or can you > commit the patch with > > #ifdef CONFIG_CC_IS_CLANG > /* https://bugs.llvm.org/show_bug.cgi?id=41424 */ > #define ftrace_return_address(n) 0UL > #else > #define ftrace_return_address(n) __builtin_return_address(n) > #endif > > instead? Ok, done. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h index 5a3c95b11952..7923c63946fb 100644 --- a/arch/s390/include/asm/ftrace.h +++ b/arch/s390/include/asm/ftrace.h @@ -13,7 +13,12 @@ #ifndef __ASSEMBLY__ +#ifdef CONFIG_CC_IS_CLANG +/* https://bugs.llvm.org/show_bug.cgi?id=41424 */ +#define ftrace_return_address(n) __builtin_return_address(0) +#else #define ftrace_return_address(n) __builtin_return_address(n) +#endif void _mcount(void); void ftrace_caller(void);
llvm on s390 has problems with __builtin_return_address(n), with n>0, this results in a somewhat cryptic error message: fatal error: error in backend: Unsupported stack frame traversal count To work around it, use the direct return address directly. This is probably not ideal here, but gets things to compile and should only lead to inferior reporting, not to misbehavior of the generated code. Link: https://bugs.llvm.org/show_bug.cgi?id=41424 Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/s390/include/asm/ftrace.h | 5 +++++ 1 file changed, 5 insertions(+) -- 2.20.0