Edgar E Iglesias writes: > 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. My offer still stands. If the generalized interface seems adequate (specific opcode argument to set the promise for), it's a rather simple change on the series. Cheers, Lluis