Bug 369 - missing XER SO/OV/32 check in test_pipe_caller.py
Summary: missing XER SO/OV/32 check in test_pipe_caller.py
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: Other Linux
: --- enhancement
Assignee: Luke Kenneth Casson Leighton
URL:
Depends on:
Blocks: 305 339
  Show dependency treegraph
 
Reported: 2020-06-08 02:09 BST by Luke Kenneth Casson Leighton
Modified: 2020-06-08 19:37 BST (History)
2 users (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2020-06-08 02:09:26 BST
https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/fu/alu/test/test_pipe_caller.py;hb=HEAD

at least ALU and ShiftRot are missing so/ov/32 checking.

test_core.py shows some changes to the XER regfiles that appear not to be correct, on execution of "adde."
Comment 1 Luke Kenneth Casson Leighton 2020-06-08 14:50:54 BST
commit c822c90668e0fcac8d1717650408bce2c2dda353 (HEAD -> master)
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Mon Jun 8 14:50:35 2020 +0100

    added check which shows that OV32 in "adde." is not correct
Comment 2 Luke Kenneth Casson Leighton 2020-06-08 15:10:44 BST
-                comb += ov_o.data[0].eq((add_o[-2] != a[-1]) & (a[-1] == b[-1]))
-                comb += ov_o.data[1].eq((add_o[32] != a[31]) & (a[31] == b[31]))
+                # 32-bit (ov[1]) and 64-bit (ov[0]) overflow
+                ov = Signal(2, reset_less=True)
+                comb += ov[0].eq(calc_ov(a[-1], b[-1], ca[0], add_o[-2]))
+                comb += ov[1].eq(calc_ov(a[31], b[31], ca[1], add_o[32]))
+                comb += ov_o.data.eq(ov)

michael i believe the former code may have been incorrect, one of the tests
should have been using the carry (32/64) flags rather than the MSB of a?

also the OV32 handling in the simulator is missing, i will try to add something.
Comment 3 Luke Kenneth Casson Leighton 2020-06-08 15:28:11 BST
in decoder/isa/caller.py does this look reasonable?  it does seem to match
the hardware version (i am however seeing subf fail in test_core.py - am
assuming for now that this is a separate issue)

+            # OV (64-bit)
             input_sgn = [exts(x.value, x.bits) < 0 for x in inputs]
             output_sgn = exts(output.value, output.bits) < 0
             ov = 1 if input_sgn[0] == input_sgn[1] and \
                 output_sgn != input_sgn[0] else 0
 
+            # OV (32-bit)
+            input32_sgn = [exts(x.value, 32) < 0 for x in inputs]
+            output32_sgn = exts(output.value, 32) < 0
+            ov32 = 1 if input32_sgn[0] == input32_sgn[1] and \
+                output32_sgn != input32_sgn[0] else 0
+
             self.spr['XER'][XER_bits['OV']] = ov
+            self.spr['XER'][XER_bits['OV32']] = ov32
Comment 4 Luke Kenneth Casson Leighton 2020-06-08 18:57:53 BST
found it.

--- a/src/soc/decoder/isa/caller.py
+++ b/src/soc/decoder/isa/caller.py
@@ -380,9 +380,8 @@ class ISACaller:
         carry_en = yield self.dec2.e.output_carry
         if carry_en:
             yield from self.handle_carry_(inputs, results, already_done)
-        ov_en = yield self.dec2.e.oe.oe
-        ov_ok = yield self.dec2.e.oe.ok
-        if ov_en & ov_ok:
+        ov_en = yield self.dec2.e.oe
+        if ov_en:
Comment 5 Luke Kenneth Casson Leighton 2020-06-08 19:37:44 BST
soc/simple/test/test_core.py now passes, with XER SO/OV/OV32 checking enabled.