From patchwork Sun Sep 4 02:27:32 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 113262 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 491ABB6F75 for ; Sun, 4 Sep 2011 12:28:13 +1000 (EST) Received: from localhost ([::1]:50201 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R02RN-0005A7-VK for incoming@patchwork.ozlabs.org; Sat, 03 Sep 2011 22:28:09 -0400 Received: from eggs.gnu.org ([140.186.70.92]:35278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R02RH-00059o-Uy for qemu-devel@nongnu.org; Sat, 03 Sep 2011 22:28:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R02RG-0004WE-Cc for qemu-devel@nongnu.org; Sat, 03 Sep 2011 22:28:03 -0400 Received: from mail-pz0-f42.google.com ([209.85.210.42]:52270) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R02RG-0004W9-7r for qemu-devel@nongnu.org; Sat, 03 Sep 2011 22:28:02 -0400 Received: by pzk37 with SMTP id 37so7725510pzk.29 for ; Sat, 03 Sep 2011 19:28:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=06z31/j47OL+2r+UnmsrYmjMBCiAvWhJLI9eMR6cswQ=; b=EUz6jNfuebz/ChElB8UHNg5hgjO3pyQhh56ZrUJsriIhebVatOs+v7MlZGZSGt9zv5 M5eglQopnMhDHzKk54EQxmg7OTeZLZEYu1IF5okc0zk9G4OYrqjq+2eRXNTcENuAYK1Q 24wZlWfMJSnngEXcVStzIcHZ4mUSy+Onqv+5Y= Received: by 10.68.0.198 with SMTP id 6mr5039541pbg.423.1315103280810; Sat, 03 Sep 2011 19:28:00 -0700 (PDT) Received: from pebble.twiddle.home ([123.231.85.129]) by mx.google.com with ESMTPS id t7sm10496769pbn.11.2011.09.03.19.27.51 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 03 Sep 2011 19:27:58 -0700 (PDT) Message-ID: <4E62E214.4080400@twiddle.net> Date: Sun, 04 Sep 2011 07:57:32 +0530 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.20) Gecko/20110817 Fedora/3.1.12-1.fc14 Thunderbird/3.1.12 MIME-Version: 1.0 To: malc References: In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.210.42 Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] TCG sar UB (fwd) X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 09/03/2011 03:47 PM, malc wrote: > Doesn't make much sense to me, guest clearly asked for 0 and not -1, > besides -1 violates TCG's sar constraints and PPC obliges by emiting > illegal instruction in this case. The shift that the guest asked for was completely folded away. The -1 comes from gen_shift_rm_T1 in the computation of the new flags value. This could instead be moved inside the test for != 0, which is the only place that value is actually used anyway. Try this. Lightly tested. r~ diff --git a/target-i386/translate.c b/target-i386/translate.c index ccef381..b966762 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -1406,70 +1406,84 @@ static void gen_shift_rm_T1(DisasContext *s, int ot, int op1, { target_ulong mask; int shift_label; - TCGv t0, t1; + TCGv t0, t1, t2; - if (ot == OT_QUAD) + if (ot == OT_QUAD) { mask = 0x3f; - else + } else { mask = 0x1f; + } /* load */ - if (op1 == OR_TMP0) + if (op1 == OR_TMP0) { gen_op_ld_T0_A0(ot + s->mem_index); - else + } else { gen_op_mov_TN_reg(ot, 0, op1); + } - tcg_gen_andi_tl(cpu_T[1], cpu_T[1], mask); + t0 = tcg_temp_local_new(); + t1 = tcg_temp_local_new(); + t2 = tcg_temp_local_new(); - tcg_gen_addi_tl(cpu_tmp5, cpu_T[1], -1); + tcg_gen_andi_tl(t2, cpu_T[1], mask); if (is_right) { if (is_arith) { gen_exts(ot, cpu_T[0]); - tcg_gen_sar_tl(cpu_T3, cpu_T[0], cpu_tmp5); - tcg_gen_sar_tl(cpu_T[0], cpu_T[0], cpu_T[1]); + tcg_gen_mov_tl(t0, cpu_T[0]); + tcg_gen_sar_tl(cpu_T[0], cpu_T[0], t2); } else { gen_extu(ot, cpu_T[0]); - tcg_gen_shr_tl(cpu_T3, cpu_T[0], cpu_tmp5); - tcg_gen_shr_tl(cpu_T[0], cpu_T[0], cpu_T[1]); + tcg_gen_mov_tl(t0, cpu_T[0]); + tcg_gen_shr_tl(cpu_T[0], cpu_T[0], t2); } } else { - tcg_gen_shl_tl(cpu_T3, cpu_T[0], cpu_tmp5); - tcg_gen_shl_tl(cpu_T[0], cpu_T[0], cpu_T[1]); + tcg_gen_mov_tl(t0, cpu_T[0]); + tcg_gen_shl_tl(cpu_T[0], cpu_T[0], t2); } /* store */ - if (op1 == OR_TMP0) + if (op1 == OR_TMP0) { gen_op_st_T0_A0(ot + s->mem_index); - else + } else { gen_op_mov_reg_T0(ot, op1); - + } + /* update eflags if non zero shift */ - if (s->cc_op != CC_OP_DYNAMIC) + if (s->cc_op != CC_OP_DYNAMIC) { gen_op_set_cc_op(s->cc_op); + } - /* XXX: inefficient */ - t0 = tcg_temp_local_new(); - t1 = tcg_temp_local_new(); - - tcg_gen_mov_tl(t0, cpu_T[0]); - tcg_gen_mov_tl(t1, cpu_T3); + tcg_gen_mov_tl(t1, cpu_T[0]); shift_label = gen_new_label(); - tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, shift_label); + tcg_gen_brcondi_tl(TCG_COND_EQ, t2, 0, shift_label); - tcg_gen_mov_tl(cpu_cc_src, t1); - tcg_gen_mov_tl(cpu_cc_dst, t0); - if (is_right) + tcg_gen_addi_tl(t2, t2, -1); + tcg_gen_mov_tl(cpu_cc_dst, t1); + + if (is_right) { + if (is_arith) { + tcg_gen_sar_tl(cpu_cc_src, t0, t2); + } else { + tcg_gen_shr_tl(cpu_cc_src, t0, t2); + } + } else { + tcg_gen_shl_tl(cpu_cc_src, t0, t2); + } + + if (is_right) { tcg_gen_movi_i32(cpu_cc_op, CC_OP_SARB + ot); - else + } else { tcg_gen_movi_i32(cpu_cc_op, CC_OP_SHLB + ot); - + } + gen_set_label(shift_label); s->cc_op = CC_OP_DYNAMIC; /* cannot predict flags after */ tcg_temp_free(t0); tcg_temp_free(t1); + tcg_temp_free(t2); } static void gen_shift_rm_im(DisasContext *s, int ot, int op1, int op2,