All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
To: "Lluís Vilanova" <vilanova@ac.upc.edu>
Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH v1 1/2] tcg: Add support for constant value promises
Date: Tue, 19 Jan 2016 18:00:29 +0100	[thread overview]
Message-ID: <20160119170029.GS29396@toto> (raw)
In-Reply-To: <87pox1xmwv.fsf@fimbulvetr.bsc.es>

On Sat, Jan 16, 2016 at 09:57:36PM +0100, Lluís Vilanova wrote:
> Richard Henderson writes:
> 
> > On 01/15/2016 12:12 PM, Lluís Vilanova wrote:
> >> Richard Henderson writes:
> >> 
> >>> On 01/15/2016 07:35 AM, Lluís Vilanova wrote:
> >>>> +TCGv_i64 tcg_promise_i64(TCGv_promise_i64 *promise)
> >>>> +{
> >>>> +    int pi = tcg_ctx.gen_next_parm_idx;
> >>>> +    *promise = (TCGv_promise_i64)&tcg_ctx.gen_opparam_buf[pi];
> >>>> +    return tcg_const_i64(0xdeadcafe);
> >>>> +}
> >> 
> >>> This doesn't work for a 32-bit host.  The constant may be split across two
> >>> different parameter indices, and you don't know exactly where the second will be.
> >> 
> >>> Because of that, I think this is over-engineered, and really prefer the simpler
> >>> interface that Edgar posted last week.
> >> 
> >> In this case, 'tcg_set_promise_i64' sets the two arguments accordingly on 32-bit
> >> targets. Both solutions depend on TCG internals (in this specific case the
> >> implementation of 'tcg_gen_movi_i64'), but now it's all implemented inside TCG.
> >> 
> >> Alternatively, promises could use the longer route of recording the opcode index
> >> (as Edgar did AFAIR), and retrieve the argument pointer from there. Still, for
> >> 32-bit targets we have to assume the two immediate moves are gonna generate two
> >> consecutive opcodes.
> 
> > Your solution also doesn't help Edgar, since he's interested in modifying an
> > argument to the insn_start opcode, not modifying a literal constant in a move.
> 
> I wasn't aware of that. If the idea was to use this for more than immediates
> stored in TCGv values, I see two options. First, modify the necessary opcodes to
> use a TCGv argument instead of an immediate. Second, generalize this patch to
> to select any opcode argument.
> 
> An example of the generalization when used to reimplement icount:
> 
>    // insn count placeholder
>    TCGv_i32 imm = tcg_const_i32(0xcafecafe);
>    // insn count promise
>    TCGv_promise_i32 imm_promise = tcg_promise_i32(
>        1,  // how many opcodes to go "backwards"
>        1); // what argument to modify on that opcode
>    // operate with imm
>    ...
>    // resolve value
>    tcg_set_promise_i32(imm_promise, insn_count);
> 
> The question still stands on how to cleanly handle promises for opcodes like a
> 64-bit mov on a 32-bit host (it's generated as two opcodes). Using this
> interface would still be cleaner than directly manipulating the low-level TCG
> arrays, and makes it easier to adopt it in future changes.
>

Thanks Lluis and Richard,

I'll stay with my version for the first try at the ARM load/store fault
reporting. If something better comes along that works for me, I'm happy
to change.

Richard if you want to take the patches through your tree feel free to
do so. Otherwise, I'll post them again with more context and try through
the ARM queue.

Best regards,
Edgar 

  reply	other threads:[~2016-01-19 17:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-15 15:34 [Qemu-devel] [PATCH v1 0/2] tcg: Add support for constant value promises Lluís Vilanova
2016-01-15 15:35 ` [Qemu-devel] [PATCH v1 1/2] " Lluís Vilanova
2016-01-15 18:20   ` Richard Henderson
2016-01-15 20:12     ` Lluís Vilanova
2016-01-15 20:26       ` Richard Henderson
2016-01-16 20:57         ` Lluís Vilanova
2016-01-19 17:00           ` Edgar E. Iglesias [this message]
2016-01-19 21:17             ` Lluís Vilanova
2016-01-15 15:35 ` [Qemu-devel] [PATCH v1 2/2] gen-icount: Use " Lluís Vilanova

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=20160119170029.GS29396@toto \
    --to=edgar.iglesias@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=vilanova@ac.upc.edu \
    /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.