RE: Simplifying the Hexagon frontend

[Taylor Simpson]

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 <address@hidden>; address@hidden
> Developers <address@hidden>
> Cc: Niccolò Izzo <address@hidden>; Brian Cain <address@hidden>
> Subject: RE: Simplifying the Hexagon frontend
>
>
>
> > -----Original Message-----
> > From: Alessandro Di Federico <address@hidden>
> > Sent: Monday, May 18, 2020 4:15 PM
> > To: address@hidden Developers <address@hidden>
> > Cc: Taylor Simpson <address@hidden>; Niccolò Izzo <address@hidden>;
> > Brian Cain <address@hidden>
> > 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