| Message ID | 1442953507-4074-6-git-send-email-rth@twiddle.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 22, 2015 at 01:24:47PM -0700, Richard Henderson wrote: > With an eye toward having this data replace the gen_opc_* arrays > that each target collects in order to enable restore_state_from_tb. Hi Richard, Instead of having each architecture front-end determine the constants to be restored on an exception, have you considered having the tcg liveness pass automatically detect them? What I was thinking was if: - each front-end stored each constant on every instruction using regular "movi" ops - tcg_liveness_analysis() tracked which global memory "sync" writes are purely due to an op that can raise an exception - then tcg_liveness_analysis() could remove "movi" instructions with outputs that are needed only during an exception and place the constant directly in the compressed table itself. I'm curious if this was considered and if there is a reason it wouldn't work well. -Kevin
On 09/23/2015 07:55 AM, Kevin O'Connor wrote: > On Tue, Sep 22, 2015 at 01:24:47PM -0700, Richard Henderson wrote: >> With an eye toward having this data replace the gen_opc_* arrays >> that each target collects in order to enable restore_state_from_tb. > > Hi Richard, > > Instead of having each architecture front-end determine the constants > to be restored on an exception, have you considered having the tcg > liveness pass automatically detect them? > > What I was thinking was if: > - each front-end stored each constant on every instruction using > regular "movi" ops > - tcg_liveness_analysis() tracked which global memory "sync" writes > are purely due to an op that can raise an exception > - then tcg_liveness_analysis() could remove "movi" instructions with > outputs that are needed only during an exception and place the > constant directly in the compressed table itself. > > I'm curious if this was considered and if there is a reason it > wouldn't work well. We certainly don't have enough information to infer something like that. The moment we reach a helper that isn't marked as TCG_CALL_NO_WG, all that inference has to go out the window ans we have to assume that the "movi op" is both necessary and overwritten. The knowledge of which helpers modify a field such as cc_op is present into the translators in code form. It would require significant effort to change that. r~
On 09/23/2015 07:55 AM, Kevin O'Connor wrote: > On Tue, Sep 22, 2015 at 01:24:47PM -0700, Richard Henderson wrote: >> With an eye toward having this data replace the gen_opc_* arrays >> that each target collects in order to enable restore_state_from_tb. > > Hi Richard, > > Instead of having each architecture front-end determine the constants > to be restored on an exception, have you considered having the tcg > liveness pass automatically detect them? > > What I was thinking was if: > - each front-end stored each constant on every instruction using > regular "movi" ops > - tcg_liveness_analysis() tracked which global memory "sync" writes > are purely due to an op that can raise an exception > - then tcg_liveness_analysis() could remove "movi" instructions with > outputs that are needed only during an exception and place the > constant directly in the compressed table itself. > > I'm curious if this was considered and if there is a reason it > wouldn't work well. We certainly don't have enough information to infer something like that. The moment we reach a helper that isn't marked as TCG_CALL_NO_WG, all that inference has to go out the window ans we have to assume that the "movi op" is both necessary and overwritten. The knowledge of which helpers modify a field such as cc_op is baked into the translators at a different level. r~
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h index 6409db8..4e20dc1 100644 --- a/tcg/tcg-op.h +++ b/tcg/tcg-op.h @@ -700,17 +700,53 @@ static inline void tcg_gen_concat32_i64(TCGv_i64 ret, TCGv_i64 lo, TCGv_i64 hi) #error must include QEMU headers #endif -/* debug info: write the PC of the corresponding QEMU CPU instruction */ -static inline void tcg_gen_insn_start(uint64_t pc) +#if TARGET_INSN_START_WORDS == 1 +# if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS +static inline void tcg_gen_insn_start(target_ulong pc) { - /* XXX: must really use a 32 bit size for TCGArg in all cases */ -#if TARGET_LONG_BITS > TCG_TARGET_REG_BITS - tcg_gen_op2ii(INDEX_op_insn_start, - (uint32_t)(pc), (uint32_t)(pc >> 32)); + tcg_gen_op1(&tcg_ctx, INDEX_op_insn_start, pc); +} +# else +static inline void tcg_gen_insn_start(target_ulong pc) +{ + tcg_gen_op2(&tcg_ctx, INDEX_op_insn_start, + (uint32_t)pc, (uint32_t)(pc >> 32)); +} +# endif +#elif TARGET_INSN_START_WORDS == 2 +# if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS +static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1) +{ + tcg_gen_op2(&tcg_ctx, INDEX_op_insn_start, pc, a1); +} +# else +static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1) +{ + tcg_gen_op4(&tcg_ctx, INDEX_op_insn_start, + (uint32_t)pc, (uint32_t)(pc >> 32), + (uint32_t)a1, (uint32_t)(a1 >> 32)); +} +# endif +#elif TARGET_INSN_START_WORDS == 3 +# if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS +static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1, + target_ulong a2) +{ + tcg_gen_op3(&tcg_ctx, INDEX_op_insn_start, pc, a1, a2); +} +# else +static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1, + target_ulong a2) +{ + tcg_gen_op6(&tcg_ctx, INDEX_op_insn_start, + (uint32_t)pc, (uint32_t)(pc >> 32), + (uint32_t)a1, (uint32_t)(a1 >> 32), + (uint32_t)a2, (uint32_t)(a2 >> 32)); +} +# endif #else - tcg_gen_op1i(INDEX_op_insn_start, pc); +# error "Unhandled number of operands to insn_start" #endif -} static inline void tcg_gen_exit_tb(uintptr_t val) { diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h index f60d3c2..c6f9570 100644 --- a/tcg/tcg-opc.h +++ b/tcg/tcg-opc.h @@ -175,9 +175,9 @@ DEF(mulsh_i64, 1, 2, 0, IMPL(TCG_TARGET_HAS_mulsh_i64)) /* QEMU specific */ #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS -DEF(insn_start, 0, 0, 2, TCG_OPF_NOT_PRESENT) +DEF(insn_start, 0, 0, 2 * TARGET_INSN_START_WORDS, TCG_OPF_NOT_PRESENT) #else -DEF(insn_start, 0, 0, 1, TCG_OPF_NOT_PRESENT) +DEF(insn_start, 0, 0, TARGET_INSN_START_WORDS, TCG_OPF_NOT_PRESENT) #endif DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_END) DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_END) diff --git a/tcg/tcg.c b/tcg/tcg.c index df8788b..3308d68 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -991,16 +991,17 @@ void tcg_dump_ops(TCGContext *s) args = &s->gen_opparam_buf[op->args]; if (c == INDEX_op_insn_start) { - uint64_t pc; + qemu_log("%s ----", oi != s->gen_first_op_idx ? "\n" : ""); + + for (i = 0; i < TARGET_INSN_START_WORDS; ++i) { + target_ulong a; #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS - pc = ((uint64_t)args[1] << 32) | args[0]; + a = ((target_ulong)args[i * 2 + 1] << 32) | args[i * 2]; #else - pc = args[0]; + a = args[i]; #endif - if (oi != s->gen_first_op_idx) { - qemu_log("\n"); + qemu_log(" " TARGET_FMT_lx, a); } - qemu_log(" ---- 0x%" PRIx64, pc); } else if (c == INDEX_op_call) { /* variable number of arguments */ nb_oargs = op->callo; diff --git a/tcg/tcg.h b/tcg/tcg.h index 879a665..c975076 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -129,6 +129,12 @@ typedef uint64_t TCGRegSet; # error "Missing unsigned widening multiply" #endif +#ifndef TARGET_INSN_START_EXTRA_WORDS +# define TARGET_INSN_START_WORDS 1 +#else +# define TARGET_INSN_START_WORDS (1 + TARGET_INSN_START_EXTRA_WORDS) +#endif + typedef enum TCGOpcode { #define DEF(name, oargs, iargs, cargs, flags) INDEX_op_ ## name, #include "tcg-opc.h"