| Message ID | 20230426124948.1072216-1-zhanggenjian@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix ___SYNC () build error when PROFILE_ALL_BRANCHES is enabled | expand |
On Wed, Apr 26, 2023 at 8:51 PM Genjian <zhanggenjian123@gmail.com> wrote: > > From: Genjian Zhang <zhanggenjian@kylinos.cn> > > compiler error (mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110): > > {standard input}: Assembler messages: > {standard input}:171: Error: found '(', expected: ')' > {standard input}:171: Error: found '(', expected: ')' > {standard input}:171: Error: non-constant expression in ".if" statement > {standard input}:171: Error: junk at end of line, first unrecognized > character is `(' > > Preprocessor expand ___SYNC () macros.However,'if' will be wrongly > replaced by C code when PROFILE_ALL_BRANCHES is enabled and ___SYNC > is used in inline assembly.This leads to syntax errors in the code. > Compilers report a lot of errors like the above. > Move '.if' into quoted strings to fix it. > > Reported-by: k2ci <kernel-bot@kylinos.cn> > Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn> > --- > arch/mips/include/asm/sync.h | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/arch/mips/include/asm/sync.h b/arch/mips/include/asm/sync.h > index aabd097933fe..d9f5a87424e7 100644 > --- a/arch/mips/include/asm/sync.h > +++ b/arch/mips/include/asm/sync.h > @@ -175,7 +175,7 @@ > */ > #ifdef CONFIG_CPU_HAS_SYNC > # define ____SYNC(_type, _reason, _else) \ > - .if (( _type ) != -1) && ( _reason ); \ > + ((_type) != -1) && (_reason); \ > .set push; \ > .set MIPS_ISA_LEVEL_RAW; \ > .rept __SYNC_rpt(_type); \ > @@ -192,13 +192,15 @@ > /* > * Preprocessor magic to expand macros used as arguments before we insert them > * into assembly code. > + * In addition,‘if’ can not be substituted when CONFIG_PROFILE_ALL_BRANCHES is > + * enabled. > */ > #ifdef __ASSEMBLY__ > # define ___SYNC(type, reason, else) \ > - ____SYNC(type, reason, else) > + .if ____SYNC(type, reason, else) > #else > # define ___SYNC(type, reason, else) \ > - __stringify(____SYNC(type, reason, else)) > + ".if" __stringify(____SYNC(type, reason, else)) > #endif > > #define __SYNC(type, reason) \ > -- > 2.25.1 > Maybe the readability of this modification is not very good. Any comments and suggestions are welcome. Thanks.
Hi-- > > Maybe the readability of this modification is not very good. Any > comments and suggestions are welcome. Thanks. Some of the patch description could use some cleaning up, then I have a question. On 4/26/23 06:02, genjian zhang wrote: > On Wed, Apr 26, 2023 at 8:51 PM Genjian <zhanggenjian123@gmail.com> wrote: >> >> From: Genjian Zhang <zhanggenjian@kylinos.cn> >> >> compiler error (mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110): >> >> {standard input}: Assembler messages: >> {standard input}:171: Error: found '(', expected: ')' >> {standard input}:171: Error: found '(', expected: ')' >> {standard input}:171: Error: non-constant expression in ".if" statement >> {standard input}:171: Error: junk at end of line, first unrecognized >> character is `(' >> >> Preprocessor expand ___SYNC () macros.However,'if' will be wrongly expands ___SYNC() macros. However, 'if' will be wrongly >> replaced by C code when PROFILE_ALL_BRANCHES is enabled and ___SYNC >> is used in inline assembly.This leads to syntax errors in the code. assembly. This >> Compilers report a lot of errors like the above. >> Move '.if' into quoted strings to fix it. >> >> Reported-by: k2ci <kernel-bot@kylinos.cn> >> Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn> >> --- >> arch/mips/include/asm/sync.h | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/arch/mips/include/asm/sync.h b/arch/mips/include/asm/sync.h >> index aabd097933fe..d9f5a87424e7 100644 >> --- a/arch/mips/include/asm/sync.h >> +++ b/arch/mips/include/asm/sync.h >> @@ -175,7 +175,7 @@ >> */ >> #ifdef CONFIG_CPU_HAS_SYNC >> # define ____SYNC(_type, _reason, _else) \ >> - .if (( _type ) != -1) && ( _reason ); \ >> + ((_type) != -1) && (_reason); \ >> .set push; \ >> .set MIPS_ISA_LEVEL_RAW; \ >> .rept __SYNC_rpt(_type); \ >> @@ -192,13 +192,15 @@ >> /* >> * Preprocessor magic to expand macros used as arguments before we insert them >> * into assembly code. >> + * In addition,‘if’ can not be substituted when CONFIG_PROFILE_ALL_BRANCHES is * In addition, 'if' cannot be >> + * enabled. >> */ >> #ifdef __ASSEMBLY__ >> # define ___SYNC(type, reason, else) \ >> - ____SYNC(type, reason, else) >> + .if ____SYNC(type, reason, else) >> #else >> # define ___SYNC(type, reason, else) \ >> - __stringify(____SYNC(type, reason, else)) >> + ".if" __stringify(____SYNC(type, reason, else)) >> #endif >> >> #define __SYNC(type, reason) \ >> -- >> 2.25.1 Is this problem that you are fixing being aggravated (caused) by the #define of if() in include/linux/compiler.h when CONFIG_PROFILE_ALL_BRANCHES is set? I suspect that it is. It wouldn't hurt to mention that (if I am correct). Thanks.
On Thu, Apr 27, 2023 at 11:16 AM Randy Dunlap <rdunlap@infradead.org> wrote: > > Hi-- > > > > > Maybe the readability of this modification is not very good. Any > > comments and suggestions are welcome. Thanks. > > Some of the patch description could use some cleaning up, > then I have a question. > > > On 4/26/23 06:02, genjian zhang wrote: > > On Wed, Apr 26, 2023 at 8:51 PM Genjian <zhanggenjian123@gmail.com> wrote: > >> > >> From: Genjian Zhang <zhanggenjian@kylinos.cn> > >> > >> compiler error (mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110): > >> > >> {standard input}: Assembler messages: > >> {standard input}:171: Error: found '(', expected: ')' > >> {standard input}:171: Error: found '(', expected: ')' > >> {standard input}:171: Error: non-constant expression in ".if" statement > >> {standard input}:171: Error: junk at end of line, first unrecognized > >> character is `(' > >> > >> Preprocessor expand ___SYNC () macros.However,'if' will be wrongly > > expands ___SYNC() macros. However, 'if' will be wrongly > > >> replaced by C code when PROFILE_ALL_BRANCHES is enabled and ___SYNC > >> is used in inline assembly.This leads to syntax errors in the code. > > assembly. This > > >> Compilers report a lot of errors like the above. > >> Move '.if' into quoted strings to fix it. > >> > >> Reported-by: k2ci <kernel-bot@kylinos.cn> > >> Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn> > >> --- > >> arch/mips/include/asm/sync.h | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/arch/mips/include/asm/sync.h b/arch/mips/include/asm/sync.h > >> index aabd097933fe..d9f5a87424e7 100644 > >> --- a/arch/mips/include/asm/sync.h > >> +++ b/arch/mips/include/asm/sync.h > >> @@ -175,7 +175,7 @@ > >> */ > >> #ifdef CONFIG_CPU_HAS_SYNC > >> # define ____SYNC(_type, _reason, _else) \ > >> - .if (( _type ) != -1) && ( _reason ); \ > >> + ((_type) != -1) && (_reason); \ > >> .set push; \ > >> .set MIPS_ISA_LEVEL_RAW; \ > >> .rept __SYNC_rpt(_type); \ > >> @@ -192,13 +192,15 @@ > >> /* > >> * Preprocessor magic to expand macros used as arguments before we insert them > >> * into assembly code. > >> + * In addition,‘if’ can not be substituted when CONFIG_PROFILE_ALL_BRANCHES is > > * In addition, 'if' cannot be > > >> + * enabled. > >> */ > >> #ifdef __ASSEMBLY__ > >> # define ___SYNC(type, reason, else) \ > >> - ____SYNC(type, reason, else) > >> + .if ____SYNC(type, reason, else) > >> #else > >> # define ___SYNC(type, reason, else) \ > >> - __stringify(____SYNC(type, reason, else)) > >> + ".if" __stringify(____SYNC(type, reason, else)) > >> #endif > >> > >> #define __SYNC(type, reason) \ > >> -- > >> 2.25.1 > > > Is this problem that you are fixing being aggravated (caused) by > the #define of if() in include/linux/compiler.h when CONFIG_PROFILE_ALL_BRANCHES > is set? I suspect that it is. right . > It wouldn't hurt to mention that (if I am correct). > > Thanks. > -- > ~Randy Thank you for your suggestions. I will send a patch v2.
diff --git a/arch/mips/include/asm/sync.h b/arch/mips/include/asm/sync.h index aabd097933fe..d9f5a87424e7 100644 --- a/arch/mips/include/asm/sync.h +++ b/arch/mips/include/asm/sync.h @@ -175,7 +175,7 @@ */ #ifdef CONFIG_CPU_HAS_SYNC # define ____SYNC(_type, _reason, _else) \ - .if (( _type ) != -1) && ( _reason ); \ + ((_type) != -1) && (_reason); \ .set push; \ .set MIPS_ISA_LEVEL_RAW; \ .rept __SYNC_rpt(_type); \ @@ -192,13 +192,15 @@ /* * Preprocessor magic to expand macros used as arguments before we insert them * into assembly code. + * In addition,‘if’ can not be substituted when CONFIG_PROFILE_ALL_BRANCHES is + * enabled. */ #ifdef __ASSEMBLY__ # define ___SYNC(type, reason, else) \ - ____SYNC(type, reason, else) + .if ____SYNC(type, reason, else) #else # define ___SYNC(type, reason, else) \ - __stringify(____SYNC(type, reason, else)) + ".if" __stringify(____SYNC(type, reason, else)) #endif #define __SYNC(type, reason) \