Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RVV: some rvv instructions' dest register is also considered as a src register. #1241

Closed
Geonwoo1998 opened this issue Jun 14, 2024 · 4 comments
Labels

Comments

@Geonwoo1998
Copy link

Describe the bug
CPU: MinorCPU
ISA: RISCV (vector extension)

vlmul : 2

0x10200 | m5op (vnop)
0x10204 | vfmul.vv v2, v0, v0
0x10208 | vfmul.vv v2, v0, v0
0x1020c | vfmul.vv v2, v0, v0
0x10210 | vfmul.vv v2, v0, v0
0x10214 | vfmul.vv v2, v0, v0
0x10218 | vfmul.vv v2, v0, v0
0x1021c | vfmul.vv v2, v0, v0
0x10220 | vfmul.vv v2, v0, v0
0x10224 | vfmul.vv v2, v0, v0
0x10228 | vfmul.vv v2, v0, v0
0x1022c | vfmul.vv v2, v0, v0
0x10230 | vfmul.vv v2, v0, v0
0x10234 | vfmul.vv v2, v0, v0
0x10238 | vfmul.vv v2, v0, v0
0x1023c | vfmul.vv v2, v0, v0
0x10240 | m5op (vnop)

In this case, there should not be any src reg. dependencies, but there was some dependency problem on execute stage in RiscvMinorCPU.
vfmul_same_dst
But it acts like there are dependencies between each of the instructions.

As i debugged, the destination register 'v2' is also considered as a source register.

0x10200 | m5op (vnop)
0x10204 | vfmul.vv v2, v0, v0
0x10208 | vfmul.vv v4, v0, v0
0x1020c | vfmul.vv v6, v0, v0
0x10210 | vfmul.vv v8, v0, v0
0x10214 | vfmul.vv v10, v0, v0
0x10218 | vfmul.vv v12, v0, v0
0x1021c | vfmul.vv v14, v0, v0
0x10220 | vfmul.vv v16, v0, v0
0x10224 | vfmul.vv v18, v0, v0
0x10228 | vfmul.vv v20, v0, v0
0x1022c | vfmul.vv v22, v0, v0
0x10230 | vfmul.vv v24, v0, v0
0x10234 | vfmul.vv v26, v0, v0
0x10238 | vfmul.vv v28, v0, v0
0x1023c | vfmul.vv v30, v0, v0
0x10240 | m5op (vnop)

To make sure my opinion, i tried new assembly codes, that each vfmul.vv has different destination register.

vfmul_diff_dest

I believe there should not be any difference on cycles between these two cases.

Affects version
23.1.0 (the latest release version)

gem5 Modifications
NOP

To Reproduce
Steps to reproduce the behavior. Please assume starting from a clean repository:

  1. Compile gem5 with command ...
    scons build/ALL/gem5.debug -j $(nrpoc)

  2. Compile c++ code below with "riscv64-unknown-elf-g++"

void main() {
    asm volatile (        
        "vsetvli	a4, a0, e32, m2, ta, ma\n"
        ".insn	4, 0x8000007b\n"
        "vfmul.vv v2, v0, v0\n"
        "vfmul.vv v4, v0, v0\n"
        "vfmul.vv v6, v0, v0\n"
        "vfmul.vv v8, v0, v0\n"
        "vfmul.vv v10, v0, v0\n"
        "vfmul.vv v12, v0, v0\n"
        "vfmul.vv v14, v0, v0\n"
        "vfmul.vv v16, v0, v0\n"
        "vfmul.vv v18, v0, v0\n"
        "vfmul.vv v20, v0, v0\n"
        "vfmul.vv v22, v0, v0\n"
        "vfmul.vv v24, v0, v0\n"
        "vfmul.vv v26, v0, v0\n"
        "vfmul.vv v28, v0, v0\n"
        "vfmul.vv v30, v0, v0\n"
        ".insn	4, 0x8200007b\n"
    );
}
  1. Execute the simulation with...

`./build/ALL/gem5.debug --debug-flags=Fetch,Decode,MinorExecute script.py vfmul_vv_example_code --cpu RiscvMinorCPU

Terminal Output

With MinorExecute Debug Flags, you can find an dependency issue due to the cycles.
At Scoreboard::canInstIssue (src/cpu/minor/scoreboard.cc : 208), I could find that dest reg is also considered as a src register.

Host Operating System
Ubuntu 22.04

Host ISA
RISC-V Vector Extension

@saul44203
Copy link
Contributor

saul44203 commented Jun 14, 2024

Right now many vector instructions will also use the destination register as a source to implement the tail/mask undisturbed policy. Currently it's the default even if you set agnostic behavior for both, but #1135 should fix this.

@Geonwoo1998
Copy link
Author

Thx for your explanation.
So, there isnt any option to change that policy without modification right now, am I right?

@saul44203
Copy link
Contributor

No. The mentioned PR adds in this option, without it gem5 will always assume an undisturbed policy for both tail and mask, even if you specify otherwise with ta ma in your vset{i}vl{i} instruction.

@ivanaamit
Copy link
Contributor

I am transitioning this into a discussion. If I am mistaken and there is a real bug that needs to be fixed, please let me know. Thanks.

@gem5 gem5 locked and limited conversation to collaborators Jun 14, 2024
@ivanaamit ivanaamit converted this issue into discussion #1245 Jun 14, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

3 participants