qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>, qemu-devel@nongnu.org
Subject: Re: [PATCH] tcg/tcg: Avoid TS_DEAD for basic block ending
Date: Tue, 21 Mar 2023 08:39:23 -0700	[thread overview]
Message-ID: <4b133e2e-7e3c-1cbd-3f72-7fbda2c4399c@linaro.org> (raw)
In-Reply-To: <ee34a299-3b57-1f97-cc08-19cebfcc0c6e@linux.alibaba.com>

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~


      reply	other threads:[~2023-03-21 15:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-21  4:53 [PATCH] tcg/tcg: Avoid TS_DEAD for basic block ending LIU Zhiwei
2023-03-21  6:06 ` Richard Henderson
2023-03-21  6:44   ` LIU Zhiwei
2023-03-21 15:39     ` Richard Henderson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b133e2e-7e3c-1cbd-3f72-7fbda2c4399c@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).