diff mbox series

MIPS: fix loongson3 llsc workaround

Message ID 20230323105959.1449936-1-yunqiang.su@cipunited.com
State Accepted
Headers
Series MIPS: fix loongson3 llsc workaround |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

YunQiang Su March 23, 2023, 10:59 a.m. UTC 
  -mfix-looongson3-llsc may add sync instructions not needed on some
asm code with lots of debug info.

	PR: 30153
	* gas/config/tc-mips.c(fix_loongson3_llsc): clear logistic.
---
 gas/config/tc-mips.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

YunQiang Su April 10, 2023, 6:37 a.m. UTC | #1 
ping

YunQiang Su <yunqiang.su@cipunited.com> 于2023年3月23日周四 19:00写道：
>
> -mfix-looongson3-llsc may add sync instructions not needed on some
> asm code with lots of debug info.
>
>         PR: 30153
>         * gas/config/tc-mips.c(fix_loongson3_llsc): clear logistic.
> ---
>  gas/config/tc-mips.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
> index e911aaa904a..8a970ceada2 100644
> --- a/gas/config/tc-mips.c
> +++ b/gas/config/tc-mips.c
> @@ -6942,10 +6942,6 @@ fix_loongson3_llsc (struct mips_cl_insn * ip)
>        unsigned long lookback = ARRAY_SIZE (history);
>        for (i = 0; i < lookback; i++)
>         {
> -         if (streq (history[i].insn_mo->name, "ll")
> -             || streq (history[i].insn_mo->name, "lld"))
> -           break;
> -
>           if (streq (history[i].insn_mo->name, "sc")
>               || streq (history[i].insn_mo->name, "scd"))
>             {
> @@ -6953,8 +6949,8 @@ fix_loongson3_llsc (struct mips_cl_insn * ip)
>
>               for (j = i + 1; j < lookback; j++)
>                 {
> -                 if (streq (history[i].insn_mo->name, "ll")
> -                     || streq (history[i].insn_mo->name, "lld"))
> +                 if (streq (history[j].insn_mo->name, "ll")
> +                     || streq (history[j].insn_mo->name, "lld"))
>                     break;
>
>                   if (delayed_branch_p (&history[j]))
> @@ -6993,7 +6989,7 @@ fix_loongson3_llsc (struct mips_cl_insn * ip)
>               for (j = i + 1; j < lookback; j++)
>                 {
>                   if (streq (history[j].insn_mo->name, "ll")
> -                     || streq (history[i].insn_mo->name, "lld"))
> +                     || streq (history[j].insn_mo->name, "lld"))
>                     break;
>                 }
>
> --
> 2.30.2
>
Maciej W. Rozycki May 11, 2023, midnight UTC | #2 
On Thu, 23 Mar 2023, YunQiang Su wrote:

> -mfix-looongson3-llsc may add sync instructions not needed on some
> asm code with lots of debug info.

 I can see the change has been committed, but who has actually reviewed 
it?

 The change description doesn't say what the change actually does, so one 
can't say whether it is correct or not.  At least an example of incorrect 
code produced ought to be shown and how the change affects it.  As it 
stands I have no idea what is going on here, and surely no one who looks 
at it in a few year's time will.

 Change descriptions cannot be retrofitted, so I think the original fix 
ought to be reverted and, assuming it is indeed the correct one, reapplied 
with a correct change description (after a proper review).

  Maciej
YunQiang Su May 11, 2023, 12:15 a.m. UTC | #3 
Maciej W. Rozycki <macro@orcam.me.uk> 于2023年5月11日周四 08:01写道：
>
> On Thu, 23 Mar 2023, YunQiang Su wrote:
>
> > -mfix-looongson3-llsc may add sync instructions not needed on some
> > asm code with lots of debug info.
>
>  I can see the change has been committed, but who has actually reviewed
> it?
>
>  The change description doesn't say what the change actually does, so one
> can't say whether it is correct or not.  At least an example of incorrect
> code produced ought to be shown and how the change affects it.  As it
> stands I have no idea what is going on here, and surely no one who looks
> at it in a few year's time will.
>

In fact, it was a typo of i/j.

>  Change descriptions cannot be retrofitted, so I think the original fix
> ought to be reverted and, assuming it is indeed the correct one, reapplied
> with a correct change description (after a proper review).
>
>   Maciej
diff mbox series

Patch

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index e911aaa904a..8a970ceada2 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -6942,10 +6942,6 @@  fix_loongson3_llsc (struct mips_cl_insn * ip)
       unsigned long lookback = ARRAY_SIZE (history);
       for (i = 0; i < lookback; i++)
 	{
-	  if (streq (history[i].insn_mo->name, "ll")
-	      || streq (history[i].insn_mo->name, "lld"))
-	    break;
-
 	  if (streq (history[i].insn_mo->name, "sc")
 	      || streq (history[i].insn_mo->name, "scd"))
 	    {
@@ -6953,8 +6949,8 @@  fix_loongson3_llsc (struct mips_cl_insn * ip)
 
 	      for (j = i + 1; j < lookback; j++)
 		{
-		  if (streq (history[i].insn_mo->name, "ll")
-		      || streq (history[i].insn_mo->name, "lld"))
+		  if (streq (history[j].insn_mo->name, "ll")
+		      || streq (history[j].insn_mo->name, "lld"))
 		    break;
 
 		  if (delayed_branch_p (&history[j]))
@@ -6993,7 +6989,7 @@  fix_loongson3_llsc (struct mips_cl_insn * ip)
 	      for (j = i + 1; j < lookback; j++)
 		{
 		  if (streq (history[j].insn_mo->name, "ll")
-		      || streq (history[i].insn_mo->name, "lld"))
+		      || streq (history[j].insn_mo->name, "lld"))
 		    break;
 		}