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

5
votes

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:

  • riscv-fesvr
  • riscv-gnu-toolchain
  • riscv-isa-sim
  • riscv-opcodes
  • riscv-pk

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 :)

3
riscvbinutils
just a guess: AFAIK the MASK_SETUPCOMP needs to mask every bit, which is not being used as an argument. So try #define MASK_SETUPCOMP 0xffffffff. Note that 0xffffffffffff8f80 is exactly the bit-complement of 0x707f. - Domso
this error comes from the function validate_riscv_insn ( file tc-riscv.c) which checks the consistencies of the masks, the used_bits and the instruction length. By default the whole delivery comes with opcodes-custom file that contains some custom instructions. But none of them are declared in the riscv-opc.c file . If you add custom1 in opc.c as you did in your example, the same issue appears. So either the doc is incomplete, or the implementation of this feature is not complete. May be worth to post a ticket to berkeley-riscv github. - Pierre G.

3 Answers

2
votes

Your best bet for adding custom instructions to the toolchain is to use our .insn assembler directive. While this doesn't give you pretty syntax (you're expected to use a preprocessor macro or C wrapper function), it does let you avoid touching the toolchain as that's fraught with issues.

The best documentation for this is the GAS testsuite for .insn. Essentially users can emit instructions of any format defined by the RISC-V ISA manuals, with options to fill out each bit pattern directly. This integrates with GCC's inline-assembly register allocation framework cleanly, where it works just like any other instruction.

1
votes

As Domso says, riscv-opc.c seems to require every bit that isn't a parameter to be explicitly masked. But if you set bits to ignore in your opcodes file, the mask parse-opcodes generates will not have those bits set. Really this is what you want, since the mask is supposed to indicate that set bits are part of the opcode.

Since fence.i uses ignored bits, you can deduce how to handle ignored bits by looking at the way it is defined:

In riscv-opc.h, MASK_FENCE_I is just mask that would be generated by parse-opcodes:

#define MATCH_FENCE_I 0x100f
#define MASK_FENCE_I  0x707f

The mask is given to DECLARE_INSN unchanged:

DECLARE_INSN(fence_i, MATCH_FENCE_I, MASK_FENCE_I)

But in riscv-opc.c the ignored bits are masked using a bitwise or.

{"fence.i", "I", "", MATCH_FENCE_I, MASK_FENCE | MASK_RD | MASK_RS1 | MASK_IMM, match_opcode, 0 }

Since fence.i takes no arguments, this is equivalent to

{"fence.i", "I", "", 0xFFFFFFFF, match_opcode, 0 }

So you should probably keep MASK_SETUPCOMP as it is, so that it can be given to DECLARE_INSN, but set the mask in riscv-opc.c to 0xFFFFFFFF. Also since MASK_SETUPCOMP gets duplicated in a bunch of header files by make install, it's better to keep them consistent.

0
votes

Just run following command to parse MASK/MATCH:

cat opcodes-pseudo opcodes opcodes-rvc opcodes-rvc-pseudo opcodes-custom | ./parse-opcodes -c > ~/temp.h

It gives you correct MASK and MATCH for riscv-gnu-toolchain/riscv-binutils-gdb/include/opcode/riscv-opc.h Now recompile and you`ve done!