From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37403) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vbb7I-0005yP-V1 for qemu-devel@nongnu.org; Wed, 30 Oct 2013 15:07:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vbb7D-0001e1-6l for qemu-devel@nongnu.org; Wed, 30 Oct 2013 15:07:44 -0400 Received: from www11.your-server.de ([213.133.104.11]:44923) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vbb7D-0001dS-0L for qemu-devel@nongnu.org; Wed, 30 Oct 2013 15:07:39 -0400 Message-ID: <527158F2.9000000@macke.de> Date: Wed, 30 Oct 2013 12:07:30 -0700 From: Sebastian Macke MIME-Version: 1.0 References: <1383073495-5332-1-git-send-email-sebastian@macke.de> <1383073495-5332-12-git-send-email-sebastian@macke.de> <527150E2.5070200@twiddle.net> In-Reply-To: <527150E2.5070200@twiddle.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 11/13] target-openrisc: use jmp_pc as flag variable for branches List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: openrisc@openrisc.net, openrisc@lists.opencores.org, qemu-devel@nongnu.org, proljc@gmail.com On 30/10/2013 11:33 AM, Richard Henderson wrote: > On 10/29/2013 12:04 PM, Sebastian Macke wrote: >> { >> int lab = gen_new_label(); >> - dc->btaken = tcg_temp_local_new(); >> - tcg_gen_movi_tl(jmp_pc, dc->pc+8); >> - tcg_gen_movi_tl(dc->btaken, 0); >> + tcg_gen_movi_tl(jmp_pc, 0); >> tcg_gen_brcondi_i32(op0 == 0x03 ? TCG_COND_NE : TCG_COND_EQ, >> cpu_srf, 0, lab); >> - tcg_gen_movi_tl(dc->btaken, 1); >> tcg_gen_movi_tl(jmp_pc, tmp_pc); >> gen_set_label(lab); > You can now use movcond here: > > tcg_gen_movi_i32(jmp_pc, tmp_pc); > tcg_gen_movcond_i32(jmp_pc, cpu_srf, zero, jmp_pc, zero, > op0 == 0x03 ? TCG_COND_NE : TCG_COND_EQ); > > Although I'd wonder about just using setcond instead, since I think > the value stored in jmp_pc here is also stored in dc->j_target, leading > to the right behaviour... Correct. The movcond function can be used here. I will change that. But I didn't know that it exists because it is not written in the document http://wiki.qemu.org/Documentation/TCG/frontend-ops > >> case JUMP_BRANCH: >> { >> int l1 = gen_new_label(); >> - tcg_gen_brcondi_tl(TCG_COND_NE, dc->btaken, 0, l1); >> + tcg_gen_brcondi_tl(TCG_COND_NE, jmp_pc, 0, l1); >> gen_goto_tb(dc, 1, dc->pc); >> gen_set_label(l1); >> - tcg_temp_free(dc->btaken); >> gen_goto_tb(dc, 0, dc->j_target); >> break; > ... here. But j_target is not known when the delayed slot is translated separately. (E.g. if the delayed slot is at a page boundary.) > >> + case JUMP_BRANCH_DELAYED: >> + { >> + int l1 = gen_new_label(); >> + tcg_gen_brcondi_tl(TCG_COND_NE, jmp_pc, 0, l1); >> + gen_goto_tb(dc, 1, dc->pc); >> + gen_set_label(l1); >> + tcg_gen_mov_tl(cpu_pc, jmp_pc); >> + tcg_gen_exit_tb(0); >> + break; > ... >> + >> + dc->delayed_branch = !!(dc->tb_flags & D_FLAG); >> + if ((dc->delayed_branch) && (dc->tb_flags&B_FLAG)) { >> + dc->j_state = JUMP_BRANCH_DELAYED; >> + } >> + > > And thus I can't see how these additions are actually useful? > > If I've missed something, and the last hunk needs to be retained, > then please fix the coding style: > > if (dc->delayed_branch && (dc->tb_flags & B_FLAG)) { The problem is the delayed slot. The decision whether a jump is taken or not depends on the previous branch instruction l.bf and l.bnf instruction. If only the delayed slot is translated then we have to know what was the previous instruction. In this case we have to differ additionally if the delayed slot is a branch or a normal jump. Because for a branch the jmp_pc field is also a flag. We could simply jump to jmp_pc like it was done before but then we miss the block chaining feature. And I have not seen another way implementing it. > > r~