Re: [gem5-dev] changeset in gem5: x86: implements x87 add/sub instructions

Andreas Sandberg Mon, 03 Feb 2014 03:16:12 -0800

I kinda missed this one on RB, but aren't the ldfp microops completelyincorrect? I wrote a quick test case for this that uses the kvm CPU as areference and it seems like single-precision adds do not work. Youshould really be using ldfp87 instead, which does the right thing whenloading 32-bit floats on the x87. See my implementation of theseinstructions in my x87-fixes branch [1] (my implementation should havebeen verified against real hardware).

Also, aren't (some of) the explicit MODRM decode blocks completelyredundant? For example, the following should be equivalent:

0x0: decode MODRM_MOD {
  0x3: Inst::FADD2(Eq);
  default: Inst::FADD2(Mq);
}
-----
0x0: Inst::FADD2(Eq);


//Andreas

[1] https://github.com/andysan/gem5/tree/fixes-x87

On 2014-01-28 01:50, Nilay Vaish wrote:

changeset 42e058cae3d0 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=42e058cae3d0
description:
        x86: implements  x87 add/sub instructions

diffstat:

  src/arch/x86/isa/decoder/x87.isa                     |  33 +++++++---
  src/arch/x86/isa/insts/x87/arithmetic/addition.py    |  59 
+++++++++++++++++++-
  src/arch/x86/isa/insts/x87/arithmetic/subtraction.py |  59 
+++++++++++++++++++-
  3 files changed, 137 insertions(+), 14 deletions(-)

diffs (218 lines):

diff -r 301f2c0b3423 -r 42e058cae3d0 src/arch/x86/isa/decoder/x87.isa
--- a/src/arch/x86/isa/decoder/x87.isa  Mon Jan 27 18:50:52 2014 -0600
+++ b/src/arch/x86/isa/decoder/x87.isa  Mon Jan 27 18:50:53 2014 -0600
@@ -39,19 +39,23 @@

format WarnUnimpl {

      0x1B: decode OPCODE_OP_BOTTOM3 {
-        //0x0: esc0();
          0x0: decode MODRM_REG {
-            // ST(0) = ST(0) + 32-bit Mem
-            0x0: fadd();
+            0x0: decode MODRM_MOD {
+                0x3: Inst::FADD1(Eq);
+                // 32-bit memory operand
+                default: Inst::FADD1(Md);
+            }
              0x1: fmul();
              0x2: fcom();
              0x3: fcomp();
-            0x4: fsub();
+            0x4: decode MODRM_MOD {
+                0x3: Inst::FSUB1(Eq);
+                default: Inst::FSUB1(Md);
+            }
              0x5: fsubr();
              0x6: fdiv();
              0x7: fdivr();
          }
-        //0x1: esc1();
          0x1: decode MODRM_REG {
              0x0: decode MODRM_MOD {
                  0x3: Inst::FLD(Eq);
@@ -202,7 +206,10 @@
          }
          //0x4: esc4();
          0x4: decode MODRM_REG {
-            0x0: fadd();
+            0x0: decode MODRM_MOD {
+                0x3: Inst::FADD2(Eq);
+                default: Inst::FADD2(Mq);
+            }
              0x1: fmul();
              0x2: decode MODRM_MOD {
                  0x3: Inst::UD2();
@@ -214,10 +221,10 @@
              }
              0x4: decode MODRM_MOD {
                  0x3: fsubr();
-                default: fsub();
+                default: Inst::FSUB2(Mq);
              }
              0x5: decode MODRM_MOD {
-                0x3: fsub();
+                0x3: Inst::FSUB2(Eq);
                  default: fsubr();
              }
              0x6: decode MODRM_MOD {
@@ -268,7 +275,10 @@
          //0x6: esc6();
          0x6: decode MODRM_REG {
              0x0: decode MODRM_MOD {
-                0x3: faddp();
+                0x3: decode MODRM_RM {
+                    0x1: Inst::FADDP();
+                    default: Inst::FADDP(Eq);
+                }
                  default: fiadd();
              }
              0x1: decode MODRM_MOD {
@@ -291,7 +301,10 @@
                  default: fisub();
              }
              0x5: decode MODRM_MOD {
-                0x3: fsubp();
+                0x3: decode MODRM_RM {
+                    0x1: Inst::FSUBP();
+                    default: Inst::FSUBP(Eq);
+                }
                  default: fisubr();
              }
              0x6: decode MODRM_MOD {
diff -r 301f2c0b3423 -r 42e058cae3d0 
src/arch/x86/isa/insts/x87/arithmetic/addition.py
--- a/src/arch/x86/isa/insts/x87/arithmetic/addition.py Mon Jan 27 18:50:52 
2014 -0600
+++ b/src/arch/x86/isa/insts/x87/arithmetic/addition.py Mon Jan 27 18:50:53 
2014 -0600
@@ -36,7 +36,62 @@
  # Authors: Gabe Black

microcode = '''

-# FADD
-# FADDP
+def macroop FADD1_R
+{
+    addfp st(0), sti, st(0)
+};
+
+
+def macroop FADD1_M
+{
+    ldfp ufp1, seg, sib, disp
+    addfp st(0), st(0), ufp1
+};
+
+def macroop FADD1_P
+{
+    rdip t7
+    ldfp ufp1, seg, riprel, disp
+    addfp st(0), st(0), ufp1
+};
+
+def macroop FADD2_R
+{
+    addfp sti, sti, st(0)
+};
+
+def macroop FADD2_M
+{
+    ldfp ufp1, seg, sib, disp
+    addfp st(0), st(0), ufp1
+};
+
+def macroop FADD2_P
+{
+    rdip t7
+    ldfp ufp1, seg, riprel, disp
+    addfp st(0), st(0), ufp1
+};
+
+def macroop FADDP
+{
+    addfp st(1), st(0), st(1), spm=1
+};
+
+def macroop FADDP_R
+{
+    addfp sti, sti, st(0), spm=1
+};
+
+def macroop FADDP_M
+{
+    fault "new UnimpInstFault"
+};
+
+def macroop FADDP_P
+{
+   fault "new UnimpInstFault"
+};
+
  # FIADD
  '''
diff -r 301f2c0b3423 -r 42e058cae3d0 
src/arch/x86/isa/insts/x87/arithmetic/subtraction.py
--- a/src/arch/x86/isa/insts/x87/arithmetic/subtraction.py      Mon Jan 27 
18:50:52 2014 -0600
+++ b/src/arch/x86/isa/insts/x87/arithmetic/subtraction.py      Mon Jan 27 
18:50:53 2014 -0600
@@ -36,8 +36,63 @@
  # Authors: Gabe Black

microcode = '''

-# FSUB
-# FSUBP
+def macroop FSUB1_R
+{
+    subfp st(0), st(0), sti
+};
+
+
+def macroop FSUB1_M
+{
+    ldfp ufp1, seg, sib, disp
+    subfp st(0), st(0), ufp1
+};
+
+def macroop FSUB1_P
+{
+    rdip t7
+    ldfp ufp1, seg, riprel, disp
+    subfp st(0), st(0), ufp1
+};
+
+def macroop FSUB2_R
+{
+    subfp sti, sti, st(0)
+};
+
+def macroop FSUB2_M
+{
+    ldfp ufp1, seg, sib, disp
+    subfp st(0), st(0), ufp1
+};
+
+def macroop FSUB2_P
+{
+    rdip t7
+    ldfp ufp1, seg, riprel, disp
+    subfp st(0), st(0), ufp1
+};
+
+def macroop FSUBP
+{
+    subfp st(1), st(1), st(0), spm=1
+};
+
+def macroop FSUBP_R
+{
+    subfp sti, sti, st(0), spm=1
+};
+
+def macroop FSUBP_M
+{
+    fault "new UnimpInstFault"
+};
+
+def macroop FSUBP_P
+{
+   fault "new UnimpInstFault"
+};
+
  # FISUB
  # FSUBR
  # FSUBRP
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev


_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] changeset in gem5: x86: implements x87 add/sub instructions

Reply via email to