Bug 439 - generated op_divde appears to be incorrect
Summary: generated op_divde appears to be incorrect
Status: CONFIRMED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- major
Assignee: Jacob Lifshay
URL:
Depends on:
Blocks: 324
  Show dependency treegraph
 
Reported: 2020-07-24 06:03 BST by Jacob Lifshay
Modified: 2020-07-28 19:01 BST (History)
1 user (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-07-24 06:03:48 BST
got a backtrace:

Traceback (most recent call last):
  File "src/soc/fu/div/test/test_pipe_caller.py", line 359, in process
    yield from isa_sim.call(opname)
  File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/caller.py", line 663, in call
    results = info.func(self, *inputs)
  File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/caller.py", line 761, in decorator
    result = func(*args, **kwargs)
  File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/fixedarith.py", line 893, in op_divde
    divisor[0:128] = concat([0 * 64], RB)
  File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/selectable_int.py", line 441, in selectconcat
    res.bits += i.bits
AttributeError: 'list' object has no attribute 'bits'

I'm on soc commit 8bf37997d31250126a664aeb3bd67ac0cd72a70c

generated code:
    @inject()
    def op_divde(self, RA, RB, RT):
        dividend = concat(0, repeat=128)
        dividend[0:128] = concat(RA, concat(0, repeat=64))
        divisor = concat(0, repeat=128)
        divisor[0:128] = concat([0 * 64], RB)
        if eq(divisor, concat(0, repeat=128)):
            overflow = 1
        else:
            result = DIVS(dividend, divisor)
            if eq(result[64:128], SelectableInt(value=0x0, bits=64)):
                RT = result[63:128]
                overflow = 0
            else:
                overflow = 1
        if eq(overflow, 1):
            RT[0:64] = undefined[0:64]
        return (overflow, RT,)
Comment 1 Luke Kenneth Casson Leighton 2020-07-24 10:55:55 BST
(In reply to Jacob Lifshay from comment #0)
> got a backtrace:
> 
> Traceback (most recent call last):
>   File "src/soc/fu/div/test/test_pipe_caller.py", line 359, in process
>     yield from isa_sim.call(opname)
>   File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/caller.py",
> line 663, in call
>     results = info.func(self, *inputs)
>   File "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/caller.py",
> line 761, in decorator
>     result = func(*args, **kwargs)
>   File
> "/home/jacob/projects/libreriscv/soc/src/soc/decoder/isa/fixedarith.py",
> line 893, in op_divde
>     divisor[0:128] = concat([0 * 64], RB)
>   File
> "/home/jacob/projects/libreriscv/soc/src/soc/decoder/selectable_int.py",
> line 441, in selectconcat
>     res.bits += i.bits
> AttributeError: 'list' object has no attribute 'bits'
> 
> I'm on soc commit 8bf37997d31250126a664aeb3bd67ac0cd72a70c

 
that's interesting.

    dividend[0:127] <-  (RA) || [0]*64

is correctly parse/interpreted to:

         dividend[0:128] = concat(RA, concat(0, repeat=64))

(note the two concats)

where

    divisor[0:127] <- [0*64] || (RB)

is *incorrectly* parse/interpreted to:

         divisor[0:128] = concat([0 * 64], RB)

(note the missing concat).  the first argument is a list.  therefore
"res" is a list, therefore "res.bits" cannot be done.

it _should_ be: divisor[0:128] = concat(concat(0, repeat=64), RB)
not             divisor[0:128] = concat([0 * 64], RB)

will take a look.  it's in the parser, somewhere.
Comment 2 Luke Kenneth Casson Leighton 2020-07-24 11:08:17 BST
wasn't in the parser, it was the pseudocode

diff --git a/openpower/isa/fixedarith.mdwn b/openpower/isa/fixedarith.mdwn
index 5f55660..7edafee 100644
--- a/openpower/isa/fixedarith.mdwn
+++ b/openpower/isa/fixedarith.mdwn
@@ -716,7 +716,7 @@ XO-Form
 Pseudo-code:
 
     dividend[0:127] <-  (RA) || [0]*64
-    divisor[0:127] <- [0*64] || (RB)
+    divisor[0:127] <- [0]*64 || (RB)
     if divisor = [0]*128 then
         overflow <- 1
     else
@@ -746,7 +746,7 @@ XO-Form
 Pseudo-code:
 
     dividend[0:127] <-  (RA) || [0]*64
-    divisor[0:127] <- [0*64] || (RB)
+    divisor[0:127] <- [0]*64 || (RB)
     if divisor = [0]*128 then
         overflow <- 1
Comment 3 Luke Kenneth Casson Leighton 2020-07-24 11:09:48 BST
should be good, give that a shot, jacob (will try it here, too)

commit cc732fee588b4422bdf0c97d2646e9b9d75860ce (HEAD -> master, origin/master)
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Fri Jul 24 11:09:14 2020 +0100

    submodule update
Comment 4 Luke Kenneth Casson Leighton 2020-07-24 15:20:16 BST
assigning to you, jacob, for confirmation on the unit test