On Thu, Jan 13, 2022 at 6:28 PM Philipp Tomsich <philipp.toms...@vrull.eu> wrote: > > On Thu, 13 Jan 2022 at 06:07, Alistair Francis <alistai...@gmail.com> wrote: > > > > On Thu, Jan 13, 2022 at 1:42 AM Philipp Tomsich > > <philipp.toms...@vrull.eu> wrote: > > > > > > Alistair, > > > > > > Do you (and the other RISC-V custodians of target/riscv) have any opinion > > > on this? > > > We can go either way — and it boils down to a style and process question. > > > > Sorry, it's a busy week! > > > > I had a quick look over this series and left some comments below. > > > Thank you for taking the time despite the busy week — I can absolutely > relate, as it seems that January is picking up right where December > left off ;-) > > > > > > > > > Thanks, > > > Philipp. > > > > > > On Mon, 10 Jan 2022 at 12:30, Philippe Mathieu-Daudé <f4...@amsat.org> > > > wrote: > > >> > > >> On 1/10/22 12:11, Philipp Tomsich wrote: > > >> > On Mon, 10 Jan 2022 at 11:03, Philippe Mathieu-Daudé <f4...@amsat.org > > >> > <mailto:f4...@amsat.org>> wrote: > > >> > > > >> > On 1/10/22 10:52, Philipp Tomsich wrote: > > >> > > For RISC-V the opcode decode will change between different vendor > > >> > > implementations of RISC-V (emulated by the same qemu binary). > > >> > > Any two vendors may reuse the same opcode space, e.g., we may end > > >> > up with: > > >> > > > > >> > > # *** RV64 Custom-3 Extension *** > > >> > > { > > >> > > vt_maskc 0000000 ..... ..... 110 ..... 1111011 @r > > >> > |has_xventanacondops_p > > >> > > vt_maskcn 0000000 ..... ..... 111 ..... 1111011 @r > > >> > |has_xventanacondops_p > > >> > > someone_something ............ ..... 000 ..... 1100111 @i > > >> > > |has_xsomeonesomething_p > > >> > > } > > > > I don't love this. If even a few vendors use this we could have a huge > > number of instructions here > > > > >> > > > > >> > > With extensions being enabled either from the commandline > > >> > > -cpu any,xventanacondops=true > > >> > > or possibly even having a AMP in one emulation setup (e.g. > > >> > application > > >> > > cores having one extension and power-mangement cores having a > > >> > > different one — or even a conflicting one). > > > > Agreed, an AMP configuration is entirely possible. > > > > >> > > > >> > I understand, I think this is what MIPS does, see commit > > >> > 9d005392390: > > >> > ("target/mips: Introduce decodetree structure for NEC Vr54xx > > >> > extension") > > >> > > > >> > > > >> > The MIPS implementation is functionally equivalent, and I could see us > > >> > doing something similar for RISC-V (although I would strongly prefer to > > >> > make everything explicit via the .decode-file instead of relying on > > >> > people being aware of the logic in decode_op). > > >> > > > >> > With the growing number of optional extensions (as of today, at least > > >> > the Zb[abcs] and vector comes to mind), we would end up with a large > > >> > number of decode-files that will then need to be sequentially called > > >> > from decode_op(). The predicates can then move up into decode_op, > > >> > predicting the auto-generated decoders from being invoked. > > >> > > > >> > As of today, we have predicates for at least the following: > > >> > > > >> > * Zb[abcs] > > >> > * Vectors > > > > I see your point, having a long list of decode_*() functions to call > > is a hassle. On the other hand having thousands of lines in > > insn32.decode is also a pain. > > > > In saying that, having official RISC-V extensions in insn32.decode and > > vendor instructions in insn<vendor>.decode seems like a reasonable > > compromise. Maybe even large extensions (vector maybe?) could have > > their own insn<extension>.decode file, on a case by case basis. > > > > >> > > > >> > As long as we are in greenfield territory (i.e. not dealing with > > >> > HINT-instructions that overlap existing opcode space), this will be > > >> > fine > > >> > and provide proper isolation between the .decode-tables. > > >> > However, as soon as we need to implement something along the lines (I > > >> > know this is a bad example, as prefetching will be a no-op on qemu) of: > > >> > > > >> > { > > >> > { > > >> > # *** RV32 Zicbop Sandard Extension (hints in the ori-space) > > >> > *** > > >> > prefetch_i ....... 00000 ..... 110 00000 0010011 @cbo_pref > > >> > prefetch_r ....... 00001 ..... 110 00000 0010011 @cbo_pref > > >> > prefetch_w ....... 00011 ..... 110 00000 0010011 @cbo_pref > > >> > } > > >> > ori ............ ..... 110 ..... 0010011 @i > > >> > } > > >> > > > >> > we'd need to make sure that the generated decoders are called in the > > >> > appropriate order (i.e. the decoder for the specialized instructions > > >> > will need to be called first), which would not be apparent from looking > > >> > at the individual .decode files. > > >> > > > >> > Let me know what direction we want to take (of course, I have a bias > > >> > towards the one in the patch). > > >> > > >> I can't say for RISCV performance, I am myself biased toward maintenance > > >> where having one compilation unit per (vendor) extension. > > >> ARM and MIPS seems to go in this direction, while PPC and RISCV in the > > >> other one. > > > > I think we could do both right? > > > > We could add the predicate support, but also isolate vendors to their > > own decode file > > > > So for example, for vendor abc > > > > +++ b/target/riscv/insnabc.decode > > +# *** Custom abc Extension *** > > +{ > > + vt_maskc 0000000 ..... ..... 110 ..... 1111011 @r |has_abc_c > > + vt_maskcn 0000000 ..... ..... 111 ..... 1111011 @r |has_abc_d > > +} > > > > Then there is a decode_abc(), but we support extension abc_c and abc_d > > > > That way we can keep all the vendor code seperate, while still > > allowing flexibility. Will that work? > > I'll split this up into multiple series then: > 1. XVentanaCondOps will use a separate decoder (so no decodetree changes in > v2). > 2. A new series that adds predicate support and uses it for Zb[abcs] > 3. A third series that factors vector out of the insn32.decode and > puts it into its own decoder. > > This will give us the chance to discuss individual design changes at > their own speed.
Great! Thanks for that Alistair > > Philipp. > > > > > We can also then use predicate support for the standard RISC-V > > extensions as described by Philipp > > > > Alistair
