RFA: XStormy16: Fix implementation of MOVF instruction

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFA: XStormy16: Fix implementation of MOVF instruction

Nick Clifton
Hi Guys,

  The patch below fixes the emulation of the XStormy16's MOVF
  instruction.  There were two problems - firstly when memory addresses
  were being aligned they were also being truncated to 16-bits.  (The
  MOVF instruction allows access to a 32-bit address space).  Secondly
  the pre-decrement and post-increment addressing modes were not
  propagating the carry into the base register.

  Tested by running lots of different xstormy16 programs under SID.

  OK to apply ?

Cheers
  Nick Clifton

cgen/ChangeLog
2010-10-28  Nick Clifton  <[hidden email]>

        * cpu/xstormy16.cpu (alignfix-mem-far): New macro.  Like
        alignfix-mem, but works with 32-bit addresses.
        (set-alignfix-mem-far): New macro.  Like set-alignfix-mem but
        works with 32-bit addresses.
        (movfgrgri, movfgrgripostinc, movfgrgripredec, movfgrgrii,
        movfgrgriipostinc, movfgrgriipredec): Use alignfix-mem-far.
        (movfgrigr, movfgripostincgr, movfgripredecgr): Use
        set-alignfix-mem-far.
        (movfgrgriipostinc, movfgriipostincgr): Propagate addition to
        source register into base register.
        (movfgrgriipredec, movfgriipredecgr): Propagate subtraction from
        source register into base register.

sid/component/cgen-cpu/xstormy16/ChangeLog
2010-10-28  Nick Clifton  <[hidden email]>

        * xstormy16-sem.cxx: Regenerate.

Index: cgen/cpu/xstormy16.cpu
===================================================================
RCS file: /cvs/src/src/cgen/cpu/xstormy16.cpu,v
retrieving revision 1.15
diff -u -3 -p -r1.15 xstormy16.cpu
--- cgen/cpu/xstormy16.cpu 1 Jun 2010 22:06:50 -0000 1.15
+++ cgen/cpu/xstormy16.cpu 28 Oct 2010 09:44:15 -0000
@@ -524,6 +524,12 @@
 (define-pmacro (set-alignfix-mem where what)
   (set (mem HI (and where #xFFFE)) what))
 
+(define-pmacro (alignfix-mem-far where)
+  (mem HI (and where #xFFFFFFFE)))
+
+(define-pmacro (set-alignfix-mem-far where what)
+  (set (mem HI (and where #xFFFFFFFE)) what))
+
 (dni movlmemimm
      "Move immediate to low memory"
      ()
@@ -824,7 +830,7 @@
      ("movf$ws2 $Rdm,($Rs)")
      (+ OP1_7 OP2A_4 ws2 Rs OP4M_0 Rdm)
      (if ws2
- (set-psw Rdm (index-of Rdm) (alignfix-mem (or (sll SI R8 16) Rs)) ws2)
+ (set-psw Rdm (index-of Rdm) (alignfix-mem-far (or (sll SI R8 16) Rs)) ws2)
  (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (or (sll SI R8 16) Rs))) ws2))
      ()
 )
@@ -836,7 +842,7 @@
      (+ OP1_6 OP2A_4 ws2 Rs OP4M_0 Rdm)
      (sequence ()
        (if ws2
-   (set-psw Rdm (index-of Rdm) (alignfix-mem (join SI HI R8 Rs)) ws2)
+   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (join SI HI R8 Rs)) ws2)
    (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (join SI HI R8 Rs))) ws2))
        (set Rs (add Rs (add ws2 1))))
      ()
@@ -850,7 +856,7 @@
      (sequence ()
        (set Rs (sub Rs (add ws2 1)))
        (if ws2
-   (set-psw Rdm (index-of Rdm) (alignfix-mem (join SI HI R8 Rs)) ws2)
+   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (join SI HI R8 Rs)) ws2)
    (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (join SI HI R8 Rs))) ws2)))
      ()
 )
@@ -862,7 +868,7 @@
      (+ OP1_7 OP2A_6 ws2 Rs OP4M_0 Rdm)
      (sequence ()
        (if ws2
-   (set-alignfix-mem (join SI HI R8 Rs) Rdm)
+   (set-alignfix-mem-far (join SI HI R8 Rs) Rdm)
    (set (mem QI (join SI HI R8 Rs)) Rdm))
        (set-psw-nowrite (index-of Rdm) Rdm ws2))
      ()
@@ -875,7 +881,7 @@
      (+ OP1_6 OP2A_6 ws2 Rs OP4M_0 Rdm)
      (sequence ()
        (if ws2
-   (set-alignfix-mem (join SI HI R8 Rs) Rdm)
+   (set-alignfix-mem-far (join SI HI R8 Rs) Rdm)
    (set (mem QI (join SI HI R8 Rs)) Rdm))
        (set-psw-nowrite (index-of Rdm) Rdm ws2)
        (set Rs (add Rs (add ws2 1))))
@@ -891,7 +897,7 @@
        (set-psw-nowrite (index-of Rdm) Rdm ws2)
        (set Rs (sub Rs (add ws2 1)))
        (if ws2
-   (set-alignfix-mem (join SI HI R8 Rs) Rdm)
+   (set-alignfix-mem-far (join SI HI R8 Rs) Rdm)
    (set (mem QI (join SI HI R8 Rs)) Rdm)))
      ()
 )
@@ -902,7 +908,7 @@
      ("movf$ws2 $Rdm,($Rb,$Rs,$imm12)")
      (+ OP1_7 OP2A_4 ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
      (if ws2
- (set-psw Rdm (index-of Rdm) (alignfix-mem (add (join SI HI Rb Rs) imm12)) ws2)
+ (set-psw Rdm (index-of Rdm) (alignfix-mem-far (add (join SI HI Rb Rs) imm12)) ws2)
  (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (add (join SI HI Rb Rs) imm12))) ws2))
      ()
 )
@@ -914,9 +920,13 @@
      (+ OP1_6 OP2A_4 ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
      (sequence ()
        (if ws2
-   (set-psw Rdm (index-of Rdm) (alignfix-mem (add (join SI HI Rb Rs) imm12)) ws2)
+   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (add (join SI HI Rb Rs) imm12)) ws2)
    (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (add (join SI HI Rb Rs) imm12))) ws2))
-       (set Rs (add Rs (add ws2 1))))
+       (set Rs (add Rs (add ws2 1)))
+       ; Note - despite the XStormy16 ISA documentation the
+       ; addition *is* propogated into the base register.
+       (if (eq Rs 0) (set Rb (add Rb 1)))
+       )
      ()
 )
 
@@ -926,9 +936,12 @@
      ("movf$ws2 $Rdm,($Rb,--$Rs,$imm12)")
      (+ OP1_6 OP2A_C ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
      (sequence ()
+       ; Note - despite the XStormy16 ISA documentation the
+       ; subtraction *is* propogated into the base register.
+       (if (eq Rs 0) (set Rb (sub Rb 1)))
        (set Rs (sub Rs (add ws2 1)))
        (if ws2
-   (set-psw Rdm (index-of Rdm) (alignfix-mem (add (join SI HI Rb Rs) imm12)) ws2)
+   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (add (join SI HI Rb Rs) imm12)) ws2)
    (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (add (join SI HI Rb Rs) imm12))) ws2)))
      ()
 )
@@ -958,7 +971,11 @@
    (set (mem HI (and (add (join SI HI Rb Rs) imm12) #xFFFFFFFE)) Rdm)
    (set (mem QI (add (join SI HI Rb Rs) imm12)) Rdm))
        (set-psw-nowrite (index-of Rdm) Rdm ws2)
-       (set Rs (add Rs (add ws2 1))))
+       (set Rs (add Rs (add ws2 1)))
+       ; Note - despite the XStormy16 ISA documentation the
+       ; addition *is* propogated into the base register.
+       (if (eq Rs 0) (set Rb (add Rb 1)))
+       )
      ()
 )
 
@@ -968,6 +985,9 @@
      ("movf$ws2 ($Rb,--$Rs,$imm12),$Rdm")
      (+ OP1_6 OP2A_E ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
      (sequence ()
+       ; Note - despite the XStormy16 ISA documentation the
+       ; subtraction *is* propogated into the base register.
+       (if (eq Rs 0) (set Rb (sub Rb 1)))
        (set Rs (sub Rs (add ws2 1)))
        (set-psw-nowrite (index-of Rdm) Rdm ws2)
        (if ws2
Reply | Threaded
Open this post in threaded view
|

Re: RFA: XStormy16: Fix implementation of MOVF instruction

Dave Brolley-2
  Hi Nick,

I don't know that this port has a maintainer for CGEN/SID. However, I
think that testing against a specific test case which exhibits the bug
and running the SID testsuite for --target=xstormy16-elf should be
sufficient. I see that there is no test case for this insn in
sid/component/testsuite/sidcomp.cgen-cpu.xstormy16, so please add one
before running the test suite.

Dave

On 10/28/2010 05:59 AM, Nick Clifton wrote:

> Hi Guys,
>
>    The patch below fixes the emulation of the XStormy16's MOVF
>    instruction.  There were two problems - firstly when memory addresses
>    were being aligned they were also being truncated to 16-bits.  (The
>    MOVF instruction allows access to a 32-bit address space).  Secondly
>    the pre-decrement and post-increment addressing modes were not
>    propagating the carry into the base register.
>
>    Tested by running lots of different xstormy16 programs under SID.
>
>    OK to apply ?
>
> Cheers
>    Nick Clifton
>
> cgen/ChangeLog
> 2010-10-28  Nick Clifton<[hidden email]>
>
> * cpu/xstormy16.cpu (alignfix-mem-far): New macro.  Like
>          alignfix-mem, but works with 32-bit addresses.
>          (set-alignfix-mem-far): New macro.  Like set-alignfix-mem but
>          works with 32-bit addresses.
>          (movfgrgri, movfgrgripostinc, movfgrgripredec, movfgrgrii,
>          movfgrgriipostinc, movfgrgriipredec): Use alignfix-mem-far.
>          (movfgrigr, movfgripostincgr, movfgripredecgr): Use
>          set-alignfix-mem-far.
> (movfgrgriipostinc, movfgriipostincgr): Propagate addition to
>          source register into base register.
> (movfgrgriipredec, movfgriipredecgr): Propagate subtraction from
>          source register into base register.
>
> sid/component/cgen-cpu/xstormy16/ChangeLog
> 2010-10-28  Nick Clifton<[hidden email]>
>
> * xstormy16-sem.cxx: Regenerate.
>
> Index: cgen/cpu/xstormy16.cpu
> ===================================================================
> RCS file: /cvs/src/src/cgen/cpu/xstormy16.cpu,v
> retrieving revision 1.15
> diff -u -3 -p -r1.15 xstormy16.cpu
> --- cgen/cpu/xstormy16.cpu 1 Jun 2010 22:06:50 -0000 1.15
> +++ cgen/cpu/xstormy16.cpu 28 Oct 2010 09:44:15 -0000
> @@ -524,6 +524,12 @@
>   (define-pmacro (set-alignfix-mem where what)
>     (set (mem HI (and where #xFFFE)) what))
>
> +(define-pmacro (alignfix-mem-far where)
> +  (mem HI (and where #xFFFFFFFE)))
> +
> +(define-pmacro (set-alignfix-mem-far where what)
> +  (set (mem HI (and where #xFFFFFFFE)) what))
> +
>   (dni movlmemimm
>        "Move immediate to low memory"
>        ()
> @@ -824,7 +830,7 @@
>        ("movf$ws2 $Rdm,($Rs)")
>        (+ OP1_7 OP2A_4 ws2 Rs OP4M_0 Rdm)
>        (if ws2
> - (set-psw Rdm (index-of Rdm) (alignfix-mem (or (sll SI R8 16) Rs)) ws2)
> + (set-psw Rdm (index-of Rdm) (alignfix-mem-far (or (sll SI R8 16) Rs)) ws2)
>   (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (or (sll SI R8 16) Rs))) ws2))
>        ()
>   )
> @@ -836,7 +842,7 @@
>        (+ OP1_6 OP2A_4 ws2 Rs OP4M_0 Rdm)
>        (sequence ()
>         (if ws2
> -   (set-psw Rdm (index-of Rdm) (alignfix-mem (join SI HI R8 Rs)) ws2)
> +   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (join SI HI R8 Rs)) ws2)
>     (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (join SI HI R8 Rs))) ws2))
>         (set Rs (add Rs (add ws2 1))))
>        ()
> @@ -850,7 +856,7 @@
>        (sequence ()
>         (set Rs (sub Rs (add ws2 1)))
>         (if ws2
> -   (set-psw Rdm (index-of Rdm) (alignfix-mem (join SI HI R8 Rs)) ws2)
> +   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (join SI HI R8 Rs)) ws2)
>     (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (join SI HI R8 Rs))) ws2)))
>        ()
>   )
> @@ -862,7 +868,7 @@
>        (+ OP1_7 OP2A_6 ws2 Rs OP4M_0 Rdm)
>        (sequence ()
>         (if ws2
> -   (set-alignfix-mem (join SI HI R8 Rs) Rdm)
> +   (set-alignfix-mem-far (join SI HI R8 Rs) Rdm)
>     (set (mem QI (join SI HI R8 Rs)) Rdm))
>         (set-psw-nowrite (index-of Rdm) Rdm ws2))
>        ()
> @@ -875,7 +881,7 @@
>        (+ OP1_6 OP2A_6 ws2 Rs OP4M_0 Rdm)
>        (sequence ()
>         (if ws2
> -   (set-alignfix-mem (join SI HI R8 Rs) Rdm)
> +   (set-alignfix-mem-far (join SI HI R8 Rs) Rdm)
>     (set (mem QI (join SI HI R8 Rs)) Rdm))
>         (set-psw-nowrite (index-of Rdm) Rdm ws2)
>         (set Rs (add Rs (add ws2 1))))
> @@ -891,7 +897,7 @@
>         (set-psw-nowrite (index-of Rdm) Rdm ws2)
>         (set Rs (sub Rs (add ws2 1)))
>         (if ws2
> -   (set-alignfix-mem (join SI HI R8 Rs) Rdm)
> +   (set-alignfix-mem-far (join SI HI R8 Rs) Rdm)
>     (set (mem QI (join SI HI R8 Rs)) Rdm)))
>        ()
>   )
> @@ -902,7 +908,7 @@
>        ("movf$ws2 $Rdm,($Rb,$Rs,$imm12)")
>        (+ OP1_7 OP2A_4 ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
>        (if ws2
> - (set-psw Rdm (index-of Rdm) (alignfix-mem (add (join SI HI Rb Rs) imm12)) ws2)
> + (set-psw Rdm (index-of Rdm) (alignfix-mem-far (add (join SI HI Rb Rs) imm12)) ws2)
>   (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (add (join SI HI Rb Rs) imm12))) ws2))
>        ()
>   )
> @@ -914,9 +920,13 @@
>        (+ OP1_6 OP2A_4 ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
>        (sequence ()
>         (if ws2
> -   (set-psw Rdm (index-of Rdm) (alignfix-mem (add (join SI HI Rb Rs) imm12)) ws2)
> +   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (add (join SI HI Rb Rs) imm12)) ws2)
>     (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (add (join SI HI Rb Rs) imm12))) ws2))
> -       (set Rs (add Rs (add ws2 1))))
> +       (set Rs (add Rs (add ws2 1)))
> +       ; Note - despite the XStormy16 ISA documentation the
> +       ; addition *is* propogated into the base register.
> +       (if (eq Rs 0) (set Rb (add Rb 1)))
> +       )
>        ()
>   )
>
> @@ -926,9 +936,12 @@
>        ("movf$ws2 $Rdm,($Rb,--$Rs,$imm12)")
>        (+ OP1_6 OP2A_C ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
>        (sequence ()
> +       ; Note - despite the XStormy16 ISA documentation the
> +       ; subtraction *is* propogated into the base register.
> +       (if (eq Rs 0) (set Rb (sub Rb 1)))
>         (set Rs (sub Rs (add ws2 1)))
>         (if ws2
> -   (set-psw Rdm (index-of Rdm) (alignfix-mem (add (join SI HI Rb Rs) imm12)) ws2)
> +   (set-psw Rdm (index-of Rdm) (alignfix-mem-far (add (join SI HI Rb Rs) imm12)) ws2)
>     (set-psw Rdm (index-of Rdm) (and #xFF (mem QI (add (join SI HI Rb Rs) imm12))) ws2)))
>        ()
>   )
> @@ -958,7 +971,11 @@
>     (set (mem HI (and (add (join SI HI Rb Rs) imm12) #xFFFFFFFE)) Rdm)
>     (set (mem QI (add (join SI HI Rb Rs) imm12)) Rdm))
>         (set-psw-nowrite (index-of Rdm) Rdm ws2)
> -       (set Rs (add Rs (add ws2 1))))
> +       (set Rs (add Rs (add ws2 1)))
> +       ; Note - despite the XStormy16 ISA documentation the
> +       ; addition *is* propogated into the base register.
> +       (if (eq Rs 0) (set Rb (add Rb 1)))
> +       )
>        ()
>   )
>
> @@ -968,6 +985,9 @@
>        ("movf$ws2 ($Rb,--$Rs,$imm12),$Rdm")
>        (+ OP1_6 OP2A_E ws2 Rs OP4M_1 Rdm OP5A_0 Rb imm12)
>        (sequence ()
> +       ; Note - despite the XStormy16 ISA documentation the
> +       ; subtraction *is* propogated into the base register.
> +       (if (eq Rs 0) (set Rb (sub Rb 1)))
>         (set Rs (sub Rs (add ws2 1)))
>         (set-psw-nowrite (index-of Rdm) Rdm ws2)
>         (if ws2
Reply | Threaded
Open this post in threaded view
|

Re: RFA: XStormy16: Fix implementation of MOVF instruction

Nick Clifton
Hi Dave, Hi Frank,

> I don't know that this port has a maintainer for CGEN/SID. However, I
> think that testing against a specific test case which exhibits the bug
> and running the SID testsuite for --target=xstormy16-elf should be
> sufficient. I see that there is no test case for this insn in
> sid/component/testsuite/sidcomp.cgen-cpu.xstormy16, so please add one
> before running the test suite.

The attached patch does that.  (It actually adds 4 new tests, one for
each variant of the pre-decrement/post-increment load/store version of
the MOVF instruction).

In the course of checking the patch I also found that all of the
XStormy16 tests were failing because the "pass" and "fail" macros had
not been updated to match the new parameter layout for the write
syscall.  So the patch fixes this as well.

Then I found several testcases that were failing due to endian mistakes,
so those have been fixed as well.

Tested with an xstormy16-elf toolchain and no regressions.

OK to apply (along with the original MOVF patch) ?

Cheers
   Nick
2010-11-02  Nick Clifton  <[hidden email]>

        * testutils.inc (pass): Update parameter layout for write syscall.
        (fail): Likewise.
        * movgrgrii.cgs: Fix endianness typo in assertion.
        * movgrgriipostinc.cgs: Likewise.
        * movgrgriipredec.cgs: Likewise.
        * movgrgripostinc.cgs: Likewise.
        * movgrgripredec.cgs: Likewise.
        * movgrigr.cgs: Likewise.
        * movgriipostincgr.cgs: Likewise.
        * movgripostincgr.cgs: Likewise.
        * movgripredecgr.cgs: Likewise.
        * rrcgrgr.cgs: Rotate only inserts carry flag once.
        * rrcgrimm4.cgs: Likewise.
        * movfgrgriipostinc.cgs: New test.  Checks MOVF load with
        post increment.
        * movfgrgriipredec.cgs: New test.  Checks MOVF load with
        pre decrement.
        * movfgriipostincgr.cgs: New test.  Checks MOVF store with
        post increment.
        * movfgriipostincgr.cgs: New test.  Checks MOVF store with
        pre decrement.

xstormy16.sid.testsuite.patch (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFA: XStormy16: Fix implementation of MOVF instruction

Dave Brolley-2
  OK with me.

Thanks,
Dave

On 11/02/2010 12:18 PM, Nick Clifton wrote:

> Hi Dave, Hi Frank,
>
>> I don't know that this port has a maintainer for CGEN/SID. However, I
>> think that testing against a specific test case which exhibits the bug
>> and running the SID testsuite for --target=xstormy16-elf should be
>> sufficient. I see that there is no test case for this insn in
>> sid/component/testsuite/sidcomp.cgen-cpu.xstormy16, so please add one
>> before running the test suite.
>
> The attached patch does that.  (It actually adds 4 new tests, one for
> each variant of the pre-decrement/post-increment load/store version of
> the MOVF instruction).
>
> In the course of checking the patch I also found that all of the
> XStormy16 tests were failing because the "pass" and "fail" macros had
> not been updated to match the new parameter layout for the write
> syscall.  So the patch fixes this as well.
>
> Then I found several testcases that were failing due to endian
> mistakes, so those have been fixed as well.
>
> Tested with an xstormy16-elf toolchain and no regressions.
>
> OK to apply (along with the original MOVF patch) ?
>
> Cheers
>   Nick
> 2010-11-02  Nick Clifton <[hidden email]>
>
>     * testutils.inc (pass): Update parameter layout for write syscall.
>     (fail): Likewise.
>     * movgrgrii.cgs: Fix endianness typo in assertion.
>     * movgrgriipostinc.cgs: Likewise.
>     * movgrgriipredec.cgs: Likewise.
>     * movgrgripostinc.cgs: Likewise.
>     * movgrgripredec.cgs: Likewise.
>     * movgrigr.cgs: Likewise.
>     * movgriipostincgr.cgs: Likewise.
>     * movgripostincgr.cgs: Likewise.
>     * movgripredecgr.cgs: Likewise.
>     * rrcgrgr.cgs: Rotate only inserts carry flag once.
>     * rrcgrimm4.cgs: Likewise.
>     * movfgrgriipostinc.cgs: New test.  Checks MOVF load with
>     post increment.
>     * movfgrgriipredec.cgs: New test.  Checks MOVF load with
>     pre decrement.
>     * movfgriipostincgr.cgs: New test.  Checks MOVF store with
>     post increment.
>     * movfgriipostincgr.cgs: New test.  Checks MOVF store with
>     pre decrement.