qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* QEMU for Qualcomm Hexagon - KVM Forum talk and code available
@ 2019-10-25 16:26 Taylor Simpson
  2019-11-01 18:29 ` Philippe Mathieu-Daudé
  2019-11-05  0:05 ` Aleksandar Markovic
  0 siblings, 2 replies; 18+ messages in thread
From: Taylor Simpson @ 2019-10-25 16:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alessandro Di Federico, nizzo, Niccolò Izzo

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

We would like inform the you that we will be doing a talk at the KVM Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo Izzo, and I have been working independently on implementations of the Hexagon target.  We plan to merge the implementations, have a community review, and ultimately have Hexagon be an official target in QEMU.  Our code is available at the links below.
https://github.com/revng/qemu-hexagon
https://github.com/quic/qemu
If anyone has any feedback on the code as it stands today or guidance on how best to prepare it for review, please let us know.

Thanks,
Taylor

[-- Attachment #2: Type: text/html, Size: 2668 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-10-25 16:26 QEMU for Qualcomm Hexagon - KVM Forum talk and code available Taylor Simpson
@ 2019-11-01 18:29 ` Philippe Mathieu-Daudé
  2019-11-04  2:35   ` Taylor Simpson
  2019-11-05  0:05 ` Aleksandar Markovic
  1 sibling, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-01 18:29 UTC (permalink / raw)
  To: Taylor Simpson, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo

Hi Taylor,

On 10/25/19 6:26 PM, Taylor Simpson wrote:
> We would like inform the you that we will be doing a talk at the KVM 
> Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, 
> Niccolo Izzo, and I have been working independently on implementations 
> of the Hexagon target.  We plan to merge the implementations, have a 
> community review, and ultimately have Hexagon be an official target in 
> QEMU.  Our code is available at the links below.
> 
> _https://github.com/revng/qemu-hexagon_
> 
> _https://github.com/quic/qemu_
> 
> If anyone has any feedback on the code as it stands today or guidance on 
> how best to prepare it for review, please let us know.

Is your target the 'Hexagon Series 600', with documentation available here?
https://developer.qualcomm.com/software/hexagon-dsp-sdk/tools

Regards,

Phil.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-01 18:29 ` Philippe Mathieu-Daudé
@ 2019-11-04  2:35   ` Taylor Simpson
  0 siblings, 0 replies; 18+ messages in thread
From: Taylor Simpson @ 2019-11-04  2:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo

That is correct.

Once you register on developer.qualcomm.com, you can download the Hexagon SDK version 3.4.3.  Note that there are different downloads for Linux and Windows hosts.

Once you have installed the SDK, look for the document bundle in the following location
<INSTALL_DIR>/tools/HEXAGON_Tools/8.3.02/Documents/Hexagon_Document_Bundle.pdf
That PDF file is a container for a bunch of other documents.  If you want to read more about the Hexagon architecture, look at the following
V67 Programmer's Reference Manual
V66 HVX Programmer's Reference Manual
The version on the quic github implements these.  Note that HVX stands for Hexagon Vector eXtensions.  It is an optional set of instructions that operate on 128-byte vectors.

IIRC, the revng github implements up to V62.  Alessandro or Niccolo can confirm.

Note that the toolchain in that installation generates code for the standalone runtime or the RTOS, not Linux that the quic qemu generates.  However, they should run on the revng version.  In the coming weeks, we'll work on setting up a container to build the toolchain that will generate binaries that will run on the quic version.

If anyone has any more questions, I'm happy to answer them.

Taylor


-----Original Message-----
From: Philippe Mathieu-Daudé <philmd@redhat.com>
Sent: Friday, November 1, 2019 1:30 PM
To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; Niccolò Izzo <izzoniccolo@gmail.com>
Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available



Hi Taylor,

On 10/25/19 6:26 PM, Taylor Simpson wrote:
> We would like inform the you that we will be doing a talk at the KVM
> Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico,
> Niccolo Izzo, and I have been working independently on implementations
> of the Hexagon target.  We plan to merge the implementations, have a
> community review, and ultimately have Hexagon be an official target in
> QEMU.  Our code is available at the links below.
>
> _https://github.com/revng/qemu-hexagon_
>
> _https://github.com/quic/qemu_
>
> If anyone has any feedback on the code as it stands today or guidance
> on how best to prepare it for review, please let us know.

Is your target the 'Hexagon Series 600', with documentation available here?
https://developer.qualcomm.com/software/hexagon-dsp-sdk/tools

Regards,

Phil.




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-10-25 16:26 QEMU for Qualcomm Hexagon - KVM Forum talk and code available Taylor Simpson
  2019-11-01 18:29 ` Philippe Mathieu-Daudé
@ 2019-11-05  0:05 ` Aleksandar Markovic
  2019-11-05 16:32   ` Taylor Simpson
  1 sibling, 1 reply; 18+ messages in thread
From: Aleksandar Markovic @ 2019-11-05  0:05 UTC (permalink / raw)
  To: Taylor Simpson
  Cc: Alessandro Di Federico, nizzo, qemu-devel, Niccolò Izzo

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

On Friday, October 25, 2019, Taylor Simpson <tsimpson@quicinc.com> wrote:

> We would like inform the you that we will be doing a talk at the KVM Forum
> next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo
> Izzo, and I have been working independently on implementations of the
> Hexagon target.  We plan to merge the implementations, have a community
> review, and ultimately have Hexagon be an official target in QEMU.  Our
> code is available at the links below.
>
> *https://github.com/revng/qemu-hexagon
> <https://github.com/revng/qemu-hexagon>*
>
> *https://github.com/quic/qemu <https://github.com/quic/qemu>*
>
> If anyone has any feedback on the code as it stands today or guidance on
> how best to prepare it for review, please let us know.
>
>
>

Hi, Taylor, Niccolo (and Alessandro too).

I didn't have a chance to take a look at neither the code nor the docs, but
I did attend you presentation at KVM Forum, and I found it superb and
attractive, one of the best on the conference, if not the very best.

I just have a couple of general questions:

- Regarding the code you plan to upstream, are all SIMD instructions
implemented via tcg API, or perhaps some of them remain being implemented
using helpers?

- Most of SIMD instructions can be viewed simply as several paralel
elementary operations. However, for a given SIMD instruction set, usually
not all of them fit into this pattern. For example, "horizontal add"
(addind data elements from the same SIMD register), various
"pack/unpack/interleave/merge" operations, and more general
"shuffle/permute" operations as well (here I am not sure which of these are
included in Hexagon SIMD set, but there must be some). How did you deal
with them?

- What were the most challenging Hexagon SIMD instructions you came accross
while developing your solution?

Sincerely,
Aleksandar





> Thanks,
>
> Taylor
>

[-- Attachment #2: Type: text/html, Size: 2968 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-05  0:05 ` Aleksandar Markovic
@ 2019-11-05 16:32   ` Taylor Simpson
  2019-11-12 22:52     ` Taylor Simpson
  0 siblings, 1 reply; 18+ messages in thread
From: Taylor Simpson @ 2019-11-05 16:32 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: Alessandro Di Federico, nizzo, qemu-devel, Niccolò Izzo

[-- Attachment #1: Type: text/plain, Size: 4820 bytes --]

Hi Aleksandar,

Thank you – We’re glad you enjoyed the talk.

One point of clarification on SIMD in Hexagon.  What we refer to as the “scalar” core does have some SIMD operations.  Register pairs are 8 bytes, and there are several SIMD instructions.  The example we showed in the talk included a VADDH instruction.  It treats the register pair as 4 half-words and does a vector add.  Then there are the Hexagon Vector eXtensions (HVX) instructions that operate on 128-byte vectors.  There is a wide variety of instructions in this set.  As you mentioned, some of them are pure SIMD and others are very complex.

For the helper generator, the vast majority of these are implemented with helpers.  There are only 2 vector instructions in the scalar core that have a TCG override, and all of the HVX instructions are implemented with helpers.  If you are interested in a deeper dive, see below.

Alessandro and Niccolo can comment on the flex/bison implementation.

Thanks,
Taylor


Now for the deeper dive in case anyone is interested.  Look at the genptr.c file in target/hexagon.

The first vector instruction that is with an override is A6_vminub_RdP.  It does a byte-wise comparison of two register pairs and sets a predicate register indicating whether the byte in the left or right operand is greater.  Here is the TCG code.
#define fWRAP_A6_vminub_RdP(GENHLPR, SHORTCODE) \
{ \
    TCGv BYTE = tcg_temp_new(); \
    TCGv left = tcg_temp_new(); \
    TCGv right = tcg_temp_new(); \
    TCGv tmp = tcg_temp_new(); \
    int i; \
    tcg_gen_movi_tl(PeV, 0); \
    tcg_gen_movi_i64(RddV, 0); \
    for (i = 0; i < 8; i++) { \
        fGETUBYTE(i, RttV); \
        tcg_gen_mov_tl(left, BYTE); \
        fGETUBYTE(i, RssV); \
        tcg_gen_mov_tl(right, BYTE); \
        tcg_gen_setcond_tl(TCG_COND_GT, tmp, left, right); \
        fSETBIT(i, PeV, tmp); \
        fMIN(tmp, left, right); \
        fSETBYTE(i, RddV, tmp); \
    } \
    tcg_temp_free(BYTE); \
    tcg_temp_free(left); \
    tcg_temp_free(right); \
    tcg_temp_free(tmp); \
}

The second instruction is S2_vsplatrb.  It takes the byte from the operand and replicates it 4 times into the destination register.  Here is the TCG code.
#define fWRAP_S2_vsplatrb(GENHLPR, SHORTCODE) \
{ \
    TCGv tmp = tcg_temp_new(); \
    int i; \
    tcg_gen_movi_tl(RdV, 0); \
    tcg_gen_andi_tl(tmp, RsV, 0xff); \
    for (i = 0; i < 4; i++) { \
        tcg_gen_shli_tl(RdV, RdV, 8); \
        tcg_gen_or_tl(RdV, RdV, tmp); \
    } \
    tcg_temp_free(tmp); \
}


From: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Sent: Monday, November 4, 2019 6:05 PM
To: Taylor Simpson <tsimpson@quicinc.com>
Cc: qemu-devel@nongnu.org; Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; Niccolò Izzo <izzoniccolo@gmail.com>
Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available


CAUTION: This email originated from outside of the organization.


On Friday, October 25, 2019, Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>> wrote:
We would like inform the you that we will be doing a talk at the KVM Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo Izzo, and I have been working independently on implementations of the Hexagon target.  We plan to merge the implementations, have a community review, and ultimately have Hexagon be an official target in QEMU.  Our code is available at the links below.
https://github.com/revng/qemu-hexagon
https://github.com/quic/qemu
If anyone has any feedback on the code as it stands today or guidance on how best to prepare it for review, please let us know.


Hi, Taylor, Niccolo (and Alessandro too).

I didn't have a chance to take a look at neither the code nor the docs, but I did attend you presentation at KVM Forum, and I found it superb and attractive, one of the best on the conference, if not the very best.

I just have a couple of general questions:

- Regarding the code you plan to upstream, are all SIMD instructions implemented via tcg API, or perhaps some of them remain being implemented using helpers?

- Most of SIMD instructions can be viewed simply as several paralel elementary operations. However, for a given SIMD instruction set, usually not all of them fit into this pattern. For example, "horizontal add" (addind data elements from the same SIMD register), various "pack/unpack/interleave/merge" operations, and more general "shuffle/permute" operations as well (here I am not sure which of these are included in Hexagon SIMD set, but there must be some). How did you deal with them?

- What were the most challenging Hexagon SIMD instructions you came accross while developing your solution?

Sincerely,
Aleksandar




Thanks,
Taylor

[-- Attachment #2: Type: text/html, Size: 13139 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-05 16:32   ` Taylor Simpson
@ 2019-11-12 22:52     ` Taylor Simpson
  2019-11-13 10:31       ` Alex Bennée
  2019-11-19  9:12       ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 18+ messages in thread
From: Taylor Simpson @ 2019-11-12 22:52 UTC (permalink / raw)
  To: Taylor Simpson, Aleksandar Markovic
  Cc: Alessandro Di Federico, nizzo, qemu-devel, Niccolò Izzo

[-- Attachment #1: Type: text/plain, Size: 6779 bytes --]

I had discussions with several people at the KVM Forum, and I’ve been thinking about how to divide up the code for community review.  Here is my proposal for the steps.

  1.  linux-user changes + linux-user/hexagon + skeleton of target/hexagon
This is the minimum amount to build and run a very simple program.  I have an assembly program that prints “Hello” and exits.  It is constructed to use very few instructions that can be added brute force in the Hexagon back end.
  2.  Add the code that is imported from the Hexagon simulator and the qemu helper generator
This will allow the scalar ISA to be executed.  This will grow the set of programs that could execute, but there will still be limitations.  In particular, there can be no packets which means the C library won’t work .  We have to build with -nostdlib
  3.  Add support for packet semantics
At this point, we will be able to execute full programs linked with the C library.  This will include the check-tcg tests.
  4.  Add support for the wide vector extensions
  5.  Add the helper overrides for performance optimization
Some of these will be written by hand, and we’ll work with rev.ng to integrate their flex/bison generator.

I would love some feedback on this proposal.  Hopefully, that is enough detail so that people can comment.  If anything isn’t clear, please ask questions.


Thanks,
Taylor


From: Qemu-devel <qemu-devel-bounces+tsimpson=quicinc.com@nongnu.org> On Behalf Of Taylor Simpson
Sent: Tuesday, November 5, 2019 10:33 AM
To: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; qemu-devel@nongnu.org; Niccolò Izzo <izzoniccolo@gmail.com>
Subject: RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available

Hi Aleksandar,

Thank you – We’re glad you enjoyed the talk.

One point of clarification on SIMD in Hexagon.  What we refer to as the “scalar” core does have some SIMD operations.  Register pairs are 8 bytes, and there are several SIMD instructions.  The example we showed in the talk included a VADDH instruction.  It treats the register pair as 4 half-words and does a vector add.  Then there are the Hexagon Vector eXtensions (HVX) instructions that operate on 128-byte vectors.  There is a wide variety of instructions in this set.  As you mentioned, some of them are pure SIMD and others are very complex.

For the helper generator, the vast majority of these are implemented with helpers.  There are only 2 vector instructions in the scalar core that have a TCG override, and all of the HVX instructions are implemented with helpers.  If you are interested in a deeper dive, see below.

Alessandro and Niccolo can comment on the flex/bison implementation.

Thanks,
Taylor


Now for the deeper dive in case anyone is interested.  Look at the genptr.c file in target/hexagon.

The first vector instruction that is with an override is A6_vminub_RdP.  It does a byte-wise comparison of two register pairs and sets a predicate register indicating whether the byte in the left or right operand is greater.  Here is the TCG code.
#define fWRAP_A6_vminub_RdP(GENHLPR, SHORTCODE) \
{ \
    TCGv BYTE = tcg_temp_new(); \
    TCGv left = tcg_temp_new(); \
    TCGv right = tcg_temp_new(); \
    TCGv tmp = tcg_temp_new(); \
    int i; \
    tcg_gen_movi_tl(PeV, 0); \
    tcg_gen_movi_i64(RddV, 0); \
    for (i = 0; i < 8; i++) { \
        fGETUBYTE(i, RttV); \
        tcg_gen_mov_tl(left, BYTE); \
        fGETUBYTE(i, RssV); \
        tcg_gen_mov_tl(right, BYTE); \
        tcg_gen_setcond_tl(TCG_COND_GT, tmp, left, right); \
        fSETBIT(i, PeV, tmp); \
        fMIN(tmp, left, right); \
        fSETBYTE(i, RddV, tmp); \
    } \
    tcg_temp_free(BYTE); \
    tcg_temp_free(left); \
    tcg_temp_free(right); \
    tcg_temp_free(tmp); \
}

The second instruction is S2_vsplatrb.  It takes the byte from the operand and replicates it 4 times into the destination register.  Here is the TCG code.
#define fWRAP_S2_vsplatrb(GENHLPR, SHORTCODE) \
{ \
    TCGv tmp = tcg_temp_new(); \
    int i; \
    tcg_gen_movi_tl(RdV, 0); \
    tcg_gen_andi_tl(tmp, RsV, 0xff); \
    for (i = 0; i < 4; i++) { \
        tcg_gen_shli_tl(RdV, RdV, 8); \
        tcg_gen_or_tl(RdV, RdV, tmp); \
    } \
    tcg_temp_free(tmp); \
}


From: Aleksandar Markovic <aleksandar.m.mail@gmail.com<mailto:aleksandar.m.mail@gmail.com>>
Sent: Monday, November 4, 2019 6:05 PM
To: Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>>
Cc: qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>; Alessandro Di Federico <ale@rev.ng<mailto:ale@rev.ng>>; nizzo@rev.ng<mailto:nizzo@rev.ng>; Niccolò Izzo <izzoniccolo@gmail.com<mailto:izzoniccolo@gmail.com>>
Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available


CAUTION: This email originated from outside of the organization.


On Friday, October 25, 2019, Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>> wrote:
We would like inform the you that we will be doing a talk at the KVM Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo Izzo, and I have been working independently on implementations of the Hexagon target.  We plan to merge the implementations, have a community review, and ultimately have Hexagon be an official target in QEMU.  Our code is available at the links below.
https://github.com/revng/qemu-hexagon
https://github.com/quic/qemu
If anyone has any feedback on the code as it stands today or guidance on how best to prepare it for review, please let us know.


Hi, Taylor, Niccolo (and Alessandro too).

I didn't have a chance to take a look at neither the code nor the docs, but I did attend you presentation at KVM Forum, and I found it superb and attractive, one of the best on the conference, if not the very best.

I just have a couple of general questions:

- Regarding the code you plan to upstream, are all SIMD instructions implemented via tcg API, or perhaps some of them remain being implemented using helpers?

- Most of SIMD instructions can be viewed simply as several paralel elementary operations. However, for a given SIMD instruction set, usually not all of them fit into this pattern. For example, "horizontal add" (addind data elements from the same SIMD register), various "pack/unpack/interleave/merge" operations, and more general "shuffle/permute" operations as well (here I am not sure which of these are included in Hexagon SIMD set, but there must be some). How did you deal with them?

- What were the most challenging Hexagon SIMD instructions you came accross while developing your solution?

Sincerely,
Aleksandar




Thanks,
Taylor

[-- Attachment #2: Type: text/html, Size: 18172 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-12 22:52     ` Taylor Simpson
@ 2019-11-13 10:31       ` Alex Bennée
  2019-11-13 19:31         ` Taylor Simpson
  2019-11-13 21:27         ` Peter Maydell
  2019-11-19  9:12       ` Philippe Mathieu-Daudé
  1 sibling, 2 replies; 18+ messages in thread
From: Alex Bennée @ 2019-11-13 10:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alessandro Di Federico, Taylor Simpson, nizzo, Niccolò Izzo,
	Aleksandar Markovic


Taylor Simpson <tsimpson@quicinc.com> writes:

> I had discussions with several people at the KVM Forum, and I’ve been thinking about how to divide up the code for community review.  Here is my proposal for the steps.
>
>   1.  linux-user changes + linux-user/hexagon + skeleton of target/hexagon
> This is the minimum amount to build and run a very simple program.  I
>   have an assembly program that prints “Hello” and exits.  It is
>   constructed to use very few instructions that can be added brute
>   force in the Hexagon back end.

I'm hoping most of the linux-user changes are in the hexagon runloop?
There has been quite a bit of work splitting up and cleaning up the
#ifdef mess in linux-user over the last few years.

>   2.  Add the code that is imported from the Hexagon simulator and the qemu helper generator
> This will allow the scalar ISA to be executed.  This will grow the set
> of programs that could execute, but there will still be limitations.
> In particular, there can be no packets which means the C library won’t
> work .  We have to build with -nostdlib

You could run -nostdlib system TCG tests (hello and memory) but that
would require modelling some sort of hardware and assumes you have a
simple serial port or semihosting solution. That said a bunch of the
MIPS tests are linux-user and -nostdlib so that isn't a major problem in
getting some of the tests running.

When you say code imported from the hexagon simulator I was under the
impression you were generating code from the instruction description.
Otherwise you'll need to be very clear about your licensing grants.

>   3.  Add support for packet semantics
> At this point, we will be able to execute full programs linked with
> the C library.  This will include the check-tcg tests.

I think the interesting question is if the roll-back semantics of the
hexagon are something we might need for other emulated architectures or
is a particularly specific solution for Hexagon (I'm guessing the later).

>   4.  Add support for the wide vector extensions
>   5.  Add the helper overrides for performance optimization
> Some of these will be written by hand, and we’ll work with rev.ng to
>   integrate their flex/bison generator.

One thing to nail down will be will we include the generated code in the
source tree with a tool to regenerate (much like we do for
linux-headers) or if we want to add the dependency and regenerate each
time from scratch. I don't see including flex/bison as a dependency
being a major issue (in fact we have it in our docker images so I guess
something uses it). However it might be trickier depending on
libclang which was also being discussed.

>
> I would love some feedback on this proposal.  Hopefully, that is enough detail so that people can comment.  If anything isn’t clear, please ask questions.
>
>
> Thanks,
> Taylor
>
>
> From: Qemu-devel <qemu-devel-bounces+tsimpson=quicinc.com@nongnu.org> On Behalf Of Taylor Simpson
> Sent: Tuesday, November 5, 2019 10:33 AM
> To: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; qemu-devel@nongnu.org; Niccolò Izzo <izzoniccolo@gmail.com>
> Subject: RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
>
> Hi Aleksandar,
>
> Thank you – We’re glad you enjoyed the talk.
>
> One point of clarification on SIMD in Hexagon.  What we refer to as the “scalar” core does have some SIMD operations.  Register pairs are 8 bytes, and there are several SIMD instructions.  The example we showed in the talk included a VADDH instruction.  It treats the register pair as 4 half-words and does a vector add.  Then there are the Hexagon Vector eXtensions (HVX) instructions that operate on 128-byte vectors.  There is a wide variety of instructions in this set.  As you mentioned, some of them are pure SIMD and others are very complex.
>
> For the helper generator, the vast majority of these are implemented with helpers.  There are only 2 vector instructions in the scalar core that have a TCG override, and all of the HVX instructions are implemented with helpers.  If you are interested in a deeper dive, see below.
>
> Alessandro and Niccolo can comment on the flex/bison implementation.
>
> Thanks,
> Taylor
>
>
> Now for the deeper dive in case anyone is interested.  Look at the genptr.c file in target/hexagon.
>
> The first vector instruction that is with an override is A6_vminub_RdP.  It does a byte-wise comparison of two register pairs and sets a predicate register indicating whether the byte in the left or right operand is greater.  Here is the TCG code.
> #define fWRAP_A6_vminub_RdP(GENHLPR, SHORTCODE) \
> { \
>     TCGv BYTE = tcg_temp_new(); \
>     TCGv left = tcg_temp_new(); \
>     TCGv right = tcg_temp_new(); \
>     TCGv tmp = tcg_temp_new(); \
>     int i; \
>     tcg_gen_movi_tl(PeV, 0); \
>     tcg_gen_movi_i64(RddV, 0); \
>     for (i = 0; i < 8; i++) { \
>         fGETUBYTE(i, RttV); \
>         tcg_gen_mov_tl(left, BYTE); \
>         fGETUBYTE(i, RssV); \
>         tcg_gen_mov_tl(right, BYTE); \
>         tcg_gen_setcond_tl(TCG_COND_GT, tmp, left, right); \
>         fSETBIT(i, PeV, tmp); \
>         fMIN(tmp, left, right); \
>         fSETBYTE(i, RddV, tmp); \
>     } \
>     tcg_temp_free(BYTE); \
>     tcg_temp_free(left); \
>     tcg_temp_free(right); \
>     tcg_temp_free(tmp); \
> }
>
> The second instruction is S2_vsplatrb.  It takes the byte from the operand and replicates it 4 times into the destination register.  Here is the TCG code.
> #define fWRAP_S2_vsplatrb(GENHLPR, SHORTCODE) \
> { \
>     TCGv tmp = tcg_temp_new(); \
>     int i; \
>     tcg_gen_movi_tl(RdV, 0); \
>     tcg_gen_andi_tl(tmp, RsV, 0xff); \
>     for (i = 0; i < 4; i++) { \
>         tcg_gen_shli_tl(RdV, RdV, 8); \
>         tcg_gen_or_tl(RdV, RdV, tmp); \
>     } \
>     tcg_temp_free(tmp); \
> }
>
>
> From: Aleksandar Markovic <aleksandar.m.mail@gmail.com<mailto:aleksandar.m.mail@gmail.com>>
> Sent: Monday, November 4, 2019 6:05 PM
> To: Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>>
> Cc: qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>; Alessandro Di Federico <ale@rev.ng<mailto:ale@rev.ng>>; nizzo@rev.ng<mailto:nizzo@rev.ng>; Niccolò Izzo <izzoniccolo@gmail.com<mailto:izzoniccolo@gmail.com>>
> Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
>
>
> CAUTION: This email originated from outside of the organization.
>
>
> On Friday, October 25, 2019, Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>> wrote:
> We would like inform the you that we will be doing a talk at the KVM Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo Izzo, and I have been working independently on implementations of the Hexagon target.  We plan to merge the implementations, have a community review, and ultimately have Hexagon be an official target in QEMU.  Our code is available at the links below.
> https://github.com/revng/qemu-hexagon
> https://github.com/quic/qemu
> If anyone has any feedback on the code as it stands today or guidance on how best to prepare it for review, please let us know.
>
>
> Hi, Taylor, Niccolo (and Alessandro too).
>
> I didn't have a chance to take a look at neither the code nor the docs, but I did attend you presentation at KVM Forum, and I found it superb and attractive, one of the best on the conference, if not the very best.
>
> I just have a couple of general questions:
>
> - Regarding the code you plan to upstream, are all SIMD instructions implemented via tcg API, or perhaps some of them remain being implemented using helpers?
>
> - Most of SIMD instructions can be viewed simply as several paralel elementary operations. However, for a given SIMD instruction set, usually not all of them fit into this pattern. For example, "horizontal add" (addind data elements from the same SIMD register), various "pack/unpack/interleave/merge" operations, and more general "shuffle/permute" operations as well (here I am not sure which of these are included in Hexagon SIMD set, but there must be some). How did you deal with them?
>
> - What were the most challenging Hexagon SIMD instructions you came accross while developing your solution?
>
> Sincerely,
> Aleksandar
>
>
>
>
> Thanks,
> Taylor


--
Alex Bennée


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-13 10:31       ` Alex Bennée
@ 2019-11-13 19:31         ` Taylor Simpson
  2019-11-13 21:10           ` Richard Henderson
  2019-11-13 21:27         ` Peter Maydell
  1 sibling, 1 reply; 18+ messages in thread
From: Taylor Simpson @ 2019-11-13 19:31 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo, Aleksandar Markovic

Responses below ...

Taylor


Taylor Simpson <tsimpson@quicinc.com> writes:

> I had discussions with several people at the KVM Forum, and I’ve been thinking about how to divide up the code for community review.  Here is my proposal for the steps.
>
>   1.  linux-user changes + linux-user/hexagon + skeleton of
> target/hexagon This is the minimum amount to build and run a very simple program.  I
>   have an assembly program that prints “Hello” and exits.  It is
>   constructed to use very few instructions that can be added brute
>   force in the Hexagon back end.

I'm hoping most of the linux-user changes are in the hexagon runloop?
There has been quite a bit of work splitting up and cleaning up the #ifdef mess in linux-user over the last few years.

[Taylor] The majority of the linux-user support is in linux-user/hexagon.  However, there were still a few changes needed in the linux-user directory.
    elfload.c Needs some code to match some existing #ifdef TARGET_xyz code (e.g., define the init_thread function).
    signal.c Needs some code to map signal 33 from the guest to something else on the target.
                  I spoke to Laurent about this at the converence.
    syscall.c Needs a definition of regpairs_aligned that returns 1.
    Ssscall_defs.h Needs some definitions (e.g., TARGET_IOC_SIZEBITS and target_stat) added to the other #ifdef TARGET_xys blocks.

>   2.  Add the code that is imported from the Hexagon simulator and the
> qemu helper generator This will allow the scalar ISA to be executed.
> This will grow the set of programs that could execute, but there will still be limitations.
> In particular, there can be no packets which means the C library won’t
> work .  We have to build with -nostdlib

You could run -nostdlib system TCG tests (hello and memory) but that would require modelling some sort of hardware and assumes you have a simple serial port or semihosting solution. That said a bunch of the MIPS tests are linux-user and -nostdlib so that isn't a major problem in getting some of the tests running.

When you say code imported from the hexagon simulator I was under the impression you were generating code from the instruction description.
Otherwise you'll need to be very clear about your licensing grants.

[Taylor] That is correct, we are generating code from the description.  There are two major pieces that are imported
    Instruction decode logic
    Any additional functions that are called from the generated code

[Taylor] All of the code will be licensed the same way.  I want to mark the imported code because it does not conform to the qemu coding standards.  I prefer not to reformat it in order to easily get bug fixes and enhancements going forward.  I also hope it will make the community review easier by allowing people to focus on the code that is new for qemu.

>   3.  Add support for packet semantics At this point, we will be able
> to execute full programs linked with the C library.  This will include
> the check-tcg tests.

I think the interesting question is if the roll-back semantics of the hexagon are something we might need for other emulated architectures or is a particularly specific solution for Hexagon (I'm guessing the later).

[Taylor] It is currently Hexagon-specific and isolated into the target/hexagon directory.  I was thinking the reviewers would have an easier time understanding the code if this were broken out.  However, it could also be merged together with step 2 if that is preferred.

>   4.  Add support for the wide vector extensions
>   5.  Add the helper overrides for performance optimization Some of
> these will be written by hand, and we’ll work with rev.ng to
>   integrate their flex/bison generator.

One thing to nail down will be will we include the generated code in the source tree with a tool to regenerate (much like we do for
linux-headers) or if we want to add the dependency and regenerate each time from scratch. I don't see including flex/bison as a dependency being a major issue (in fact we have it in our docker images so I guess something uses it). However it might be trickier depending on libclang which was also being discussed.

[Taylor] Currently, I have the generator and the generated code sitting in the source tree.  I'm flexible on this if the decision is to regenerate it every time.

>
> I would love some feedback on this proposal.  Hopefully, that is enough detail so that people can comment.  If anything isn’t clear, please ask questions.
>
>
> Thanks,
> Taylor
>
>
> From: Qemu-devel <qemu-devel-bounces+tsimpson=quicinc.com@nongnu.org>
> On Behalf Of Taylor Simpson
> Sent: Tuesday, November 5, 2019 10:33 AM
> To: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
> Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng;
> qemu-devel@nongnu.org; Niccolò Izzo <izzoniccolo@gmail.com>
> Subject: RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code
> available
>
> Hi Aleksandar,
>
> Thank you – We’re glad you enjoyed the talk.
>
> One point of clarification on SIMD in Hexagon.  What we refer to as the “scalar” core does have some SIMD operations.  Register pairs are 8 bytes, and there are several SIMD instructions.  The example we showed in the talk included a VADDH instruction.  It treats the register pair as 4 half-words and does a vector add.  Then there are the Hexagon Vector eXtensions (HVX) instructions that operate on 128-byte vectors.  There is a wide variety of instructions in this set.  As you mentioned, some of them are pure SIMD and others are very complex.
>
> For the helper generator, the vast majority of these are implemented with helpers.  There are only 2 vector instructions in the scalar core that have a TCG override, and all of the HVX instructions are implemented with helpers.  If you are interested in a deeper dive, see below.
>
> Alessandro and Niccolo can comment on the flex/bison implementation.
>
> Thanks,
> Taylor
>
>
> Now for the deeper dive in case anyone is interested.  Look at the genptr.c file in target/hexagon.
>
> The first vector instruction that is with an override is A6_vminub_RdP.  It does a byte-wise comparison of two register pairs and sets a predicate register indicating whether the byte in the left or right operand is greater.  Here is the TCG code.
> #define fWRAP_A6_vminub_RdP(GENHLPR, SHORTCODE) \ { \
>     TCGv BYTE = tcg_temp_new(); \
>     TCGv left = tcg_temp_new(); \
>     TCGv right = tcg_temp_new(); \
>     TCGv tmp = tcg_temp_new(); \
>     int i; \
>     tcg_gen_movi_tl(PeV, 0); \
>     tcg_gen_movi_i64(RddV, 0); \
>     for (i = 0; i < 8; i++) { \
>         fGETUBYTE(i, RttV); \
>         tcg_gen_mov_tl(left, BYTE); \
>         fGETUBYTE(i, RssV); \
>         tcg_gen_mov_tl(right, BYTE); \
>         tcg_gen_setcond_tl(TCG_COND_GT, tmp, left, right); \
>         fSETBIT(i, PeV, tmp); \
>         fMIN(tmp, left, right); \
>         fSETBYTE(i, RddV, tmp); \
>     } \
>     tcg_temp_free(BYTE); \
>     tcg_temp_free(left); \
>     tcg_temp_free(right); \
>     tcg_temp_free(tmp); \
> }
>
> The second instruction is S2_vsplatrb.  It takes the byte from the operand and replicates it 4 times into the destination register.  Here is the TCG code.
> #define fWRAP_S2_vsplatrb(GENHLPR, SHORTCODE) \ { \
>     TCGv tmp = tcg_temp_new(); \
>     int i; \
>     tcg_gen_movi_tl(RdV, 0); \
>     tcg_gen_andi_tl(tmp, RsV, 0xff); \
>     for (i = 0; i < 4; i++) { \
>         tcg_gen_shli_tl(RdV, RdV, 8); \
>         tcg_gen_or_tl(RdV, RdV, tmp); \
>     } \
>     tcg_temp_free(tmp); \
> }
>
>
> From: Aleksandar Markovic <aleksandar.m.mail@gmail.com<mailto:aleksandar.m.mail@gmail.com>>
> Sent: Monday, November 4, 2019 6:05 PM
> To: Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>>
> Cc: qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>; Alessandro Di Federico <ale@rev.ng<mailto:ale@rev.ng>>; nizzo@rev.ng<mailto:nizzo@rev.ng>; Niccolò Izzo <izzoniccolo@gmail.com<mailto:izzoniccolo@gmail.com>>
> Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
>
>
> CAUTION: This email originated from outside of the organization.
>
>
> On Friday, October 25, 2019, Taylor Simpson <tsimpson@quicinc.com<mailto:tsimpson@quicinc.com>> wrote:
> We would like inform the you that we will be doing a talk at the KVM Forum next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo Izzo, and I have been working independently on implementations of the Hexagon target.  We plan to merge the implementations, have a community review, and ultimately have Hexagon be an official target in QEMU.  Our code is available at the links below.
> https://github.com/revng/qemu-hexagon
> https://github.com/quic/qemu
> If anyone has any feedback on the code as it stands today or guidance on how best to prepare it for review, please let us know.
>
>
> Hi, Taylor, Niccolo (and Alessandro too).
>
> I didn't have a chance to take a look at neither the code nor the docs, but I did attend you presentation at KVM Forum, and I found it superb and attractive, one of the best on the conference, if not the very best.
>
> I just have a couple of general questions:
>
> - Regarding the code you plan to upstream, are all SIMD instructions implemented via tcg API, or perhaps some of them remain being implemented using helpers?
>
> - Most of SIMD instructions can be viewed simply as several paralel elementary operations. However, for a given SIMD instruction set, usually not all of them fit into this pattern. For example, "horizontal add" (addind data elements from the same SIMD register), various "pack/unpack/interleave/merge" operations, and more general "shuffle/permute" operations as well (here I am not sure which of these are included in Hexagon SIMD set, but there must be some). How did you deal with them?
>
> - What were the most challenging Hexagon SIMD instructions you came accross while developing your solution?
>
> Sincerely,
> Aleksandar
>
>
>
>
> Thanks,
> Taylor


--
Alex Bennée

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-13 19:31         ` Taylor Simpson
@ 2019-11-13 21:10           ` Richard Henderson
  2019-11-15  0:54             ` Taylor Simpson
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2019-11-13 21:10 UTC (permalink / raw)
  To: Taylor Simpson, Alex Bennée, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo, Aleksandar Markovic

On 11/13/19 8:31 PM, Taylor Simpson wrote:
> [Taylor] Currently, I have the generator and the generated code sitting in the source tree.  I'm flexible on this if the decision is to regenerate it every time.

I would prefer to regenerate every time, and not store the generated code in
the source tree at all.  It makes it a no-brainer to modify the source and not
have to remember how to regenerate, because the rules are right there in the
makefile.


r~


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-13 10:31       ` Alex Bennée
  2019-11-13 19:31         ` Taylor Simpson
@ 2019-11-13 21:27         ` Peter Maydell
  1 sibling, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2019-11-13 21:27 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Alessandro Di Federico, QEMU Developers, Niccolò Izzo,
	Taylor Simpson, nizzo, Aleksandar Markovic

On Wed, 13 Nov 2019 at 10:32, Alex Bennée <alex.bennee@linaro.org> wrote:
> I don't see including flex/bison as a dependency
> being a major issue (in fact we have it in our docker images so I guess
> something uses it).

They're used by the dtc submodule, so only in setups where you
need to use the submodule rather than the system libfdt.
In fact I think that dtc doesn't require them for building
libfdt, but its build machinery complains about them being
missing (it needs them for building the 'dtc' binary, which
we don't try to build), so providing them shuts up the
misleading warning.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-13 21:10           ` Richard Henderson
@ 2019-11-15  0:54             ` Taylor Simpson
  2019-12-17 18:14               ` Taylor Simpson
  0 siblings, 1 reply; 18+ messages in thread
From: Taylor Simpson @ 2019-11-15  0:54 UTC (permalink / raw)
  To: Richard Henderson, Alex Bennée, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo, Aleksandar Markovic

OK, I changed the code in github and will upstream it that way.



-----Original Message-----
From: Richard Henderson <richard.henderson@linaro.org>
Sent: Wednesday, November 13, 2019 3:10 PM
To: Taylor Simpson <tsimpson@quicinc.com>; Alex Bennée <alex.bennee@linaro.org>; qemu-devel@nongnu.org
Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; Niccolò Izzo <izzoniccolo@gmail.com>; Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available

-------------------------------------------------------------------------
CAUTION: This email originated from outside of the organization.
-------------------------------------------------------------------------

On 11/13/19 8:31 PM, Taylor Simpson wrote:
> [Taylor] Currently, I have the generator and the generated code sitting in the source tree.  I'm flexible on this if the decision is to regenerate it every time.

I would prefer to regenerate every time, and not store the generated code in the source tree at all.  It makes it a no-brainer to modify the source and not have to remember how to regenerate, because the rules are right there in the makefile.


r~

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-12 22:52     ` Taylor Simpson
  2019-11-13 10:31       ` Alex Bennée
@ 2019-11-19  9:12       ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-19  9:12 UTC (permalink / raw)
  To: Alessandro Di Federico, nizzo, Niccolò Izzo
  Cc: Peter Maydell, qemu-devel, Taylor Simpson, Aleksandar Markovic,
	Alex Bennée, Luc Michel, Richard Henderson

Hi Alessandro and Niccolò,

On Tue, Nov 12, 2019 at 11:54 PM Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> I had discussions with several people at the KVM Forum, and I’ve been thinking about how to divide up the code for community review.  Here is my proposal for the steps.
>
> linux-user changes + linux-user/hexagon + skeleton of target/hexagon
> This is the minimum amount to build and run a very simple program.  I have an assembly program that prints “Hello” and exits.  It is constructed to use very few instructions that can be added brute force in the Hexagon back end.
> Add the code that is imported from the Hexagon simulator and the qemu helper generator
> This will allow the scalar ISA to be executed.  This will grow the set of programs that could execute, but there will still be limitations.  In particular, there can be no packets which means the C library won’t work .  We have to build with -nostdlib
> Add support for packet semantics
> At this point, we will be able to execute full programs linked with the C library.  This will include the check-tcg tests.
> Add support for the wide vector extensions
> Add the helper overrides for performance optimization
> Some of these will be written by hand, and we’ll work with rev.ng to integrate their flex/bison generator.

Few years ago Luc Michel added the TMS320C6x.
The original git repository is down, I saved a copy:
https://github.com/philmd/qemu/commit/44a32515d
Last time I checked I had it rebased on QEMU 2.8.

I wonder if rev.ng flex/bison generator as it would also work with
this architecture (obviously I mean with the VLIW 'backend' logic, not
the Hexagon 'frontend').

Regards,

Phil.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-11-15  0:54             ` Taylor Simpson
@ 2019-12-17 18:14               ` Taylor Simpson
  2019-12-17 18:19                 ` Thomas Huth
  2019-12-17 18:21                 ` Peter Maydell
  0 siblings, 2 replies; 18+ messages in thread
From: Taylor Simpson @ 2019-12-17 18:14 UTC (permalink / raw)
  To: Taylor Simpson, Richard Henderson, Alex Bennée,
	Aleksandar Markovic, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo

Hi again,

Per Aleksandar's feedback, I'm finishing up the rewrite of the code imported from the existing Hexagon simulator.  Once I'm finished with that work, I'll start breaking down the code into a patch series.  I have a couple of questions I hope someone would be kind enough to answer.

Question 1:
I see this error from checkpatch.pl
ERROR: Macros with complex values should be enclosed in parenthesis
However, there are times when the code will not compile with parenthesis.  For example, we have a file that defined all the instruction attributes.  Each line has
DEF_ATTRIB(LOAD, "Loads from memory", "", "")
So, we create an enum of all the possible attributes as follows
enum {
#define DEF_ATTRIB(NAME, ...) A_##NAME,
#include "attribs_def.h"
#undef DEF_ATTRIB
};
This is one simple example.  We also things like create an array of values or a series of small functions or cases in a switch statement.

Question 2:
What is the best source of guidance on breaking down support for a new target into a patch series?  I see avr being reviewed currently.  I have mostly new files: 12 in linux-user/hexagon, and ~50 in target/hexagon.  I also need to add test cases and a container for the toolchain.  Is it OK to break things down mostly at file boundaries?  In some cases small files could be combined, and large files would be broken down into reviewable chunks.  Late in the series, I can introduce the changes to common code.  The implication of having the common code changes late is that code would compile and run only at the end of the series.

Thanks,
Taylor


-----Original Message-----
From: Qemu-devel <qemu-devel-bounces+tsimpson=quicinc.com@nongnu.org> On Behalf Of Taylor Simpson
Sent: Thursday, November 14, 2019 6:54 PM
To: Richard Henderson <richard.henderson@linaro.org>; Alex Bennée <alex.bennee@linaro.org>; qemu-devel@nongnu.org
Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; Niccolò Izzo <izzoniccolo@gmail.com>; Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Subject: RE: QEMU for Qualcomm Hexagon - KVM Forum talk and code available

OK, I changed the code in github and will upstream it that way.



-----Original Message-----
From: Richard Henderson <richard.henderson@linaro.org>
Sent: Wednesday, November 13, 2019 3:10 PM
To: Taylor Simpson <tsimpson@quicinc.com>; Alex Bennée <alex.bennee@linaro.org>; qemu-devel@nongnu.org
Cc: Alessandro Di Federico <ale@rev.ng>; nizzo@rev.ng; Niccolò Izzo <izzoniccolo@gmail.com>; Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Subject: Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available

On 11/13/19 8:31 PM, Taylor Simpson wrote:
> [Taylor] Currently, I have the generator and the generated code sitting in the source tree.  I'm flexible on this if the decision is to regenerate it every time.

I would prefer to regenerate every time, and not store the generated code in the source tree at all.  It makes it a no-brainer to modify the source and not have to remember how to regenerate, because the rules are right there in the makefile.


r~

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-12-17 18:14               ` Taylor Simpson
@ 2019-12-17 18:19                 ` Thomas Huth
  2019-12-17 18:21                 ` Peter Maydell
  1 sibling, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2019-12-17 18:19 UTC (permalink / raw)
  To: Taylor Simpson, Richard Henderson, Alex Bennée,
	Aleksandar Markovic, qemu-devel
  Cc: Alessandro Di Federico, nizzo, Niccolò Izzo

On 17/12/2019 19.14, Taylor Simpson wrote:
> Hi again,
> 
> Per Aleksandar's feedback, I'm finishing up the rewrite of the code imported from the existing Hexagon simulator.  Once I'm finished with that work, I'll start breaking down the code into a patch series.  I have a couple of questions I hope someone would be kind enough to answer.
> 
> Question 1:
> I see this error from checkpatch.pl
> ERROR: Macros with complex values should be enclosed in parenthesis
> However, there are times when the code will not compile with parenthesis.

checkpatch.pl sometimes reports false positives - or things that do not
make sense in all cases, like the parenthesis here. You can simply
ignore the warnings / errors in that case.

 Thomas



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-12-17 18:14               ` Taylor Simpson
  2019-12-17 18:19                 ` Thomas Huth
@ 2019-12-17 18:21                 ` Peter Maydell
  2019-12-17 18:41                   ` Philippe Mathieu-Daudé
  2019-12-18 23:50                   ` Richard Henderson
  1 sibling, 2 replies; 18+ messages in thread
From: Peter Maydell @ 2019-12-17 18:21 UTC (permalink / raw)
  To: Taylor Simpson
  Cc: Alessandro Di Federico, Richard Henderson, qemu-devel,
	Niccolò Izzo, nizzo, Alex Bennée, Aleksandar Markovic

On Tue, 17 Dec 2019 at 18:16, Taylor Simpson <tsimpson@quicinc.com> wrote:
> Question 1:
> I see this error from checkpatch.pl
> ERROR: Macros with complex values should be enclosed in parenthesis
> However, there are times when the code will not compile with parenthesis.  For example, we have a file that defined all the instruction attributes.  Each line has
> DEF_ATTRIB(LOAD, "Loads from memory", "", "")
> So, we create an enum of all the possible attributes as follows
> enum {
> #define DEF_ATTRIB(NAME, ...) A_##NAME,
> #include "attribs_def.h"
> #undef DEF_ATTRIB
> };

checkpatch is often right, but also often wrong,
especially for C macros which are in the general case
impossible to parse. If the error makes no sense, you can
ignore it.

> Question 2:
> What is the best source of guidance on breaking down support for a new target into a patch series?

Look at how previous ports did it. Also I thought we'd
had a subthread on how best to split things up, but maybe I'm
misremembering.

>  I see avr being reviewed currently.  I have mostly new files: 12 in linux-user/hexagon, and ~50 in target/hexagon.  I also need to add test cases and a container for the toolchain.  Is it OK to break things down mostly at file boundaries?

No, file boundaries are generally a bad choice of breakdown.
You want to split at conceptual boundaries, ie one chunk
of functionality that can be comprehended in one go without
having to refer forward to other patches.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-12-17 18:21                 ` Peter Maydell
@ 2019-12-17 18:41                   ` Philippe Mathieu-Daudé
  2019-12-17 18:44                     ` Philippe Mathieu-Daudé
  2019-12-18 23:50                   ` Richard Henderson
  1 sibling, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-17 18:41 UTC (permalink / raw)
  To: Peter Maydell, Taylor Simpson
  Cc: Alessandro Di Federico, Richard Henderson, qemu-devel,
	Niccolò Izzo, nizzo, Alex Bennée, Aleksandar Markovic

On 12/17/19 7:21 PM, Peter Maydell wrote:
> On Tue, 17 Dec 2019 at 18:16, Taylor Simpson <tsimpson@quicinc.com> wrote:
>> Question 1:
>> I see this error from checkpatch.pl
>> ERROR: Macros with complex values should be enclosed in parenthesis
>> However, there are times when the code will not compile with parenthesis.  For example, we have a file that defined all the instruction attributes.  Each line has
>> DEF_ATTRIB(LOAD, "Loads from memory", "", "")
>> So, we create an enum of all the possible attributes as follows
>> enum {
>> #define DEF_ATTRIB(NAME, ...) A_##NAME,
>> #include "attribs_def.h"
>> #undef DEF_ATTRIB
>> };
> 
> checkpatch is often right, but also often wrong,
> especially for C macros which are in the general case
> impossible to parse. If the error makes no sense, you can
> ignore it.
> 
>> Question 2:
>> What is the best source of guidance on breaking down support for a new target into a patch series?
> 
> Look at how previous ports did it.

Recent ports were system (softmmu), this is a linux-user port. The last 
architecture merged is RISCV, they did that with commit, so I'm not sure 
this is our best example on breaking down:

$ git show --stat ea10325917c8
commit ea10325917c8a8f92611025c85950c00f826cb73
Author: Michael Clark <mjc@sifive.com>
Date:   Sat Mar 3 01:31:10 2018 +1300

     RISC-V Disassembler

     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

     Expected checkpatch errors for consistency and brevity reasons:

     ERROR: line over 90 characters
     ERROR: trailing statements should be on next line
     ERROR: space prohibited between function name and open parenthesis '('

     Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
     Signed-off-by: Michael Clark <mjc@sifive.com>

  include/disas/bfd.h |    2 +
  disas.c             |    2 +
  disas/riscv.c       | 3048 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  disas/Makefile.objs |    1 +
  4 files changed, 3053 insertions(+)

$ git show --stat 55c2a12cbcd3d
commit 55c2a12cbcd3d417de39ee82dfe1d26b22a07116
Author: Michael Clark <mjc@sifive.com>
Date:   Sat Mar 3 01:31:11 2018 +1300

     RISC-V TCG Code Generation

     TCG code generation for the RV32IMAFDC and RV64IMAFDC. The QEMU
     RISC-V code generator has complete coverage for the Base ISA v2.2,
     Privileged ISA v1.9.1 and Privileged ISA v1.10:

     - RISC-V Instruction Set Manual Volume I: User-Level ISA Version 2.2
     - RISC-V Instruction Set Manual Volume II: Privileged ISA Version 1.9.1
     - RISC-V Instruction Set Manual Volume II: Privileged ISA Version 1.10

     Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
     Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
     Signed-off-by: Sagar Karandikar <sagark@eecs.berkeley.edu>
     Signed-off-by: Michael Clark <mjc@sifive.com>

  target/riscv/instmap.h   |  364 ++++++++++++++++++++++++++++++++++++++
  target/riscv/translate.c | 1978 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 2342 insertions(+)

$ git show --stat 47ae93cdfed
commit 47ae93cdfedc683c56e19113d516d7ce4971c8e6
Author: Michael Clark <mjc@sifive.com>
Date:   Sat Mar 3 01:31:11 2018 +1300

     RISC-V Linux User Emulation

     Implementation of linux user emulation for RISC-V.

     Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
     Signed-off-by: Sagar Karandikar <sagark@eecs.berkeley.edu>
     Signed-off-by: Michael Clark <mjc@sifive.com>

  linux-user/riscv/syscall_nr.h     | 287 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  linux-user/riscv/target_cpu.h     |  18 +++++++++++++
  linux-user/riscv/target_elf.h     |  14 ++++++++++
  linux-user/riscv/target_signal.h  |  23 ++++++++++++++++
  linux-user/riscv/target_structs.h |  46 ++++++++++++++++++++++++++++++++
  linux-user/riscv/target_syscall.h |  56 
++++++++++++++++++++++++++++++++++++++
  linux-user/riscv/termbits.h       | 222 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  linux-user/syscall_defs.h         |  13 +++++----
  target/riscv/cpu_user.h           |  13 +++++++++
  linux-user/elfload.c              |  22 +++++++++++++++
  linux-user/main.c                 |  99 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  linux-user/signal.c               | 203 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
  linux-user/syscall.c              |   2 ++
  13 files changed, 1012 insertions(+), 6 deletions(-)


> Also I thought we'd
> had a subthread on how best to split things up, but maybe I'm
> misremembering.

I remember something too, I hope you are right :P

>>   I see avr being reviewed currently.  I have mostly new files: 12 in linux-user/hexagon, and ~50 in target/hexagon.  I also need to add test cases and a container for the toolchain.  Is it OK to break things down mostly at file boundaries?
> 
> No, file boundaries are generally a bad choice of breakdown.
> You want to split at conceptual boundaries, ie one chunk
> of functionality that can be comprehended in one go without
> having to refer forward to other patches.
> 
> thanks
> -- PMM
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-12-17 18:41                   ` Philippe Mathieu-Daudé
@ 2019-12-17 18:44                     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-17 18:44 UTC (permalink / raw)
  To: Peter Maydell, Taylor Simpson
  Cc: Alessandro Di Federico, Richard Henderson, qemu-devel,
	Niccolò Izzo, nizzo, Alex Bennée, Aleksandar Markovic

On Tue, Dec 17, 2019 at 7:41 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 12/17/19 7:21 PM, Peter Maydell wrote:
> > On Tue, 17 Dec 2019 at 18:16, Taylor Simpson <tsimpson@quicinc.com> wrote:
> >> Question 2:
> >> What is the best source of guidance on breaking down support for a new target into a patch series?
> >
> > Look at how previous ports did it.
>
> Recent ports were system (softmmu), this is a linux-user port. The last
> architecture merged is RISCV, they did that with commit, so I'm not sure
> this is our best example on breaking down:
>
> $ git show --stat ea10325917c8
> commit ea10325917c8a8f92611025c85950c00f826cb73
> Author: Michael Clark <mjc@sifive.com>
> Date:   Sat Mar 3 01:31:10 2018 +1300
>
>      RISC-V Disassembler
>
>      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
>
>      Expected checkpatch errors for consistency and brevity reasons:
>
>      ERROR: line over 90 characters
>      ERROR: trailing statements should be on next line
>      ERROR: space prohibited between function name and open parenthesis '('
>
>      Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>      Signed-off-by: Michael Clark <mjc@sifive.com>
>
>   include/disas/bfd.h |    2 +
>   disas.c             |    2 +
>   disas/riscv.c       | 3048
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   disas/Makefile.objs |    1 +
>   4 files changed, 3053 insertions(+)
>
> $ git show --stat 55c2a12cbcd3d
> commit 55c2a12cbcd3d417de39ee82dfe1d26b22a07116
> Author: Michael Clark <mjc@sifive.com>
> Date:   Sat Mar 3 01:31:11 2018 +1300
>
>      RISC-V TCG Code Generation
>
>      TCG code generation for the RV32IMAFDC and RV64IMAFDC. The QEMU
>      RISC-V code generator has complete coverage for the Base ISA v2.2,
>      Privileged ISA v1.9.1 and Privileged ISA v1.10:
>
>      - RISC-V Instruction Set Manual Volume I: User-Level ISA Version 2.2
>      - RISC-V Instruction Set Manual Volume II: Privileged ISA Version 1.9.1
>      - RISC-V Instruction Set Manual Volume II: Privileged ISA Version 1.10
>
>      Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>      Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>      Signed-off-by: Sagar Karandikar <sagark@eecs.berkeley.edu>
>      Signed-off-by: Michael Clark <mjc@sifive.com>
>
>   target/riscv/instmap.h   |  364 ++++++++++++++++++++++++++++++++++++++
>   target/riscv/translate.c | 1978
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 2342 insertions(+)
>
> $ git show --stat 47ae93cdfed
> commit 47ae93cdfedc683c56e19113d516d7ce4971c8e6
> Author: Michael Clark <mjc@sifive.com>
> Date:   Sat Mar 3 01:31:11 2018 +1300
>
>      RISC-V Linux User Emulation
>
>      Implementation of linux user emulation for RISC-V.
>
>      Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>      Signed-off-by: Sagar Karandikar <sagark@eecs.berkeley.edu>
>      Signed-off-by: Michael Clark <mjc@sifive.com>
>
>   linux-user/riscv/syscall_nr.h     | 287
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   linux-user/riscv/target_cpu.h     |  18 +++++++++++++
>   linux-user/riscv/target_elf.h     |  14 ++++++++++
>   linux-user/riscv/target_signal.h  |  23 ++++++++++++++++
>   linux-user/riscv/target_structs.h |  46 ++++++++++++++++++++++++++++++++
>   linux-user/riscv/target_syscall.h |  56
> ++++++++++++++++++++++++++++++++++++++
>   linux-user/riscv/termbits.h       | 222
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   linux-user/syscall_defs.h         |  13 +++++----
>   target/riscv/cpu_user.h           |  13 +++++++++
>   linux-user/elfload.c              |  22 +++++++++++++++
>   linux-user/main.c                 |  99
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   linux-user/signal.c               | 203
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   linux-user/syscall.c              |   2 ++
>   13 files changed, 1012 insertions(+), 6 deletions(-)

I sent too quick. You can read a summary of the different review
comments before the final merge in tag 'riscv-qemu-upstream-v8.2'.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available
  2019-12-17 18:21                 ` Peter Maydell
  2019-12-17 18:41                   ` Philippe Mathieu-Daudé
@ 2019-12-18 23:50                   ` Richard Henderson
  1 sibling, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2019-12-18 23:50 UTC (permalink / raw)
  To: Peter Maydell, Taylor Simpson
  Cc: Alessandro Di Federico, qemu-devel, Niccolò Izzo, nizzo,
	Alex Bennée, Aleksandar Markovic

On 12/17/19 8:21 AM, Peter Maydell wrote:
> On Tue, 17 Dec 2019 at 18:16, Taylor Simpson <tsimpson@quicinc.com> wrote:
>> Question 2:
>> What is the best source of guidance on breaking down support for a new target into a patch series?
> 
> Look at how previous ports did it.

E.g. the hppa-linux-user port.  The initial merge ends at ebe9383caefd.

There are 15 patches -- mostly in linux-user -- before the one that enables
compilation.  At which point exactly zero instructions are actually implemented.

The actual cpu emulation comes afterwards in 8 patches (hppa 1.1 isn't terribly
complicated).


r~


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-12-18 23:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25 16:26 QEMU for Qualcomm Hexagon - KVM Forum talk and code available Taylor Simpson
2019-11-01 18:29 ` Philippe Mathieu-Daudé
2019-11-04  2:35   ` Taylor Simpson
2019-11-05  0:05 ` Aleksandar Markovic
2019-11-05 16:32   ` Taylor Simpson
2019-11-12 22:52     ` Taylor Simpson
2019-11-13 10:31       ` Alex Bennée
2019-11-13 19:31         ` Taylor Simpson
2019-11-13 21:10           ` Richard Henderson
2019-11-15  0:54             ` Taylor Simpson
2019-12-17 18:14               ` Taylor Simpson
2019-12-17 18:19                 ` Thomas Huth
2019-12-17 18:21                 ` Peter Maydell
2019-12-17 18:41                   ` Philippe Mathieu-Daudé
2019-12-17 18:44                     ` Philippe Mathieu-Daudé
2019-12-18 23:50                   ` Richard Henderson
2019-11-13 21:27         ` Peter Maydell
2019-11-19  9:12       ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).