XiangShan icon indicating copy to clipboard operation
XiangShan copied to clipboard

riscv-zicond: Add Zicond Extension

Open cyyself opened this issue 9 months ago • 6 comments

This PR added RISC-V Integer Conditional Operations Extension, which is in the RVA23U64 Profile Mandatory Base. And the performance of conditional move instructions in micro-architecture is an interesting point to explore.

Zicond instructions added: czero.eqz, czero.nez

Changes based on spec: https://github.com/riscvarchive/riscv-zicond/releases/download/v1.0.1/riscv-zicond_1.0.1.pdf

cyyself avatar Apr 30 '24 12:04 cyyself

I don't know much about kunminghu. Let current developers have a review

poemonsense avatar Apr 30 '24 12:04 poemonsense

It can pass a simple zicond test with NEMU difftest I wrote here.

void test_zicond() {
    long x = 0xabcddeadbeef;
    long y_0 = 0;
    long y_1 = 1;
    long res0, res1, res2, res3;
    asm volatile(
        "czero.eqz %0, %1, %2"
        : "=r" (res0)
        : "r" (x), "r" (y_0)
    );
    asm volatile(
        "czero.eqz %0, %1, %2"
        : "=r" (res1)
        : "r" (x), "r" (y_1)
    );
    asm volatile(
        "czero.nez %0, %1, %2"
        : "=r" (res2)
        : "r" (x), "r" (y_0)
    );
    asm volatile(
        "czero.nez %0, %1, %2"
        : "=r" (res3)
        : "r" (x), "r" (y_1)
    );
    dump_hex(res0);
    dump_hex(res1);
    dump_hex(res2);
    dump_hex(res3);
}

I got the same output as qemu and gem5, which currently have Zicond supported on the mainline.

Using simulated 8192MB RAM
The image is ../simple-sw-workbench/start.bin
The reference model is /mnt/data/xs/xs-env-2/NEMU/build/riscv64-nemu-interpreter-so
The first instruction of core 0 has commited. Difftest enabled. 
Hello World from hart 0
0x0000000000000000
0x0000abcddeadbeef
0x0000abcddeadbeef
0x0000000000000000
Core 0: [33mSOME SIGNAL STOPS THE PROGRAM at pc = 0x80000054
[0m[35minstrCnt = 12,593, cycleCnt = 28,994, IPC = 0.434331
[0m

cyyself avatar Apr 30 '24 19:04 cyyself

[Generated by IPC robot] commit: 4bc304c246429a09aa61485d65c45975ef91cb89

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
4bc304c 1.862 0.450 2.092 1.182 2.178 2.174 2.335 0.966 1.370 1.253 2.738 2.555 2.283 2.930

master branch:

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
73c515a 1.862 0.450 2.096 1.174 2.178 2.174 2.335 0.966 1.374 1.253 2.738 2.552 2.283 2.930
0c22420 1.862 0.450 2.092 1.174 2.178 2.174 2.333 0.966 1.370 1.253 2.738 2.556 2.283 2.930
afd7818 1.862 0.450 2.090 1.184 2.178 2.174 2.335 0.966 1.370 1.253 2.738 2.554 2.283 2.930
0785388 1.862 0.450 2.090 1.175 2.178 2.174 2.332 0.966 1.392 1.253 2.738 2.556 2.283 2.930
78c76c7 1.862 0.450 2.097 1.174 2.178 2.174 2.335 0.966 1.370 1.253 2.738 2.549 2.283 2.930
7bc477b 1.862 0.450 2.092 1.174 2.178 2.174 2.335 0.966 1.405 1.253 2.738 2.557 2.283 2.930
aee6a6d 1.862 0.450 2.092 1.173 2.178 2.174 2.335 0.966 1.370 1.253 2.738 2.560 2.283 2.930
19fbeaf 1.874 0.449 2.099 1.159 2.162 2.182 2.332 0.956 1.408 0.836 2.736 2.573 2.275 2.894
37b8fde 1.874 0.449 2.099 1.159 2.162 2.182 2.333 0.956 1.355 0.836 2.736 2.573 2.275 2.894
ffd3154 1.874 0.449 2.105 1.159 2.162 2.182 2.334 0.956 1.411 0.836 2.736 2.576 2.275 2.894

XiangShanRobot avatar Apr 30 '24 22:04 XiangShanRobot

It can pass a simple zicond test with NEMU difftest I wrote

Should we update the default ISA strings in spike and nemu for zicond? We may need to update the spike/nemu so in ready-to-run as well.

poemonsense avatar May 01 '24 13:05 poemonsense

It can pass a simple zicond test with NEMU difftest I wrote

Should we update the default ISA strings in spike and nemu for zicond? We may need to update the spike/nemu so in ready-to-run as well.

I think so. BTW the ext in isa string also need to be fixed with ratified formal name. Like replace b with '_zba_zbb_zbc_zbs_zbkb_zbkc_zbkx'.

cyyself avatar May 01 '24 13:05 cyyself

It can pass a simple zicond test with NEMU difftest I wrote

Should we update the default ISA strings in spike and nemu for zicond? We may need to update the spike/nemu so in ready-to-run as well.

I think it can be fixed in another PR. There are many other things need to be fixed like dts. We have only rv64imafdc now.

cyyself avatar May 01 '24 14:05 cyyself