Bug 509 - divwe. causing assertion failure in handle_comparison
Summary: divwe. causing assertion failure in handle_comparison
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- normal
Assignee: Jacob Lifshay
URL:
Depends on:
Blocks:
 
Reported: 2020-10-03 02:12 BST by Jacob Lifshay
Modified: 2020-10-05 02:20 BST (History)
3 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 Jacob Lifshay 2020-10-03 02:12:38 BST
I'm trying to clean up the div unit's test cases so they all pass. I modified the spec pseudocode for divwe, attempting to cause it to match power-instruction-analyzer.

https://git.libre-soc.org/?p=libreriscv.git;a=commitdiff;h=29c78ebd8ac03fd811a41ea772fe20813bdb1927

https://git.libre-soc.org/?p=soc.git;a=blobdiff;f=src/soc/fu/div/test/test_pipe_caller.py;h=188196e7e8f8a7613db1825de036990011b60442;hp=9c4ba3feeceb834cfb87869d0c7d764660e74b64;hb=dedb4073749bda0f85dfbcc43d45a5d1a66260ad;hpb=83f350a38d10a3b40a3f81ce15fbad1757440741

In the process of fixing the pseudocode, I encountered an assertion failure which I'm pretty sure is not caused by my changes:
FAIL: test_sim_only (__main__.TestPipe) [case_divwe__regression]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jacob/projects/libreriscv/soc/src/soc/fu/div/test/helper.py", line 211, in process
    yield from self.execute(alu, instruction, pdecode2,
  File "/home/jacob/projects/libreriscv/soc/src/soc/fu/div/test/helper.py", line 139, in execute
    yield from isa_sim.call(opname)
  File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/caller.py", line 756, in call
    self.handle_comparison(results)
  File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/caller.py", line 507, in handle_comparison
    assert isinstance(out, SelectableInt), \
AssertionError: out zero not a SelectableInt (1, SelectableInt(value=0x0, bits=64))
Comment 1 Jacob Lifshay 2020-10-03 02:15:23 BST
Generated divwe. python code:
    @inject()
    def op_divwe_(self, RA, RB, RT):
        dividend = concat(0, repeat=64)
        dividend[0:64] = concat(RA[32:64], concat(0, repeat=32))
        divisor = concat(0, repeat=64)
        divisor[0:64] = EXTS64(RB[32:64])
        if eq(divisor, SelectableInt(value=0x0, bits=64)):
            overflow = 1
        else:
            result = DIVS(dividend, divisor)
            result32 = concat(0, repeat=64)
            result32[0:64] = EXTS64(result[32:64])
            if eq(result32, result):
                RT[32:64] = result[32:64]
                RT[0:32] = undefined[0:32]
                overflow = 0
            else:
                overflow = 1
        if eq(overflow, 1):
            RT[0:64] = undefined[0:64]
        return (overflow, RT,)
Comment 2 Luke Kenneth Casson Leighton 2020-10-03 02:57:48 BST
(In reply to Jacob Lifshay from comment #1)
> Generated divwe. python code:
>     @inject()
>     def op_divwe_(self, RA, RB, RT):

>         return (overflow, RT,)

these two are the wrong way round.  what the heck they are doing
in that order i have no idea.

if you look at op_mullwu_ you'll see "return (RT, overflow)".

it could be due to them being a set.  can you take a look in
pseudo/parser and pywriter and, if necessary, bludgeon
write_regs into submission?

pywriter calls back to isa.caller "create_args()" so if you
can fudge that to sort the order, making overflow always be last
in the list returned it should "work".
Comment 3 Luke Kenneth Casson Leighton 2020-10-04 15:33:35 BST
jacob can you take care of this one, so i can concentrate on the layout?

overflow is detected and added to the list of write_regs here:

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/decoder/pseudo/parser.py;h=34c8aae8074dec61771d48d94d66fe88c31e6263;hb=4d4d7dcd00a6d40a297ae7f9a85931a8992293c5#l680

however in the op_divwe_ pseudocode, "overflow" as a reg occurs *before*
RT appears.

this is "solved" - post-parsing- by re-ordering write_regs either in parser.py,
or pywriter.py or even in create_args() back in pseudo/isa/caller.py

whatever "bad hack" gets things going is what's needed.
Comment 4 Jacob Lifshay 2020-10-04 17:25:29 BST
(In reply to Luke Kenneth Casson Leighton from comment #3)
> jacob can you take care of this one, so i can concentrate on the layout?

sure!

If the outputs have requirements on the order, it's only logical to sort them into the correct order, so I wouldn't call it a bad hack.
Comment 5 Jacob Lifshay 2020-10-05 01:41:51 BST
Fixed.
https://git.libre-soc.org/?p=soc.git;a=commitdiff;h=ca2fad49abde84d356ad31d3a8a6ae65db63afb4

I'm assuming `overflow` is probably not the only variable that needs to be in a particular order, so I left a TODO comment in src/soc/decoder/isa/caller.py:

REG_SORT_ORDER = {
    # TODO (lkcl): adjust other registers that should be in a particular order
    # probably CA, CA32, and CR
    "RT": 0,
    "RA": 0,
    "RB": 0,
    "RS": 0,
    "CR": 0,
    "LR": 0,
    "CTR": 0,
    "TAR": 0,
    "CA": 0,
    "CA32": 0,
    "MSR": 0,

    "overflow": 1,
}


def create_args(reglist, extra=None):
    regset = OrderedSet(reglist)
    retval = []
    for reg in regset:
        retval.append(reg)
    retval.sort(key=lambda reg: REG_SORT_ORDER[reg])
    if extra is not None:
        return [extra] + retval
    return retval
Comment 6 Luke Kenneth Casson Leighton 2020-10-05 02:20:23 BST
star.  RT and RS really are the only ones that should be output[0] althougj LD-with-update can have 2 regs returned.  i do not know if cache version also has an update as well, that would be awkward.