[BUG] instruction bugs for RV32 isa

[SoloGao]

Update:

As is replied in #1642 (comment), please ensure RV32 customized instructions properly sign-extended their results

---

Hi all,

Recently we try to simulate 32bit programs with "spike --with-isa rv32imac". A test program can be summarized to:

[ff000cb7] lui s9, 0xff000
[ff000937] lui s2, 0xff000
[fff94493] not s1, s2
[0194f4b3] and s1, s1, s9
[0000e0f1] c.bnez s1, someplace (actual address is omitted)

However, the bnez always cause jump where it should not have done.

Digging into the code, the reason is obvious:
not is implemented with "xori: WRITE_RD(insn.i_imm() ^ RS1);". However, xori.h is written 10+ years ago and might be forgotten when we use uint64_t as reg_t instead of uint32_t.
Thus, NOT 0xff000000 equals to 0xFFFFFFFF00FFFFFF. AND 0xFF000000 equals to 0xFFFFFFFF0000000, which is not zero and causes a jump.

There are already some ops (like c_not.h) apply sext_xlen to avoid this problem, but it is clearly not implemented to all ops.

Two possible fix are:

1. For each kernel using xor/xnor/not etc. Apply sext_xlen
2. For every WRITE_(F)REG, apply sext_xlen

I may able to help fix that, but which one is preferred? Thanks.

[SoloGao]

I did a simple search, it seems only c_xor/xnor/xor/xori/andn/cmix/orn are not protected with sext_xlen. Thus maybe fix 1 is optimal.

[aswaterman]

I disagree with this analysis. LUI will load the constant in sign-extended form (FFFFFFFFFF000000), and so the XORI will produce 0000000000FFFFFF. In general, the bitwise-logical instructions do not need sext_xlen because, if the two inputs are canonically sign-extended, the result will be canonically sign-extended, too.

[aswaterman]

I wrote a test program based on your code, and I see the expected behavior:

$ cat test.s
.text
.globl main
main:

  lui s9, 0xff000
  lui s2, 0xff000
  not s1, s2
  and s1, s1, s9
  c.bnez s1, 1f

  li a0, 0
  ret

1:
  li a0, 1
  ret
$ riscv64-unknown-elf-gcc -march=rv32gc -mabi=ilp32d test.s
$ ./spike --isa=rv32gc ./pk ./a.out
$ echo $?
0

I hacked Spike to print out the full 64-bit contents of the registers (by changing sim_t::interactive_reg) to illustrate what's going on:

$ ./spike -d --isa=rv32gc ./pk ./a.out
(spike) until pc 0 1014c
(spike) 
core   0: 0x0001014c (0xff000cb7) lui     s9, 0xff000
(spike) 
core   0: 0x00010150 (0xff000937) lui     s2, 0xff000
(spike) 
core   0: 0x00010154 (0xfff94493) not     s1, s2
(spike) reg 0
zero: 0x0000000000000000  ra: 0x00000000000100fa  sp: 0x000000007ffffda0  gp: 0x0000000000011c38
  tp: 0x0000000000000000  t0: 0x000000000001021a  t1: 0x000000000000000f  t2: 0x0000000000000000
  s0: 0x0000000000000000  s1: 0x0000000000ffffff  a0: 0x0000000000000001  a1: 0x000000007ffffda4
  a2: 0x0000000000000000  a3: 0x0000000000000000  a4: 0x0000000000000001  a5: 0x0000000000000000
  a6: 0x0000000000000000  a7: 0x0000000000000000  s2: 0xffffffffff000000  s3: 0x0000000000000000
  s4: 0x0000000000000000  s5: 0x0000000000000000  s6: 0x0000000000000000  s7: 0x0000000000000000
  s8: 0x0000000000000000  s9: 0xffffffffff000000 s10: 0x0000000000000000 s11: 0x0000000000000000
  t3: 0x0000000000000000  t4: 0x0000000000000000  t5: 0x0000000000000000  t6: 0x0000000000000000
(spike) 
core   0: 0x00010158 (0x0194f4b3) and     s1, s1, s9
(spike) 
core   0: 0x0001015c (0x0000e099) c.bnez  s1, pc + 6
(spike) 
core   0: 0x0001015e (0x00004501) c.li    a0, 0
(spike) 
core   0: 0x00010160 (0x00008082) ret
(spike) 

Which is what I wrote in my earlier comment. So it's hard to say why you're having the problem that you're having, but I'm certain it's not because of a missing sext_xlen.

[SoloGao]

Thanks for a quick reply!
I remembered that s9 in and is actually sign-extended. So it is definitely my fault
In the code I mentioned, [ff000937] lui s2, 0xff000 is actually a customized instruction I thought to be the same as 'lui'. It stores "0x00000000FF000000" into a register when xlen=32. This finally cause the mismatch

So, to be clear. In xlen=32, should we apply all our instructions in sign-extended form? Thanks

[aswaterman]

I see, it makes sense that the customized LUI would be at fault here. Yes, in general, you should guarantee that RV32 instructions properly sign-extended their results, but you can assume as a precondition that the inputs are properly sign-extended.