| Message ID | 20230321045340.838-1-zhiwei_liu@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tcg/tcg: Avoid TS_DEAD for basic block ending | expand |
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~
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:
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(-)