linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
To: Huang Pei <huangpei@loongson.cn>
Cc: ambrosehua@gmail.com, Bibo Mao <maobibo@loongson.cn>,
	linux-mips@vger.kernel.org, Jiaxun Yang <jiaxun.yang@flygoat.com>,
	Paul Burton <paulburton@kernel.org>,
	Li Xuefeng <lixuefeng@loongson.cn>,
	Yang Tiezhu <yangtiezhu@loongson.cn>,
	Gao Juxin <gaojuxin@loongson.cn>,
	Huacai Chen <chenhuacai@loongson.cn>
Subject: Re: [PATCH 1/4] MIPS: fix local_{add,sub}_return on MIPS64
Date: Wed, 5 Jan 2022 11:34:48 +0100	[thread overview]
Message-ID: <20220105103448.GA7009@alpha.franken.de> (raw)
In-Reply-To: <20211218032312.4lwoo2moxptw2hcq@loongson-pc>

On Sat, Dec 18, 2021 at 11:23:12AM +0800, Huang Pei wrote:
> On Thu, Dec 16, 2021 at 01:49:48PM +0100, Thomas Bogendoerfer wrote:
> > On Wed, Dec 15, 2021 at 04:44:57PM +0800, Huang Pei wrote:
> > > Use "daddu/dsubu" for long int on MIPS64 instead of "addu/subu"
> > > 
> > > Fixes: 7232311ef14c ("local_t: mips extension")
> > > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > > ---
> > >  arch/mips/include/asm/llsc.h  | 4 ++++
> > >  arch/mips/include/asm/local.h | 8 ++++----
> > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/mips/include/asm/llsc.h b/arch/mips/include/asm/llsc.h
> > > index ec09fe5d6d6c..8cc28177c37f 100644
> > > --- a/arch/mips/include/asm/llsc.h
> > > +++ b/arch/mips/include/asm/llsc.h
> > > @@ -14,10 +14,14 @@
> > >  #if _MIPS_SZLONG == 32
> > >  #define __LL		"ll	"
> > >  #define __SC		"sc	"
> > > +#define __ADDU		"addu	"
> > > +#define __SUBU		"subu	"
> > >  #define __INS		"ins	"
> > >  #define __EXT		"ext	"
> > >  #elif _MIPS_SZLONG == 64
> > >  #define __LL		"lld	"
> > > +#define __ADDU		"daddu	"
> > > +#define __SUBU		"dsubu	"
> > >  #define __SC		"scd	"
> > >  #define __INS		"dins	"
> > >  #define __EXT		"dext	"
> > 
> > maybe I wasn't clear enough, I don't want your orginal fix, but use
> > fix patch using __stringify(LONG_ADDU)/__stringify(LONG_SUBU).
> > 
> > Thomas.
> > 
> My point is to keep code style in consistency. If you insist, you can
> fix it by yourself. It is ok, I don't mind.

sorry for causing such frustration on your side.

I've applied

diff --git a/arch/mips/include/asm/local.h b/arch/mips/include/asm/local.h
index ecda7295ddcd..3fa634090388 100644
--- a/arch/mips/include/asm/local.h
+++ b/arch/mips/include/asm/local.h
@@ -5,6 +5,7 @@
 #include <linux/percpu.h>
 #include <linux/bitops.h>
 #include <linux/atomic.h>
+#include <asm/asm.h>
 #include <asm/cmpxchg.h>
 #include <asm/compiler.h>
 #include <asm/war.h>
@@ -39,7 +40,7 @@ static __inline__ long local_add_return(long i, local_t * l)
                "       .set    arch=r4000                              \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_add_return      \n"
-               "       addu    %0, %1, %3                              \n"
+                       __stringify(LONG_ADDU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqzl   %0, 1b                                  \n"
                "       addu    %0, %1, %3                              \n"
@@ -55,7 +56,7 @@ static __inline__ long local_add_return(long i, local_t * l)
                "       .set    "MIPS_ISA_ARCH_LEVEL"                   \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_add_return      \n"
-               "       addu    %0, %1, %3                              \n"
+                       __stringify(LONG_ADDU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqz    %0, 1b                                  \n"
                "       addu    %0, %1, %3                              \n"
@@ -88,7 +89,7 @@ static __inline__ long local_sub_return(long i, local_t * l)
                "       .set    arch=r4000                              \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_sub_return      \n"
-               "       subu    %0, %1, %3                              \n"
+                       __stringify(LONG_SUBU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqzl   %0, 1b                                  \n"
                "       subu    %0, %1, %3                              \n"
@@ -104,7 +105,7 @@ static __inline__ long local_sub_return(long i, local_t * l)
                "       .set    "MIPS_ISA_ARCH_LEVEL"                   \n"
                        __SYNC(full, loongson3_war) "                   \n"
                "1:"    __LL    "%1, %2         # local_sub_return      \n"
-               "       subu    %0, %1, %3                              \n"
+                       __stringify(LONG_SUBU)  "       %0, %1, %3      \n"
                        __SC    "%0, %2                                 \n"
                "       beqz    %0, 1b                                  \n"
                "       subu    %0, %1, %3                              \n"

and the reason is I prefer to keep the changes as small and local 
as possible. This makes the reviews and applying to older trees easier.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

  reply	other threads:[~2022-01-05 10:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-15  8:44 [PATCH V6]: bugfix Huang Pei
2021-12-15  8:44 ` [PATCH 1/4] MIPS: fix local_{add,sub}_return on MIPS64 Huang Pei
2021-12-16 12:49   ` Thomas Bogendoerfer
2021-12-18  3:23     ` Huang Pei
2022-01-05 10:34       ` Thomas Bogendoerfer [this message]
2021-12-15  8:44 ` [PATCH 2/4] MIPS: tx39: adjust tx39_flush_cache_page Huang Pei
2021-12-16  8:29   ` Sergey Shtylyov
2021-12-16 12:52   ` Thomas Bogendoerfer
2021-12-18  3:30     ` Huang Pei
2021-12-15  8:44 ` [PATCH 3/4] MIPS: rework local_t operation on MIPS64 Huang Pei
2022-01-05 10:35   ` Thomas Bogendoerfer
2021-12-15  8:45 ` [PATCH 4/4] MIPS: retire "asm/llsc.h" Huang Pei
2022-01-05 10:39   ` Thomas Bogendoerfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220105103448.GA7009@alpha.franken.de \
    --to=tsbogend@alpha.franken.de \
    --cc=ambrosehua@gmail.com \
    --cc=chenhuacai@loongson.cn \
    --cc=gaojuxin@loongson.cn \
    --cc=huangpei@loongson.cn \
    --cc=jiaxun.yang@flygoat.com \
    --cc=linux-mips@vger.kernel.org \
    --cc=lixuefeng@loongson.cn \
    --cc=maobibo@loongson.cn \
    --cc=paulburton@kernel.org \
    --cc=yangtiezhu@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).