[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/13] tcg: rewrite tcg_reg_alloc_mov()
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH 07/13] tcg: rewrite tcg_reg_alloc_mov() |
Date: |
Thu, 27 Sep 2012 12:09:35 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
On 09/27/2012 10:15 AM, Aurelien Jarno wrote:
> + /* We have to load the value in a register for moving it to another
> + or for saving it. We assume it's better to keep it there than to
> + reload it later. */
> + if ((NEED_SYNC_ARG(0) && ts->val_type != TEMP_VAL_REG)
> + || ts->val_type == TEMP_VAL_MEM) {
> + ts->reg = tcg_reg_alloc(s, arg_ct->u.regs, allocated_regs);
> + if (ts->val_type == TEMP_VAL_MEM) {
> + tcg_out_ld(s, ts->type, ts->reg, ts->mem_reg, ts->mem_offset);
> + } else if (ts->val_type == TEMP_VAL_CONST) {
> + tcg_out_movi(s, ts->type, ts->reg, ts->val);
> + }
> + s->reg_to_temp[ts->reg] = args[1];
> + ts->val_type = TEMP_VAL_REG;
> + ts->mem_coherent = 1;
> + }
I don't understand this block. In particular, the ts->mem_coherent = 1
in the TEMP_VAL_CONST case looks wrong.
Why are you handling NEED_SYNC_ARG before the move, rather than after?
> + if (IS_DEAD_ARG(0) && !ots->fixed_reg) {
> + /* mov to a non-saved dead register makes no sense (even with
> + liveness analysis disabled). */
> + assert(NEED_SYNC_ARG(0));
> + /* The code above should have moved the temp to a register. */
> + assert(ts->val_type == TEMP_VAL_REG);
> + if (!ots->mem_allocated) {
> + temp_allocate_frame(s, args[0]);
> }
> + if (ts->val_type == TEMP_VAL_REG) {
> + tcg_out_st(s, ots->type, ts->reg, ots->mem_reg, ots->mem_offset);
> + if (IS_DEAD_ARG(1)) {
> + temp_dead(s, args[1]);
> + }
> }
> + temp_dead(s, args[0]);
Isn't this store going to happen via temp_dead -> temp_sync -> tcg_reg_sync?
> } else if (ts->val_type == TEMP_VAL_CONST) {
> if (ots->fixed_reg) {
> + tcg_out_movi(s, ots->type, ots->reg, ts->val);
> + s->reg_to_temp[ots->reg] = args[0];
> } else {
> /* propagate constant */
> + if (ots->val_type == TEMP_VAL_REG) {
> s->reg_to_temp[ots->reg] = -1;
> + }
> ots->val_type = TEMP_VAL_CONST;
> ots->val = ts->val;
> }
How much of the first block above is redundant with this?
Especially given that I think there's a missing sync.
> } else {
> + /* The code above should have moved the temp to a register. */
> + assert(ts->val_type == TEMP_VAL_REG);
> + if (IS_DEAD_ARG(1) && !ts->fixed_reg && !ots->fixed_reg) {
> + /* the mov can be suppressed */
> + if (ots->val_type == TEMP_VAL_REG) {
> + s->reg_to_temp[ots->reg] = -1;
> + }
> + ots->reg = ts->reg;
> + temp_dead(s, args[1]);
> + } else {
> + if (ots->val_type != TEMP_VAL_REG) {
> + /* When allocating a new register, make sure to not spill the
> + input one. */
> + tcg_regset_set_reg(allocated_regs, ts->reg);
> + ots->reg = tcg_reg_alloc(s, oarg_ct->u.regs, allocated_regs);
> + }
> + tcg_out_mov(s, ots->type, ots->reg, ts->reg);
> + }
> + ots->val_type = TEMP_VAL_REG;
> + ots->mem_coherent = 0;
> + s->reg_to_temp[ots->reg] = args[0];
> + if (NEED_SYNC_ARG(0)) {
> + tcg_reg_sync(s, ots->reg);
> + }
... as we do here.
r~
- [Qemu-devel] [PATCH 11/13] tcg: sync globals for pure helpers instead of saving them, (continued)
- [Qemu-devel] [PATCH 09/13] tcg: start with local temps in TEMP_VAL_MEM state, Aurelien Jarno, 2012/09/27
- [Qemu-devel] [PATCH 10/13] tcg: don't explicitely save globals and temps, Aurelien Jarno, 2012/09/27
- [Qemu-devel] [PATCH 07/13] tcg: rewrite tcg_reg_alloc_mov(), Aurelien Jarno, 2012/09/27
- [Qemu-devel] [PATCH 05/13] tcg: rework liveness analysis, Aurelien Jarno, 2012/09/27
- [Qemu-devel] [PATCH 04/13] tcg: sync output arguments on liveness request, Aurelien Jarno, 2012/09/27
[Qemu-devel] [PATCH 06/13] tcg: improve tcg_reg_alloc_movi(), Aurelien Jarno, 2012/09/27