[Libre-soc-bugs] [Bug 425] DIV overflow not being calculated correctly

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=425

--- Comment #7 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
the class that contains the failing code is DivOutputStage.
therefore we need to "get at" DivOutputStage.



the gtkwave tree (and yosys graphvis) show the tree into the failing
code as:

alu -> pipe_end -> output_stage

the classes/instances are:

alu=DIVBasePipe -> pipe_end=DivStagesEnd -> div_out=DivOutputStage

div_out is local therefore i am committing a change which makes it
accessible:

+++ b/src/soc/fu/div/pipeline.py
@@ -34,6 +34,7 @@ class DivStagesEnd(PipeModBaseChain):
         core_final = DivCoreFinalStage(self.pspec)
         div_out = DivOutputStage(self.pspec)
         alu_out = DivMulOutputStage(self.pspec)
+        self.div_out = div_out # debugging - bug #425
         return [core_final, div_out, alu_out]

 commit f11ffd6b4be87763d7b93a77215212e1004f76e3 (HEAD -> master)
Author: Luke Kenneth Casson Leighton <lkcl at lkcl.net>
Date:   Fri Jul 10 10:16:28 2020 +0100

    add debugging chain for #425


now we can look at that, back in fu/div/test/test_pipe_caller.py:
                    vld = yield alu.n.valid_o
                    while not vld:
                        yield
                        vld = yield alu.n.valid_o
                        print ("bug track", alu.pipe_end.div_out)
                    yield

this has to be printed out *during* that analysis because it's a pipeline,
and if we leave the inspection too late the data is gone.

after valid_o has dropped is.... too late.

ok so running the test with that, outputted lots of copies of the
DivOutputStage object.  now for something more useful:

                    vld = yield alu.n.valid_o
                    while not vld:
                        yield
                        vld = yield alu.n.valid_o
                        # bug #425 investigation
                        do = alu.pipe_end.div_out
                        dive_abs_ov32 = yield do.i.dive_abs_ov32
                        quotient_neg = yield do.quotient_neg
                        print ("dive_abs_ov32", hex(dive_abs_ov32))
                        print ("quotient_neg", hex(quotient_neg))
                    yield

that outputs this:

dive_abs_ov32 0x0
quotient_neg 0x1
dive_abs_ov32 0x0
quotient_neg 0x1


ok let's add some more:

                        # bug #425 investigation
                        do = alu.pipe_end.div_out
                        ctx_op = do.i.ctx.op
                        is_32bit = yield ctx_op.is_32bit
                        is_signed = yield ctx_op.is_signed
                        quotient_root = yield do.i.core.quotient_root
                        dive_abs_ov32 = yield do.i.dive_abs_ov32
                        div_by_zero = yield do.i.div_by_zero
                        quotient_neg = yield do.quotient_neg
                        print ("32bit", hex(is_32bit))
                        print ("signed", hex(is_signed))
                        print ("quotient_root", hex(quotient_root))
                        print ("div_by_zero", hex(div_by_zero))
                        print ("dive_abs_ov32", hex(dive_abs_ov32))
                        print ("quotient_neg", hex(quotient_neg))
                        print ("")

committed with this:

commit 1040fedb60ba617b0a85997330c10e5769c06051 (HEAD -> master, origin/master)
Author: Luke Kenneth Casson Leighton <lkcl at lkcl.net>
Date:   Fri Jul 10 10:40:15 2020 +0100

    add more debug output for #425


and we get this output:

32bit 0x1
signed 0x1
quotient_root 0xfc00000000000003
div_by_zero 0x0
dive_abs_ov32 0x0
quotient_neg 0x1

so that's 32-bit, and signed.  therefore sign_bit_mask is 0x8000000.

therefore, 0xfc0000000000003 > 0x8000000 is successful.

therefore, overflow is indicated when it should *not* be indicated.

what i therefore suspect is that when op.is_32bit is set, the top
bits of abs_quotient (quotient_root) must be ignored.  but that
dive_abs_ov32 obviously should not.


the relevant code in microwatt divide.vhdl is here:

        if is_32bit = '0' then
            did_ovf <= overflow or (is_signed and (sresult(64) xor
sresult(63)));
        elsif is_signed = '1' then
            if ovf32 = '1' or sresult(32) /= sresult(31) then
                did_ovf <= '1';
            end if;
        else
            did_ovf <= ovf32;
        end if;


where it can be seen that they XOR bit 64 and 63 for 64-bit overflow
detection in the signed case

and for 32-bit overflow, bit 32 and 31 are (effectively) XORed together.

i'm going to give that a shot, see what happens (translate microwatt
overflow detection into nmigen)

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