riscv-software-src / riscv-tools

RISC-V Tools (ISA Simulator and Tests)
1.13k stars 446 forks source link

Adding new instruction to RISCV-32ima: “bad RISCV-opcode” #233

Open lmorel-insa opened 5 years ago

lmorel-insa commented 5 years ago

DISCLAIMER: this is a duplicate of a post I opened on Stackoverflow : https://stackoverflow.com/questions/51825942/adding-new-instruction-to-riscv-32ima-bad-riscv-opcode

Hope to reach more/different people and answers around here.....

I'm seeking for help concerning the extensions I'm trying to add to riscv.

GLOBAL SETTING

My working baseline is a clone of the riscv-tools repo, containing the usual tools, among which are:

nb: the last commit I cloned from was c6d58cecb3862deb742e1a5cc9d1c682f2c50ba9 (2018-04-24).

I base my work on a riscv32-ima core. I want to add one instruction to this processor's ISA that will activate a particular component within my processor.

From the behavior of the proc itself, I have no problem: I modified spike and my instruction (as well as the component I add to the processor) work perfectly well.

In assembler, the instruction would look like:

addi a0, a0, 0
...            // other code
setupcomp      // activate my component ... 
...            // other code

See that this instruction has no operands whatsoever.

WHAT I DO

I duck-ducked-go-ed a while and found the this tutorial, which is a bit old.

So, I:

  1. go to riscv-tools/riscv-opcodes/

  2. add the opcode and its mask to riscv-tools/riscv-opcodes/opcodes. Mine look like this:

    setupcomp 31..28=ignore 27..20=ignore 19..15=ignore 14..12=0 11..7=ignore 6..2=0x1a 1..0=3
  3. from there, I rebuild the necessary .h files:

    make install
  4. Now, I add the necessary stucts to riscv-tools/riscv-gnu-toolchain/riscv-binutils-gdb/include/opcode/riscv-opc.h, and I also declare the instruction officially:

    #define MATCH_SETUPCOMP 0x6b
    #define MASK_SETUPCOMP  0x707f
    DECLARE_INSN(setupcomp, MATCH_SETUPCOMP, MASK_SETUPCOMP)

    These values, I got from what was generated from the opcodes project.

  5. I also add the necessary definitions to: riscv-tools/riscv-gnu-toolchain/riscv-binutils-gdb/opcodes/riscv-opc.c:

    {"setupcomp", "I", "", MATCH_SETUPCOMP, MASK_SETUPCOMP, match_opcode, 0 },

Now, up to here, I believe I've done everything necessary. I still have a doubt on the opcode I want, but I don't believe this has an impact on the behavior I observe and which I describe now.

PROBLEM I HAVE

When I build everything with the riscv-tools/build-rv32ima.sh script, near the end of the process (I believe in something like a test suite) I get a message complaining that:

Assembler messages:
Error: internal: bad RISC-V opcode (bits 0xffffffffffff8f80 undefined): setupcomp 
Fatal error: Broken assembler.  No assembly attempted.
make[6]: *** [lib_a-dummy.o] Error 1

I think I'm missing something in the declaration of the instruction, probably that this declaration is not "forwarded" properly to every part of the toolchain that actually needs it. But I can't find where/what/how/when and I would very much appreciate any input on this.

Of course, I'm most probably missing something obvious, so be gentle :)

jim-wilson commented 5 years ago

Ignore riscv-opcodes. It isn't used for maintaining the assembler. Just worry about your assembler changes.

The assembler does a sanity check to verify that every bit is defined by an instruction, either a bit that must be matched as one, or matched as zero, or set by an operand. The assembler error is telling you that you have undefined bits in your instruction. These are the bits that are showing up as ones in the error message. I suspect these are all of the ignored bits in your riscv-opcodes line. You can't ignore bits in an instruction encoding. You have to define them.

Try looking at other instruction definitions in the assembler, or try stepping through it in a debugger to see how it works.

lmorel-insa commented 5 years ago

Jim, Thanks for your advice. I had to leave this question hanging for a while due to other more urgent matters at work. But I'll come back to it soon and explore more of the definitions of riscv instructions.

WilliamWangPeng commented 4 years ago

Ignore riscv-opcodes. It isn't used for maintaining the assembler. Just worry about your assembler changes.

The assembler does a sanity check to verify that every bit is defined by an instruction, either a bit that must be matched as one, or matched as zero, or set by an operand. The assembler error is telling you that you have undefined bits in your instruction. These are the bits that are showing up as ones in the error message. I suspect these are all of the ignored bits in your riscv-opcodes line. You can't ignore bits in an instruction encoding. You have to define them.

Try looking at other instruction definitions in the assembler, or try stepping through it in a debugger to see how it works.

Jim is always so helpful. I got a question which confused me for a while, like instruction "mod" what's the meaning of MATCH_MOD and MASK_MOD here?

define MATCH_MOD 0x57

define MASK_MOD 0x707f

... DECLARE_INSN(mod, MATCH_MOD, MASK_MOD)

jim-wilson commented 4 years ago

MATCH_MOD is the value of all of the non-operand bits. MASK_MOD is all of the non-operand bits. So the disassembler does something like "((insn ^ MATCH_MOD) & MASK_MOD) == 0" to determine if this is a mod instruction. See the match_opcode function. The assembler does something like "insn = MATCH_MOD | ...operand bits..." to create the instruction. There are some exceptions for cases where this isn't enough info. That is what the other functions like match_rd_nonzero are for. These are mostly for compressed instructions which have extra conditions on when they are valid.

msingla403 commented 2 years ago

Ignore riscv-opcodes. It isn't used for maintaining the assembler. Just worry about your assembler changes.

The assembler does a sanity check to verify that every bit is defined by an instruction, either a bit that must be matched as one, or matched as zero, or set by an operand. The assembler error is telling you that you have undefined bits in your instruction. These are the bits that are showing up as ones in the error message. I suspect these are all of the ignored bits in your riscv-opcodes line. You can't ignore bits in an instruction encoding. You have to define them.

Try looking at other instruction definitions in the assembler, or try stepping through it in a debugger to see how it works.

I am facing same problem. I added my instruction similar to ecall.

In file riscv-binutils/include/opcode/riscv-opc.h:

#define MATCH_ECALL 0x73
#define MASK_ECALL  0xffffffff
DECLARE_INSN(ecall, MATCH_ECALL, MASK_ECALL)
#define MATCH_TRIPEDM 0x74
#define MASK_TRIPEDM 0xffffffff
DECLARE_INSN(tripedm, MATCH_TRIPEDM, MASK_TRIPEDM)

In file riscv-binutils/opcodes/riscv-opc.c:

{"ecall",       0, INSN_CLASS_I, "",          MATCH_SCALL, MASK_SCALL, match_opcode, 0 },
{"tripedm",     0, INSN_CLASS_I, "",          MATCH_TRIPEDM, MASK_TRIPEDM, match_opcode, 0 },

Error:

Assembler messages:
Error: internal: bad RISC-V opcode (bits 0xffffffffffff0000 undefined): tripedm 
Fatal error: internal: broken assembler.  No assembly attempted
make: *** [Makefile:336: entry.o] Error 1

Thanks in advance!

aswaterman commented 2 years ago

I think this is happening because 0x74 is not a valid opcode for a 32-bit-long RISC-V instruction. If instruction bits 1..0 are not equal to 3, then it's a 16-bit-long instruction. So you should pick another opcode for which bits 1..0 equal 3. (Or, if you intended to make a 16-bit-long instruction, change the mask to 0xffff.)

msingla403 commented 2 years ago

Thanks @aswaterman ! It worked. I made instruction to a 16-bit one by changing mask to 0xffff.