All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Simpson <tsimpson@quicinc.com>
To: Paolo Montesel <paolo.montesel.revng@gmail.com>
Cc: Alessandro Di Federico <ale@rev.ng>,
	Brian Cain <bcain@quicinc.com>,
	"richard.henderson@linaro.org" <richard.henderson@linaro.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Alessandro Di Federico <ale.qemu@rev.ng>,
	"nizzo@rev.ng" <nizzo@rev.ng>,
	"philmd@redhat.com" <philmd@redhat.com>
Subject: RE: [PATCH v4 09/12] target/hexagon: import lexer for idef-parser
Date: Wed, 28 Apr 2021 15:55:01 +0000	[thread overview]
Message-ID: <BYAPR02MB4886605BF4386A88B443375EDE409@BYAPR02MB4886.namprd02.prod.outlook.com> (raw)
In-Reply-To: <CANnx7NNMoPxGeP2tda-0yBbvN3Donc+m2cFsr8ZaLg_JnZBQNA@mail.gmail.com>



>From: Paolo Montesel <paolo.montesel.revng@gmail.com> 
>Sent: Wednesday, April 28, 2021 5:25 AM
>To: Taylor Simpson <tsimpson@quicinc.com>
>Cc: Alessandro Di Federico <ale.qemu@rev.ng>; qemu-devel@nongnu.org; Brian Cain <bcain@quicinc.com>; nizzo@rev.ng; >philmd@redhat.com; richard.henderson@linaro.org; Alessandro Di Federico <ale@rev.ng>
>Subject: Re: [PATCH v4 09/12] target/hexagon: import lexer for idef-parser
>
>Thanks for spotting this. It's actually a bug in the lexer. The token `{IMM_ID}"iV"` didn't initialize `bit_width`. Now it does. This is the >result:
>
>void emit_J2_jump(DisasContext *ctx, Insn *insn, Packet *pkt, int riV)
>/* fIMMEXT(riV); (riV = riV & ~3); (PC = fREAD_PC()+riV);} */
>{
>int64_t qemu_tmp_0 = ~((int64_t)3ULL);
>int32_t qemu_tmp_1 = riV & qemu_tmp_0;
>riV = qemu_tmp_1;
>TCGv_i32 tmp_0 = tcg_temp_local_new_i32();
>tcg_gen_movi_i32(tmp_0, ctx->base.pc_next);
>TCGv_i32 tmp_1 = tcg_temp_local_new_i32();
>tcg_gen_addi_i32(tmp_1, tmp_0, (int64_t)riV);
>tcg_temp_free_i32(tmp_0);
>gen_write_new_pc(tmp_1);
>tcg_temp_free_i32(tmp_1);
>}
>
>The `(int64_t)riV` cast is actually useless so I simply dropped it, thanks for pointing it out.
>
>This is all gonna be in the next patchset ofc.
>
>~Paolo

This could be further simplified by doing the add in the parser and generating
    TCGv tmp_1 = tcg_const_tl(ctx->base.pc_next + riV);
Have you looked at the host code that is generated?  I would expect it to do the constant folding, so the executed code is OK.  However, there's extra time spent building up TCG that could be avoided.


Taylor


  reply	other threads:[~2021-04-28 16:09 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15 16:34 [PATCH v4 00/12] target/hexagon: introduce idef-parser Alessandro Di Federico via
2021-04-15 16:34 ` [PATCH v4 01/12] tcg: expose TCGCond manipulation routines Alessandro Di Federico via
2021-04-18 20:57   ` Richard Henderson
2021-04-15 16:34 ` [PATCH v4 02/12] target/hexagon: update MAINTAINERS for idef-parser Alessandro Di Federico via
2021-04-18 20:58   ` Richard Henderson
2021-04-15 16:34 ` [PATCH v4 03/12] target/hexagon: import README " Alessandro Di Federico via
2021-04-15 16:34 ` [PATCH v4 04/12] target/hexagon: make slot number an unsigned Alessandro Di Federico via
2021-04-18 21:10   ` Richard Henderson
2021-04-15 16:34 ` [PATCH v4 05/12] target/hexagon: make helper functions non-static Alessandro Di Federico via
2021-04-18 21:11   ` Richard Henderson
2021-04-15 16:34 ` [PATCH v4 06/12] target/hexagon: introduce new helper functions Alessandro Di Federico via
2021-04-18 21:31   ` Richard Henderson
2021-04-19 15:00     ` Taylor Simpson
2021-04-20  6:39       ` Alessandro Di Federico via
2021-04-15 16:34 ` [PATCH v4 07/12] target/hexagon: expose next PC in DisasContext Alessandro Di Federico via
2021-04-18 21:34   ` Richard Henderson
2021-04-15 16:34 ` [PATCH v4 08/12] target/hexagon: prepare input for the idef-parser Alessandro Di Federico via
2021-04-18 21:43   ` Richard Henderson
2021-04-15 16:34 ` [PATCH v4 09/12] target/hexagon: import lexer for idef-parser Alessandro Di Federico via
2021-04-27  2:01   ` Taylor Simpson
2021-04-28 10:25     ` Paolo Montesel
2021-04-28 15:55       ` Taylor Simpson [this message]
2021-04-29 10:53         ` Paolo Montesel
2021-04-15 16:34 ` [PATCH v4 10/12] target/hexagon: import parser " Alessandro Di Federico via
2021-04-15 16:34 ` [PATCH v4 11/12] target/hexagon: call idef-parser functions Alessandro Di Federico via
2021-04-29 15:59   ` Taylor Simpson
2021-04-15 16:34 ` [PATCH v4 12/12] target/hexagon: import additional tests Alessandro Di Federico via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR02MB4886605BF4386A88B443375EDE409@BYAPR02MB4886.namprd02.prod.outlook.com \
    --to=tsimpson@quicinc.com \
    --cc=ale.qemu@rev.ng \
    --cc=ale@rev.ng \
    --cc=bcain@quicinc.com \
    --cc=nizzo@rev.ng \
    --cc=paolo.montesel.revng@gmail.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.