On Sat, Apr 28, 2018 at 12:26 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 2 March 2018 at 13:51, Michael Clark <m...@sifive.com> wrote: > > The RISC-V disassembler has no dependencies outside of the 'disas' > > directory so it can be applied independently. The majority of the > > disassembler is machine-generated from instruction set metadata: > > > > - https://github.com/michaeljclark/riscv-meta > > > + case 5: > > + if (isa == rv128) { > > + op = rv_op_c_sqsp; > > + } else { > > + op = rv_op_c_fsdsp; break; > > + } > > + case 6: op = rv_op_c_swsp; break; > > Coverity (CID1390575) points out that for the > case 5 / isa == rv128 codepath, we set op, and > then fall through into case 6 which overwrites it. > > Is there a missing "break" statement here? (If the > fallthrough is deliberate it should be marked with a > /* fallthrough */ comment.) > Thanks! The disassembler is largely machine generated but the generator has an issue with ISAs > 2 on a single set of encodings i.e. RVC. The generator was originally coded for 1 or 2 encodings but I later added rv128 to the opcodes metadata. I manually edited the generated code to add rv128 disassembly support for compressed instructions, and obviously made a mistake in hand transcription (the reason I went to the effort of making the generator). The compressed instructions overload the same encodings with different meanings for the 3 ISAs, and this is where the generator fell down, so it was only rv128 compressed instructions that I hand edited. My own propensity to make mistakes is what attracts me to machine generated code and of course lots of testing. 128-bit support would be interesting. Fabrice's RISCVEMU supports RV128 however it is an interpreter. If the large guest code is factored correctly, then we could support RV128 on 64-bit hosts much like we can support 64-bit targets on 32-bit hosts. I've just spun and sent a patch for review. Given it's only broken on rv128 I'll just add it to the existing series. Regards, Michael.