Potential bugs in InstEntry.cpp InstTable::setupInstVec()

[sylvanez]

Dear SwerV-ISS team, while formally modelling the ISS core for Onespin's C++ ISA checker module, several discrepancies between the formal ISA model and the SwerV InstTable as created during setupInstVec() have been found. The affected instructions are listed below. As far as I can test, eliminating these discrepancies does not have any effect on the program execution. This could point to the reason why they have not been detected earlier: As far as I can see, the decoding does not make direct use of the information available in the InstTable member of the Hart object. E.g. operand masks are available in the InstTable but are not being used during decode() to obtain the operands from the instruction bitset. I assume this is due to better performance and easier code writability. B ut nevertheless, duplicating information is always risky ;-)

Best regards, Markus Kettern onespin.com

List of discrepancies with my explanations / interpretations (line numbers may be slightly inaccurate):

• InstEntry.cpp / 153: immTop20 = 0xffff << 12 this is only the top 16 bits, should be : immTop20 = 0xfffff << 12
• InstEntry.cpp / 390: FENCE mask includes dummy variable (should possibly be ignored), as well as fm, is this expected? FENCE.I mask as well, is set to 0xffffffff, should probably be 0xffff. FENCE.I is named "fencei" - this is the only instruction where the dot is left out in the name string
• InstEntry.cpp / 456 ADDIW imm wrongly added to code mask (11111111111100000111000001111111, should be 111000001111111)
• InstEntry.cpp / 602ff: all "amo..." instructions appear to have wrong mask (only 4 lowest bits masked instead of full 7-bis opcode)
• InstEntry.cpp / 833 FMV.X.W rd is 0 according to standard, is wrongls forced to be 01001 in code
• InstEntry.cpp / 871 FMV.W.X mask wronlgy set to 11111111111100011100000001111111 (0xfff1c07f), should probably be 11111111111100000111000001111111 (0xfff0707f)
• InstEntry.cpp / 956ff FDIV.D, FMIN.D, FMAX.D highest bit of rs2 set to 1 in code (is outside codeMask)
• InstEntry.cpp / 962 FSQRT.D wrong value 1 in code (is inside codeMask)
• InstEntry.cpp / 1025 FCLASS.D, FCLASS.S wrong code mask (looks like 7-bit rd register)
• InstEntry.cpp / 1030 FCVT.W.D wrong code for funct7 (upper 7 bits): 1111001, should be 1100001 cf. RISC-V standard, p.134
• InstEntry.cpp / 1076 FMV.D.X wrong code for upper 12 bits: 111011110000, should be 111100100000 cf. RISC-V standard, p.134
• InstEntry.cpp / 1082 MRET, SRET, URET, WFI possible wrong bit in code (around bit 22), not able to verify in standard
• InstEntry.cpp / general C.NOP is not encoded as a special case of C.ADDI
• InstEntry.cpp / 1115 C.LW: typo in code "04000", should be "0x4000"
• InstEntry.cpp / 1169 C.JAL, C.ADDIW top 3 bits are 0 in code but 001 in the standard
• InstEntry.cpp / 1174 C.LI has three operands, yet in the standard, only IMM and RD are listed. Is this on purpose?
• InstEntry.cpp / 1180 C.ADDI16SP wrong code, "110000000000110" (0x6006), should probably be "110000100000001" (0x6101)
• InstEntry.cpp / 1330: C.ADD Mask for rs2 is "11111000000" (0x7c0), should be "1111100" (0x07c)
• InstEntry.cpp / general: Enforcement of shamt[5] to be 0 by adding it to the instruction mask seems inconsistent between C.SRLI/C.SRAI/C.SLLI and their uncompressed counterparts

This is the generated log file with all detected discrepancies: log.txt

[jrahmeh]

Hi Markus,

All the issues you reported are valid. I fixed all of them except c.nop. The problem codes/masks are currently not used but it is good to get them right since they will eventually be used. Many thanks for finding and reporting these issues. Will release new code shortly.

Joe