tcg/tcg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
TS_DEAD means we will release the register allocated for this temporary. But
at basic block ending, we can still use the allocted register.
Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
tcg/tcg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index bb52bc060b..0c93e6e6f8 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2822,7 +2822,7 @@ static void la_bb_end(TCGContext *s, int ng, int nt)
case TEMP_FIXED:
case TEMP_GLOBAL:
case TEMP_TB:
- state = TS_DEAD | TS_MEM;
+ state = TS_MEM;
break;
case TEMP_EBB:
case TEMP_CONST:
--
2.17.1
On 3/20/23 21:53, LIU Zhiwei wrote: > TS_DEAD means we will release the register allocated for this temporary. But > at basic block ending, we can still use the allocted register. > > Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Test case? r~ > --- > tcg/tcg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index bb52bc060b..0c93e6e6f8 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -2822,7 +2822,7 @@ static void la_bb_end(TCGContext *s, int ng, int nt) > case TEMP_FIXED: > case TEMP_GLOBAL: > case TEMP_TB: > - state = TS_DEAD | TS_MEM; > + state = TS_MEM; > break; > case TEMP_EBB: > case TEMP_CONST:
On 2023/3/21 14:06, Richard Henderson wrote: > On 3/20/23 21:53, LIU Zhiwei wrote: >> TS_DEAD means we will release the register allocated for this >> temporary. But >> at basic block ending, we can still use the allocted register. >> >> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > > Test case? I have run an Ubuntu image after this patch. It can boot. But I can't find a direct test case. Because the IRs supported with flags TCG_OPF_BB_END do not have input or output parameter, such as the set_label or br. Best Regards, Zhiwei > > > r~ > >> --- >> tcg/tcg.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index bb52bc060b..0c93e6e6f8 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -2822,7 +2822,7 @@ static void la_bb_end(TCGContext *s, int ng, >> int nt) >> case TEMP_FIXED: >> case TEMP_GLOBAL: >> case TEMP_TB: >> - state = TS_DEAD | TS_MEM; >> + state = TS_MEM; >> break; >> case TEMP_EBB: >> case TEMP_CONST:
On 3/20/23 23:44, LIU Zhiwei wrote: > > On 2023/3/21 14:06, Richard Henderson wrote: >> On 3/20/23 21:53, LIU Zhiwei wrote: >>> TS_DEAD means we will release the register allocated for this temporary. But >>> at basic block ending, we can still use the allocted register. >>> >>> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> >> >> Test case? > > I have run an Ubuntu image after this patch. It can boot. That's surprising. I would have expected an assert with --enable-debug-tcg, but this appears to be an oversight in tcg_reg_alloc_bb_end. We only validate the liveness data for TEMP_EBB and TEMP_CONST, but not TEMP_TB or TEMP_GLOBAL. > But I can't find a direct test case. Because the IRs supported with flags TCG_OPF_BB_END > do not have input or output parameter, such as the set_label or br. That's exactly why we want all GLOBAL and TB to be DEAD | MEM, so that they're saved back to their home slots and released from their registers. The register allocator for TCG does not work across extended basic blocks. Importantly, if you have a forward branch like so: g1 = func(a) brcond ..., L1 stuff g2 = func(b) g1 = g2 discard g2 L1: What value should g1->reg have at L1? The allocator does not do the global control flow and allocation required to ensure that g1->reg is the same at the brcond and at the label. Nominally, I would have expected one value for g1->reg at the branch, a different value for g2->reg in the second BB, and for the assignment to steal g2->reg and move it to g1->reg (per tcg_reg_alloc_mov of an IS_DEAD_ARG temp). Which would result in an incorrect allocation at L1. What are you attempting to do? Is this just guesswork? r~
© 2016 - 2024 Red Hat, Inc.