Re: [Qemu-devel] [PATCH v1 1/2] tcg: Add support for constant value promises

[Lluís Vilanova]

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.


Cheers,
  Lluis