diff mbox series

[v4,01/27] tcg: Fix tcg_reg_alloc_dup*

Message ID 20221213212541.1820840-2-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg misc patches | expand

Commit Message

Richard Henderson Dec. 13, 2022, 9:25 p.m. UTC
The assignment to mem_coherent should be done with any
modification, not simply with a newly allocated register.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tcg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alex Bennée Dec. 19, 2022, 3:49 p.m. UTC | #1
Richard Henderson <richard.henderson@linaro.org> writes:

> The assignment to mem_coherent should be done with any
> modification, not simply with a newly allocated register.

What exactly does mem_coherent mean in this case? Is it that our
register store is potentially out of sync with live values in temp regs
or that we have memory loads and stores in flight?

I think it would be useful to add a doc patch for TCGTemp do specify
what the various fields mean. It would certainly help reviewers that
don't have it committed to memory ;-)
Richard Henderson Dec. 19, 2022, 6:17 p.m. UTC | #2
On 12/19/22 07:49, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> The assignment to mem_coherent should be done with any
>> modification, not simply with a newly allocated register.
> 
> What exactly does mem_coherent mean in this case? Is it that our
> register store is potentially out of sync with live values in temp regs
> or that we have memory loads and stores in flight?
> 
> I think it would be useful to add a doc patch for TCGTemp do specify
> what the various fields mean. It would certainly help reviewers that
> don't have it committed to memory ;-)
> 

mem_coherent means that the register value and the memory value are the same, so that if 
we must release the register we do not need to save the value back to memory.


r~
diff mbox series

Patch

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 05d2b70ab7..371908b34b 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -3498,7 +3498,6 @@  static void tcg_reg_alloc_dup(TCGContext *s, const TCGOp *op)
         ots->reg = tcg_reg_alloc(s, dup_out_regs, allocated_regs,
                                  op->output_pref[0], ots->indirect_base);
         ots->val_type = TEMP_VAL_REG;
-        ots->mem_coherent = 0;
         s->reg_to_temp[ots->reg] = ots;
     }
 
@@ -3552,6 +3551,7 @@  static void tcg_reg_alloc_dup(TCGContext *s, const TCGOp *op)
     tcg_debug_assert(ok);
 
  done:
+    ots->mem_coherent = 0;
     if (IS_DEAD_ARG(1)) {
         temp_dead(s, its);
     }
@@ -3779,7 +3779,6 @@  static bool tcg_reg_alloc_dup2(TCGContext *s, const TCGOp *op)
         ots->reg = tcg_reg_alloc(s, dup_out_regs, allocated_regs,
                                  op->output_pref[0], ots->indirect_base);
         ots->val_type = TEMP_VAL_REG;
-        ots->mem_coherent = 0;
         s->reg_to_temp[ots->reg] = ots;
     }
 
@@ -3823,6 +3822,7 @@  static bool tcg_reg_alloc_dup2(TCGContext *s, const TCGOp *op)
     return false;
 
  done:
+    ots->mem_coherent = 0;
     if (IS_DEAD_ARG(1)) {
         temp_dead(s, itsl);
     }