From patchwork Tue Jun 11 10:53:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 166423 Delivered-To: patch@linaro.org Received: by 2002:a92:4782:0:0:0:0:0 with SMTP id e2csp2252117ilk; Tue, 11 Jun 2019 04:49:37 -0700 (PDT) X-Google-Smtp-Source: APXvYqwzR4di9DMv8Zr31rNcWy8SfPPWrQBEWsh7ArVwGf+6zEvIs/ZAWC2W8MHNPHaJwnWfdN6A X-Received: by 2002:a37:ac14:: with SMTP id e20mr58539684qkm.243.1560253777695; Tue, 11 Jun 2019 04:49:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1560253777; cv=none; d=google.com; s=arc-20160816; b=E17qu8qA1ZpGAG0sJTV3P+6FzxGjQwpHYk4lzS+2YqHp+MqReXuNZQYng7/NHDeUkO To2HCjwPMAEOuUWTA2QssI7G8XimcFrSAWYh2aHAQJsZhHtYlyJ91ADc/cN8otf9plcB 7j4E3RXH57nOBplAwrMasOOPZoXXbfwza0KrjhTYuYnN8NIG79POVxdOAk1U/bEUfuJf faBLF5d5lRnk9TSqQ+PKSbysjJEO0dyVsjYXMautVc9rEiwevbdqY4SJxcJ1L63/p0f8 7YUrc3g2yvUDrHea94WwuS1wXmnmDZN2vxCljyMApu3hU4l0gnEvdgQfN/vmS1mr0Fkb OXew== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject :content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:to:from:dkim-signature; bh=gPYLikUR5ZuBlGRaNvlfd3hHYV0xbs6/Y2luBsY5Vwk=; b=OwGXEcaDogqo3B6TZGB0ZRVYlb4eu8RJ801snOjRspCwT9BujLeyQp1+r12GLDUvEQ 2hSUbx4RkbXBRaJUGH7om2KuFdp6G0rwVpcweWEwzvVYZkYIyUcOB5ISG85aqYgkBzdG 9/EvMLvoTa9TsLQAescDfEpDzGGjhZnEaBBnIEy0N60Kj7TtCx/Bz1Z1A1cbTUkQRw/d YZ3m/xK96Ciiu/Dbz8BHBWKnVkWNNDQx4qhItJAO7myf5nLQATYcml+pG7bt/FTdbOmr FtGpH2yOrhQtS7/R5mVjoHrFkzl5kjG0JttJIN4aVfDQASY/ScgToEgG7VbzwCoZQyIu 0uSg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=cY2uIjwJ; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id o38si5279201qte.117.2019.06.11.04.49.37 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 11 Jun 2019 04:49:37 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=cY2uIjwJ; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:56656 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hafHV-0005C2-3Q for patch@linaro.org; Tue, 11 Jun 2019 07:49:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39644) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1haeQQ-0007de-Al for qemu-devel@nongnu.org; Tue, 11 Jun 2019 06:54:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1haeQK-0007qU-L3 for qemu-devel@nongnu.org; Tue, 11 Jun 2019 06:54:46 -0400 Received: from mail-wr1-x441.google.com ([2a00:1450:4864:20::441]:43071) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1haeQK-0007ot-2l for qemu-devel@nongnu.org; Tue, 11 Jun 2019 06:54:40 -0400 Received: by mail-wr1-x441.google.com with SMTP id p13so2389334wru.10 for ; Tue, 11 Jun 2019 03:54:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=gPYLikUR5ZuBlGRaNvlfd3hHYV0xbs6/Y2luBsY5Vwk=; b=cY2uIjwJlfKnxKC5LvBX2R1HHBm6XmPD5C5JRZiBG7PpDMpQ+ID1a3JR2qC9OezKYb xdL65tJIR84ZILz7hKKYNUVTM7g5g7ex+diwwX5wEYSnNze5WP1eGowKOX2u5NC7zf/O PFRKzrX8Rg/BqaAclpyrAetH8wsNT0wWo7FiYV/T40SKeDkOjA1JSRYhu6cPVdVQm5P5 IagG0rJX7+UG1FGCMDnpkixlHJ2AWAm9YoFFR2z6oDyT0BUF30ycTsu60bdeMGOmxS5W UKdlNIYM+JzvOPyOyRUoI3AGcENMyhUuID25v7U0eUzzao6YsObZaWN3/RUHEO0A09q9 4asA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=gPYLikUR5ZuBlGRaNvlfd3hHYV0xbs6/Y2luBsY5Vwk=; b=JYbHYxwA8G876965l9Pi2S4O969DZ10gYlGZvY3dwtlHHMw+dx+zBKHSL4GP0QO1vI i3hb3gbCACnApMx2b249ycQXaTAaqz8fbN4z2yp8M/bbclueZzdpMt3ky88s689TK2/N 5huKV1AvsEFPKg+E4lqkbKyaiB+Hz5lenwS+UeF6OmdsU6UJDtt0jWbaMH4PcBA6NJ+N 96mtrBA+oEvaYcqpzVkxMt3UPTLyE86h2FSe6lYSR7D1rKq7rQyK8yx40foyi0tQ6vIl d2UcZbHyW+NPit60iKlKsYjgKbhmUEXxENxT+sG3IqLVplAEl5PqcZrESLie89BQUZUL gTaQ== X-Gm-Message-State: APjAAAXaKvyJqlrEfE8UOKCgOgbAwiUrDyHV0kNUobCyFKkFU5gsI1XR +XI92jQmOxwHrwehVLHz95BnRQ== X-Received: by 2002:adf:9dcc:: with SMTP id q12mr24220861wre.93.1560250478626; Tue, 11 Jun 2019 03:54:38 -0700 (PDT) Received: from orth.archaic.org.uk (orth.archaic.org.uk. [81.2.115.148]) by smtp.gmail.com with ESMTPSA id e17sm10785945wrt.95.2019.06.11.03.54.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 11 Jun 2019 03:54:38 -0700 (PDT) From: Peter Maydell To: qemu-arm@nongnu.org, qemu-devel@nongnu.org Date: Tue, 11 Jun 2019 11:53:51 +0100 Message-Id: <20190611105351.9871-43-peter.maydell@linaro.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190611105351.9871-1-peter.maydell@linaro.org> References: <20190611105351.9871-1-peter.maydell@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::441 Subject: [Qemu-devel] [PATCH v2 42/42] target/arm: Fix short-vector increment behaviour X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Richard Henderson Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" For VFP short vectors, the VFP registers are divided into a series of banks: for single-precision these are s0-s7, s8-s15, s16-s23 and s24-s31; for double-precision they are d0-d3, d4-d7, ... d28-d31. Some banks are "scalar" meaning that use of a register within them triggers a pure-scalar or mixed vector-scalar operation rather than a full vector operation. The scalar banks are s0-s7, d0-d3 and d16-d19. When using a bank as part of a vector operation, we iterate through it, increasing the register number by the specified stride each time, and wrapping around to the beginning of the bank. Unfortunately our calculation of the "increment" part of this was incorrect: vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask) will only do the intended thing if bank_mask has exactly one set high bit. For instance for doubles (bank_mask = 0xc), if we start with vd = 6 and delta_d = 2 then vd is updated to 12 rather than the intended 4. This only causes problems in the unlikely case that the starting register is not the first in its bank: if the register number doesn't have to wrap around then the expression happens to give the right answer. Fix this bug by abstracting out the "check whether register is in a scalar bank" and "advance register within bank" operations to utility functions which use the right bit masking operations. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- target/arm/translate-vfp.inc.c | 100 ++++++++++++++++++++------------- 1 file changed, 60 insertions(+), 40 deletions(-) -- 2.20.1 diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index 8216dba796e..709fc65374d 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -1139,6 +1139,42 @@ typedef void VFPGen3OpDPFn(TCGv_i64 vd, typedef void VFPGen2OpSPFn(TCGv_i32 vd, TCGv_i32 vm); typedef void VFPGen2OpDPFn(TCGv_i64 vd, TCGv_i64 vm); +/* + * Return true if the specified S reg is in a scalar bank + * (ie if it is s0..s7) + */ +static inline bool vfp_sreg_is_scalar(int reg) +{ + return (reg & 0x18) == 0; +} + +/* + * Return true if the specified D reg is in a scalar bank + * (ie if it is d0..d3 or d16..d19) + */ +static inline bool vfp_dreg_is_scalar(int reg) +{ + return (reg & 0xc) == 0; +} + +/* + * Advance the S reg number forwards by delta within its bank + * (ie increment the low 3 bits but leave the rest the same) + */ +static inline int vfp_advance_sreg(int reg, int delta) +{ + return ((reg + delta) & 0x7) | (reg & ~0x7); +} + +/* + * Advance the D reg number forwards by delta within its bank + * (ie increment the low 2 bits but leave the rest the same) + */ +static inline int vfp_advance_dreg(int reg, int delta) +{ + return ((reg + delta) & 0x3) | (reg & ~0x3); +} + /* * Perform a 3-operand VFP data processing instruction. fn is the * callback to do the actual operation; this function deals with the @@ -1149,7 +1185,6 @@ static bool do_vfp_3op_sp(DisasContext *s, VFPGen3OpSPFn *fn, { uint32_t delta_m = 0; uint32_t delta_d = 0; - uint32_t bank_mask = 0; int veclen = s->vec_len; TCGv_i32 f0, f1, fd; TCGv_ptr fpst; @@ -1164,16 +1199,14 @@ static bool do_vfp_3op_sp(DisasContext *s, VFPGen3OpSPFn *fn, } if (veclen > 0) { - bank_mask = 0x18; - /* Figure out what type of vector operation this is. */ - if ((vd & bank_mask) == 0) { + if (vfp_sreg_is_scalar(vd)) { /* scalar */ veclen = 0; } else { delta_d = s->vec_stride + 1; - if ((vm & bank_mask) == 0) { + if (vfp_sreg_is_scalar(vm)) { /* mixed scalar/vector */ delta_m = 0; } else { @@ -1204,11 +1237,11 @@ static bool do_vfp_3op_sp(DisasContext *s, VFPGen3OpSPFn *fn, /* Set up the operands for the next iteration */ veclen--; - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); - vn = ((vn + delta_d) & (bank_mask - 1)) | (vn & bank_mask); + vd = vfp_advance_sreg(vd, delta_d); + vn = vfp_advance_sreg(vn, delta_d); neon_load_reg32(f0, vn); if (delta_m) { - vm = ((vm + delta_m) & (bank_mask - 1)) | (vm & bank_mask); + vm = vfp_advance_sreg(vm, delta_m); neon_load_reg32(f1, vm); } } @@ -1226,7 +1259,6 @@ static bool do_vfp_3op_dp(DisasContext *s, VFPGen3OpDPFn *fn, { uint32_t delta_m = 0; uint32_t delta_d = 0; - uint32_t bank_mask = 0; int veclen = s->vec_len; TCGv_i64 f0, f1, fd; TCGv_ptr fpst; @@ -1246,16 +1278,14 @@ static bool do_vfp_3op_dp(DisasContext *s, VFPGen3OpDPFn *fn, } if (veclen > 0) { - bank_mask = 0xc; - /* Figure out what type of vector operation this is. */ - if ((vd & bank_mask) == 0) { + if (vfp_dreg_is_scalar(vd)) { /* scalar */ veclen = 0; } else { delta_d = (s->vec_stride >> 1) + 1; - if ((vm & bank_mask) == 0) { + if (vfp_dreg_is_scalar(vm)) { /* mixed scalar/vector */ delta_m = 0; } else { @@ -1285,11 +1315,11 @@ static bool do_vfp_3op_dp(DisasContext *s, VFPGen3OpDPFn *fn, } /* Set up the operands for the next iteration */ veclen--; - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); - vn = ((vn + delta_d) & (bank_mask - 1)) | (vn & bank_mask); + vd = vfp_advance_dreg(vd, delta_d); + vn = vfp_advance_dreg(vn, delta_d); neon_load_reg64(f0, vn); if (delta_m) { - vm = ((vm + delta_m) & (bank_mask - 1)) | (vm & bank_mask); + vm = vfp_advance_dreg(vm, delta_m); neon_load_reg64(f1, vm); } } @@ -1306,7 +1336,6 @@ static bool do_vfp_2op_sp(DisasContext *s, VFPGen2OpSPFn *fn, int vd, int vm) { uint32_t delta_m = 0; uint32_t delta_d = 0; - uint32_t bank_mask = 0; int veclen = s->vec_len; TCGv_i32 f0, fd; @@ -1320,16 +1349,14 @@ static bool do_vfp_2op_sp(DisasContext *s, VFPGen2OpSPFn *fn, int vd, int vm) } if (veclen > 0) { - bank_mask = 0x18; - /* Figure out what type of vector operation this is. */ - if ((vd & bank_mask) == 0) { + if (vfp_sreg_is_scalar(vd)) { /* scalar */ veclen = 0; } else { delta_d = s->vec_stride + 1; - if ((vm & bank_mask) == 0) { + if (vfp_sreg_is_scalar(vm)) { /* mixed scalar/vector */ delta_m = 0; } else { @@ -1355,7 +1382,7 @@ static bool do_vfp_2op_sp(DisasContext *s, VFPGen2OpSPFn *fn, int vd, int vm) if (delta_m == 0) { /* single source one-many */ while (veclen--) { - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); + vd = vfp_advance_sreg(vd, delta_d); neon_store_reg32(fd, vd); } break; @@ -1363,8 +1390,8 @@ static bool do_vfp_2op_sp(DisasContext *s, VFPGen2OpSPFn *fn, int vd, int vm) /* Set up the operands for the next iteration */ veclen--; - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); - vm = ((vm + delta_m) & (bank_mask - 1)) | (vm & bank_mask); + vd = vfp_advance_sreg(vd, delta_d); + vm = vfp_advance_sreg(vm, delta_m); neon_load_reg32(f0, vm); } @@ -1378,7 +1405,6 @@ static bool do_vfp_2op_dp(DisasContext *s, VFPGen2OpDPFn *fn, int vd, int vm) { uint32_t delta_m = 0; uint32_t delta_d = 0; - uint32_t bank_mask = 0; int veclen = s->vec_len; TCGv_i64 f0, fd; @@ -1397,16 +1423,14 @@ static bool do_vfp_2op_dp(DisasContext *s, VFPGen2OpDPFn *fn, int vd, int vm) } if (veclen > 0) { - bank_mask = 0xc; - /* Figure out what type of vector operation this is. */ - if ((vd & bank_mask) == 0) { + if (vfp_dreg_is_scalar(vd)) { /* scalar */ veclen = 0; } else { delta_d = (s->vec_stride >> 1) + 1; - if ((vm & bank_mask) == 0) { + if (vfp_dreg_is_scalar(vm)) { /* mixed scalar/vector */ delta_m = 0; } else { @@ -1432,7 +1456,7 @@ static bool do_vfp_2op_dp(DisasContext *s, VFPGen2OpDPFn *fn, int vd, int vm) if (delta_m == 0) { /* single source one-many */ while (veclen--) { - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); + vd = vfp_advance_dreg(vd, delta_d); neon_store_reg64(fd, vd); } break; @@ -1440,8 +1464,8 @@ static bool do_vfp_2op_dp(DisasContext *s, VFPGen2OpDPFn *fn, int vd, int vm) /* Set up the operands for the next iteration */ veclen--; - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); - vm = ((vm + delta_m) & (bank_mask - 1)) | (vm & bank_mask); + vd = vfp_advance_dreg(vd, delta_d); + vd = vfp_advance_dreg(vm, delta_m); neon_load_reg64(f0, vm); } @@ -1783,7 +1807,6 @@ static bool trans_VFM_dp(DisasContext *s, arg_VFM_sp *a) static bool trans_VMOV_imm_sp(DisasContext *s, arg_VMOV_imm_sp *a) { uint32_t delta_d = 0; - uint32_t bank_mask = 0; int veclen = s->vec_len; TCGv_i32 fd; uint32_t n, i, vd; @@ -1804,9 +1827,8 @@ static bool trans_VMOV_imm_sp(DisasContext *s, arg_VMOV_imm_sp *a) } if (veclen > 0) { - bank_mask = 0x18; /* Figure out what type of vector operation this is. */ - if ((vd & bank_mask) == 0) { + if (vfp_sreg_is_scalar(vd)) { /* scalar */ veclen = 0; } else { @@ -1835,7 +1857,7 @@ static bool trans_VMOV_imm_sp(DisasContext *s, arg_VMOV_imm_sp *a) /* Set up the operands for the next iteration */ veclen--; - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); + vd = vfp_advance_sreg(vd, delta_d); } tcg_temp_free_i32(fd); @@ -1845,7 +1867,6 @@ static bool trans_VMOV_imm_sp(DisasContext *s, arg_VMOV_imm_sp *a) static bool trans_VMOV_imm_dp(DisasContext *s, arg_VMOV_imm_dp *a) { uint32_t delta_d = 0; - uint32_t bank_mask = 0; int veclen = s->vec_len; TCGv_i64 fd; uint32_t n, i, vd; @@ -1871,9 +1892,8 @@ static bool trans_VMOV_imm_dp(DisasContext *s, arg_VMOV_imm_dp *a) } if (veclen > 0) { - bank_mask = 0xc; /* Figure out what type of vector operation this is. */ - if ((vd & bank_mask) == 0) { + if (vfp_dreg_is_scalar(vd)) { /* scalar */ veclen = 0; } else { @@ -1902,7 +1922,7 @@ static bool trans_VMOV_imm_dp(DisasContext *s, arg_VMOV_imm_dp *a) /* Set up the operands for the next iteration */ veclen--; - vd = ((vd + delta_d) & (bank_mask - 1)) | (vd & bank_mask); + vfp_advance_dreg(vd, delta_d); } tcg_temp_free_i64(fd);