Bug 339 - create POWER9 ROTATE (SHIFTROT) pipeline
Summary: create POWER9 ROTATE (SHIFTROT) pipeline
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: Michael Nolan
URL:
Depends on: 356 361 340 360 369
Blocks: 383
  Show dependency treegraph
 
Reported: 2020-05-22 18:27 BST by Luke Kenneth Casson Leighton
Modified: 2021-12-07 11:26 GMT (History)
1 user (show)

See Also:
NLnet milestone: NLNet.2019.10.043.Wishbone
total budget (EUR) for completion of task and all subtasks: 300
budget (EUR) for this task, excluding subtasks' budget: 300
parent task for budget allocation: 383
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:
"lkcl"={amount=300, paid=2020-08-21}


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2020-05-22 18:27:32 BST
a pipeline is needed covering shift and rotate operations.
https://libre-soc.org/openpower/isa/fixedshift/

* OP_EXTSWSLI  - done
* OP_SHL       - done
* OP_SHR       - done
* OP_RLC       - done
* OP_RLCL      - done
* OP_RLCR      - done
Comment 1 Luke Kenneth Casson Leighton 2020-05-22 18:41:11 BST
examining the shift operations, they generate CR0, and optionally the Arithmetic ones will set CA and also read carry in.

do the Arith Shift also set the overflow flag?

microwatt does not have OV set, which tends to suggest that we can drop XER.SO from the output regspec for this pipeline.
Comment 2 Luke Kenneth Casson Leighton 2020-05-22 20:07:24 BST
done, here, recorded the commit in case it needs to be reverted.

commit e8e10da8650ff797faaad320fbd8b442a4bc4f0e (HEAD -> master, origin/master)
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Fri May 22 20:06:41 2020 +0100

    remove sticky overflow from Shift Rot pipeline
Comment 3 Luke Kenneth Casson Leighton 2020-06-01 13:40:38 BST
ha, hilarious.

took a while to notice: M-Form is this:

    rlwnm RA,RS,RB,MB,ME (Rc=0)

where XO-Form is this:

    divdeu RT,RA,RB

however, underneath, the 2nd argument is *consistently a constant*.  this needs
to be swapped in the CSV file.  see issue #360
Comment 4 Luke Kenneth Casson Leighton 2020-06-01 19:09:01 BST
michael, i need your help, here: sraw "works" in shift_rot/test_pipe_caller.py
only because carry isn't being checked in the test.  i'm not sure if it's
being set in the simulator?

* sraw RA,RS,RB (Rc=0)
* sraw.  RA,RS,RB (Rc=1)

    n <- (RB)[59:63]
    r <- ROTL32((RS)[32:63], 64-n)
    if (RB)[58] = 0 then
        m <-  MASK(n+32, 63)
    else m <- [0]*64
    s <- (RS)[32]
    RA <- r&m | ([s]*64)& ¬m
    carry <-  s & ((r&¬m)[32:63] != 0)
    CA    <-  carry
    CA32  <-  carry

Special Registers Altered:

    CA CA32
    CR0                    (if Rc=1)


* srawi RA,RS,SH (Rc=0)
* srawi.  RA,RS,SH (Rc=1)

    n <- SH
    r <- ROTL32((RS)[32:63], 64-n)
    m <- MASK(n+32, 63)
    s <- (RS)[32]
    RA <- r&m | ([s]*64)& ¬m
    carry <- s & ((r&¬m)[32:63] != 0)
    CA    <- carry
    CA32  <- carry

Special Registers Altered:

    CA CA32
    CR0                    (if Rc=1)
Comment 5 Michael Nolan 2020-06-01 20:31:18 BST
(In reply to Luke Kenneth Casson Leighton from comment #4)
> michael, i need your help, here: sraw "works" in
> shift_rot/test_pipe_caller.py
> only because carry isn't being checked in the test.  i'm not sure if it's
> being set in the simulator?

>     s <- (RS)[32]
>     RA <- r&m | ([s]*64)& ¬m
>     carry <-  s & ((r&¬m)[32:63] != 0)
>     CA    <-  carry
>     CA32  <-  carry
> 
> Special Registers Altered:
> 
>     CA CA32
>     CR0                    (if Rc=1)


Hmm. So the way carry gets set currently in test_pipe_caller.py is that it completely ignores modifications to the carry flag from the pseudocode and instead updates it based on the inputs and outputs. This is because for the arithmetic instructions (add/sub with carry), they don't actually set the carry flag in the pseudocode. However, it seems that the shift instructions do have pseudocode explicitly setting CA and CA32. I think parser.py should do something to instruction_info to tell caller.py to use the carry flag from the instruction globals and not generate one from the inputs and outputs. What would be the best way of going about this, does it fit with one of the existing fields?
Comment 6 Luke Kenneth Casson Leighton 2020-06-01 21:13:36 BST
(In reply to Michael Nolan from comment #5)

> carry flag in the pseudocode. However, it seems that the shift instructions
> do have pseudocode explicitly setting CA and CA32. I think parser.py should
> do something to instruction_info to tell caller.py to use the carry flag
> from the instruction globals and not generate one from the inputs and
> outputs. What would be the best way of going about this, does it fit with
> one of the existing fields?

it appears to be a special case, just for sraw etc.

i would be inclined to suggest allowing CA and CA32 to be picked up as output parameters then if spotted in the outputs the values put into corresponding XER fields.

otherwise we need to do some messing about.

although... when i looked at the pseudicode it looked very much like it was simply doing 32 bit carry i.e. bit 31 was detected as sign then 32 and above nonzero is "carry".
Comment 7 Luke Kenneth Casson Leighton 2020-06-01 21:17:16 BST
sradi is different

carry <- s & ((r& ¬m) != 0) CA <- carry CA32 <- carry

best add CA and CA32 to the fields that can be returned.
Comment 8 Luke Kenneth Casson Leighton 2020-06-07 15:32:31 BST
(In reply to Luke Kenneth Casson Leighton from comment #7)
> sradi is different
> 
> carry <- s & ((r& ¬m) != 0) CA <- carry CA32 <- carry
> 
> best add CA and CA32 to the fields that can be returned.

sorted.  it's a bit of a mess, had to special-case it, but it's
functional.

also noted that rotator.vhdl has changed (support for OP_EXTSWSLI
which i may add next)
Comment 9 Luke Kenneth Casson Leighton 2020-07-12 23:18:26 BST
(In reply to Luke Kenneth Casson Leighton from comment #8)

> also noted that rotator.vhdl has changed (support for OP_EXTSWSLI
> which i may add next)

done in rotator.py should just need calling (setting the mode) from main_stage.py
Comment 10 Luke Kenneth Casson Leighton 2020-07-13 14:19:38 BST
(In reply to Luke Kenneth Casson Leighton from comment #9)

> done in rotator.py should just need calling (setting the mode) from
> main_stage.py

done.  found and reported bugs in V3.0B specification regarding
sh.
http://lists.libre-riscv.org/pipermail/libre-riscv-dev/2020-July/008549.html