Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230426124948.1072216-1-zhanggenjian@kylinos.cn>
 <CAOd03yQ98st7KvAkwfFqFvXGBdmWmOZZ-kWAH5DAi7gLd=ngaA@mail.gmail.com> <17660b99-d68d-328e-5fe6-c011709fa3e4@infradead.org>
In-Reply-To: <17660b99-d68d-328e-5fe6-c011709fa3e4@infradead.org>
From:   genjian zhang <zhanggenjian123@gmail.com>
Date:   Thu, 27 Apr 2023 15:18:56 +0800
Message-ID: <CAOd03yRAmPULzi8fYTnukCpRPHa1Zsv5ZFFeQRcQ7B1abQCGqA@mail.gmail.com>
Subject: Re: [PATCH] fix ___SYNC () build error when PROFILE_ALL_BRANCHES is enabled
To:     Randy Dunlap <rdunlap@infradead.org>
Cc:     tsbogend@alpha.franken.de, linux-mips@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Genjian Zhang <zhanggenjian@kylinos.cn>,
        k2ci <kernel-bot@kylinos.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Apr 27, 2023 at 11:16=E2=80=AFAM Randy Dunlap <rdunlap@infradead.or=
g> 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=E2=80=AFPM 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" statemen=
t
> >> {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 ) !=3D -1) && ( _reason );               \
> >> +       ((_type) !=3D -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 in=
sert them
> >>   * into assembly code.
> >> + * In addition,=E2=80=98if=E2=80=99 can not be substituted when CONFI=
G_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_B=
RANCHES
> 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.