I made the change discussed below. > #ifdef fGEN_TCG_<tag> > fGEN_TCG_<tag>(<shortcode>); > #else > gen_helper_<tag>(<generated args>); > #endif
In addition, here's a list of changes since I submitted v2 of the patch series - Use Laurent's gensyscall.sh script to generate linux-user/hexagon/syscall_nr.h - Handle mem_noshuf - Helper overrides for all store (and predicated store) instructions - Remove "RsV = RsV" per review feedback - Fix bug in GP relative addressing mode - Fix bugs in 64-bit add/sub with carry - Simplify include file structure - Fix vhist instructions - Add directed tests in <qemu>/tests/tcg/hexagon - Change fWRAP_* macros to fGEN_TCG_* Are there other changes I should make before submitting v3 of the patch series? Much appreciated, Taylor > -----Original Message----- > From: Taylor Simpson > Sent: Monday, May 18, 2020 9:42 PM > To: Alessandro Di Federico <ale.q...@rev.ng>; qemu-devel@nongnu.org > Developers <qemu-devel@nongnu.org> > Cc: Niccolò Izzo <ni...@rev.ng>; Brian Cain <bc...@codeaurora.org> > Subject: RE: Simplifying the Hexagon frontend > > > > > -----Original Message----- > > From: Alessandro Di Federico <ale.q...@rev.ng> > > Sent: Monday, May 18, 2020 4:15 PM > > To: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org> > > Cc: Taylor Simpson <tsimp...@quicinc.com>; Niccolò Izzo <ni...@rev.ng>; > > Brian Cain <bc...@codeaurora.org> > > Subject: Simplifying the Hexagon frontend > > > > Hi, this e-mail is intended to bootstrap a public discussion on how to > > improve the Hexagon frontend implementation. At rev.ng, Niccolò and I, > > developed an Hexagon frontend, and we're (finally!) joining forces with > > the QuIC guys to merge our efforts (did you see our talk [1]?). > > > > The status is as follows: > > > > * QuIC has its own fully working implementation that has been submitted > > for review. > > * We're working to integrate in their implementation our mechanism to > > automatically generate code to generate tiny code. But this will take > > some more work. > > > > In the following, some initial considerations on how the latest > > patchset could be simplified. > > > > Here you can find a graph I've put together of the build process: > > > > https://rev.ng/downloads/qemu-hexagon/temporary/graph.svg > > https://rev.ng/downloads/qemu-hexagon/temporary/graph.dot > > > > Colors indicate language. > > Oval nodes are generated. > > Rectangles are hand-written. > > > > Taylor, I think some simplifications can be made to the process in order > > to ease the review process. > > > > * As far as I understand, from he "Source of Truth" set of files > > (`alu.idef`, `encode_pp.def`...), through `gen_semantics`, you > > generate `semantics_generated.pyinc`, which is then included by > > `do_qemu.py` script, which does the real job. > > > > I would suggest to keep `gen_semantics` and all its inputs > > out-of-tree. It increases complexity in a non-negligible way, while > > bringing a reduced benefit in terms of automation. > > I'm not a lawyer, but I believe the original sources are required to conform > to > the license. > > > > > I'd suggest replace `gen_semantics`'s output > > (`semantics_generated.pyinc`) with a human readable JSON file that > > could be manipulated by hand and is then parsed by `do_qemu.py`. I > > think JSON is more appropriate than generating executable python code > > that is then imported. > > I'm not married to python, but we need something that is executable. The > python code looks at the semantics of each instruction to determine the > number and types of the helper arguments. It also looks at some of the > attributes to decide if certain things are needed (e.g., FPOP_START) and it > scans the semantics (see need_part1 and need_ea_functions in > do_qemu.py). > > > > > * I suggest to switch to the decoding approach developed by Richard. > > That would simplify the build process and reduce the code that has to > > be reviewed. > > I'm not 100% of the effort required to do this, maybe Richard can > > weigh on this. > > > > I agree in principal, but I haven't looked into it. One thing to consider is > that > we'll need to reorder the instructions in a packet so that .new producer > instructions are ahead of the consumer. > > > * The current implementation can generate a helper function for each > > Hexagon instruction and, for a subset of instructions, it has an > > "override" mechanism to directly generate tiny code instructions > > corresponding to the semantics of the original instruction (i.e., > > without using helpers). > > > > This override mechanism is implemented with the `fWRAP` macros. They > > have benefits, but they are quite convoluted. We should strive to > > minimize the number of macros and alternative macro implementations > > to what's strictly necessary in order to generate as much code as we > > can from the "Source of Truth", but no more than that. > > > > I think the problem is that fWRAP is a pretty generic name and it serves > multiple purposes. I'll change it to a single purpose. Each instruction will > check for fGEN_TCG_<tag>. If this macro is defined, we won't create a > helper, and we'll execute the GEN_TCG macro instead. The generator will > genterate: > > #ifdef fGEN_TCG_<tag> > fGEN_TCG_<tag>(<shortcode>); > #else > gen_helper_<tag>(<generated args>); > #endif > > This will also let us get rid of the qemu_wrap_generated.h file. > > I'll include this in the next version of the patch series. > > > > > As a simpler override mechanism, we could use weak functions. But I > > think that, for simplicity, we should try to get in tree a simpler > > version of the frontend that relies exclusively on helper functions. > > It won't have optimal performances, but it will be fully functional. > > If it makes the review more straightforward, I can remove the overrides from > the patch series. Also, the series has a definite point where the scalar > core is > fully implemented, and the remaining patches in the series add the HVX > vector coprocessor. > > Would these changes make it easier for the community to review? > > > > > > Later on, once our work for automatically generating functions > > generating tiny code is mature enough, we can extend the existing > > implementation with an appropriate override system. > > > > In the meantime, we're setting up a Dockerfile based on Debian 10 > > providing a minimal C toolchain that we can use to automate testing. > > > > Feedback is more than welcome. > > > > -- > > Alessandro Di Federico > > rev.ng > > > > [1] https://www.youtube.com/watch?v=3EpnTYBOXCI
