bug-binutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Bug gas/31072] New: gas doesn't handle MIPS1 FPR load hazard correctly


From: tsutsui at ceres dot dti.ne.jp
Subject: [Bug gas/31072] New: gas doesn't handle MIPS1 FPR load hazard correctly
Date: Thu, 16 Nov 2023 17:20:21 +0000

https://sourceware.org/bugzilla/show_bug.cgi?id=31072

            Bug ID: 31072
           Summary: gas doesn't handle MIPS1 FPR load hazard correctly
           Product: binutils
           Version: 2.42 (HEAD)
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gas
          Assignee: unassigned at sourceware dot org
          Reporter: tsutsui at ceres dot dti.ne.jp
  Target Milestone: ---

Found on analysis of a bug in NetBSD/newsmips 9.3:
https://mail-index.netbsd.org/netbsd-bugs/2023/11/16/msg080372.html

gas doesn't handle MIPS1 FPR load hazard correctly after the following commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=67dc82bc511e35ef134952014b4deb2fdcf10676;hp=2df4d1d5c4393fd06c2bffe75499e70a8d8ac8a8

To reproduce the problem:
---
% cat > test.s
.set reorder
lwc1 $f0,4($sp)
lwc1 $f1,0($sp)
add.d $f2,$f0,$f0
% gas/as-new -o test.o test.s
% binutils/objdump -dz test.o

test.o:     file format elf32-tradbigmips


Disassembly of section .text:

00000000 <.text>:
   0:   c7a00004        lwc1    $f0,4(sp)
   4:   c7a10000        lwc1    $f1,0(sp)
   8:   46200080        add.d   $f2,$f0,$f0
   c:   00000000        nop
% 
---

The pinfo field has been changed in the above commit and after the change
INSN_LOAD_MEMORY can be defined both GPR and FPR.

Actually on binutils 2.27 pinfo of lwc1 is 01020811, i.e.
(FP_S|INSN_COPROC_MEMORY_DELAY|INSN_LOAD_MEMORY|INSN_READ_3|INSN_WRITE_1),
but tc-mips.c still uses "else if" after an INSN_LOAD_MEMORY check
against GPR.

The following patch fixes the issue:

---
% git diff -U20 origin master
diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 4d40d56902a..ccf8b337b6d 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -6454,42 +6454,42 @@ insns_between (const struct mips_cl_insn *insn1,
   if (!HAVE_CODE_COMPRESSION)
     {
       /* Check for GPR or coprocessor load delays.  All such delays
         are on the RT register.  */
       /* Itbl support may require additional care here.  */
       if ((!gpr_interlocks && (pinfo1 & INSN_LOAD_MEMORY))
          || (!cop_interlocks && (pinfo1 & INSN_LOAD_COPROC)))
        {
          if (insn2 == NULL || (gpr_read_mask (insn2) & gpr_write_mask
(insn1)))
            return 1;
        }

       /* Check for generic coprocessor hazards.

         This case is not handled very well.  There is no special
         knowledge of CP0 handling, and the coprocessors other than
         the floating point unit are not distinguished at all.  */
       /* Itbl support may require additional care here. FIXME!
         Need to modify this to include knowledge about
         user specified delays!  */
-      else if ((!cop_interlocks && (pinfo1 & INSN_COPROC_MOVE))
-              || (!cop_mem_interlocks && (pinfo1 & INSN_COPROC_MEMORY_DELAY)))
+      if ((!cop_interlocks && (pinfo1 & INSN_COPROC_MOVE))
+         || (!cop_mem_interlocks && (pinfo1 & INSN_COPROC_MEMORY_DELAY)))
        {
          /* Handle cases where INSN1 writes to a known general coprocessor
             register.  There must be a one instruction delay before INSN2
             if INSN2 reads that register, otherwise no delay is needed.  */
          mask = fpr_write_mask (insn1);
          if (mask != 0)
            {
              if (!insn2 || (mask & fpr_read_mask (insn2)) != 0)
                return 1;
            }
          else
            {
              /* Read-after-write dependencies on the control registers
                 require a two-instruction gap.  */
              if ((pinfo1 & INSN_WRITE_COND_CODE)
                  && (pinfo2 & INSN_READ_COND_CODE))
                return 2;

              /* We don't know exactly what INSN1 does.  If INSN2 is
                 also a coprocessor instruction, assume there must be
% gas/as-new -o test.o test.s
% binutils/objdump -dz test.o

test.o:     file format elf32-tradbigmips


Disassembly of section .text:

00000000 <.text>:
   0:   c7a00004        lwc1    $f0,4(sp)
   4:   c7a10000        lwc1    $f1,0(sp)
   8:   00000000        nop
   c:   46200080        add.d   $f2,$f0,$f0
%

-- 
You are receiving this mail because:
You are on the CC list for the bug.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]