From patchwork Fri Sep 5 09:07:27 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 386213 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 82C0E140087 for ; Fri, 5 Sep 2014 19:07:50 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=mSzMjDasPfYKZJg9GLk5yZre5G224uY/APhDJyHD2g+ jphga2CSCZCXXejoxEgHEDdaeyocNt72lSS4iEBhxC5myQInNppI3z8ELbkkaMIb bSMI2ZuYijWOvNnFfT4wN5G2j5NYmHSh+g++XfsXrfDP7PIKfI+DWtGxIN6TowHw = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=I+bYoPT4xQ1g+tXuZR6mshUNFCM=; b=A88BvTjgbFuE5cSE2 t5JVvUSdzl0kYHOZjzbkjG2imcMT3xC9CH3Xq5CnCxADHJ4o1Vb5y5Rpymtol4Eu ifjkpHRXKnDkgHzJ5gBRXfm/Pf+GpDPB+TjCpNVSLKl/JFwMX7QzVFlODp9QyQ/k NTaHR8Sa7nyKUC8zX/WkbsaBPI= Received: (qmail 3568 invoked by alias); 5 Sep 2014 09:07:35 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 3554 invoked by uid 89); 5 Sep 2014 09:07:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 05 Sep 2014 09:07:32 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Fri, 05 Sep 2014 10:07:28 +0100 Received: from [10.1.208.24] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 5 Sep 2014 10:07:27 +0100 Message-ID: <54097D4F.9010609@arm.com> Date: Fri, 05 Sep 2014 10:07:27 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: GCC Patches CC: Marcus Shawcroft , Richard Earnshaw Subject: [PATCH][AArch64] PR 61749: Do not ICE in lane intrinsics when passed non-constant lane number X-MC-Unique: 114090510072811501 X-IsSubscribed: yes Hi all, As the PR says we currently ICE in aarch64_simd_lane_bounds when processing #include "arm_neon.h" int32x4_t foo (int32x4_t a, int16x4_t b, int16x4_t c, int d) { return vqdmlal_lane_s16 (a, b, c, d); } This code is invalid since the lane argument (d) should be a compile-time constant. This can be fixed by setting the qualifier for the 4th argument for these intrinsics to qualifier_immediate so that the expansion code in aarch64-builtins.c can detect that and emit the appropriate message. This, however, is not enough by itself. We will emit the error but then proceed anyway and ICE. From looking around other backends (and rs6000 in particular), the correct thing to do in these cases is to return const0_rtx to signify that a user input error occured. This patch does that and also makes sure we hit gcc_unreachable () instead of returning NULL_RTX when the requested builtin to expand cannot be found. This is the correct thing to do because returning NULL_RTX is apparently just the way to show that the builtin does not return a result (e.g. for void builtins). Before this patch on the above code we would get: $BUILD/install/lib/gcc/aarch64-none-elf/4.10.0/include/arm_neon.h: In function 'foo': $BUILD/install/lib/gcc/aarch64-none-elf/4.10.0/include/arm_neon.h:19294:10: internal compiler error: in aarch64_simd_lane_bounds, at config/aarch64/aarch64.c:7715 return __builtin_aarch64_sqdmlal_lanev4hi (__a, __b, __c, __d); ^ 0xc608d0 aarch64_simd_lane_bounds(rtx_def*, long, long) $SRC/gcc/config/aarch64/aarch64.c:7715 0xcb0221 gen_aarch64_sqdmlal_lanev4hi(rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*) $SRC/gcc/config/aarch64/aarch64-simd.md:3015 0xc65b7f insn_gen_fn::operator()(rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*) const $SRC/src/gcc/gcc/recog.h:311 0xc65b7f aarch64_simd_expand_args $SRC/gcc/config/aarch64/aarch64-builtins.c:888 0xc66318 aarch64_simd_expand_builtin(int, tree_node*, rtx_def*) $SRC/gcc/config/aarch64/aarch64-builtins.c:990 0xc66968 aarch64_expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int) etc... Now we get the more helpful: build-aarch64/install/lib/gcc/aarch64-none-elf/4.10.0/include/arm_neon.h:19371:10: error: incompatible type for argument 4, expected 'const int' return __builtin_aarch64_sqdmlal_lanev4hi (__a, __b, __c, __d); As for the testcase, we want to check that we give an error but do not ICE. The dg-excess-errors directive is the closest I've found to that. The test appears as an expected fail. If, however, we were to ICE it would appear as an unexpected failure, which is what we would want. Tested on aarch64-none-elf and bootstrapped on aarch64-linux. Ok for trunk? 2014-09-05 Kyrylo Tkachov PR target/61749 * config/aarch64/aarch64-builtins.c (aarch64_types_quadop_qualifiers): Use qualifier_immediate for last operand. Rename to... (aarch64_types_ternop_lane_qualifiers): ... This. (TYPES_QUADOP): Rename to... (TYPES_TERNOP_LANE): ... This. (aarch64_simd_expand_args): Return const0_rtx when encountering user error. Change return of 0 to return of NULL_RTX. (aarch64_crc32_expand_builtin): Likewise. (aarch64_expand_builtin): Return NULL_RTX instead of 0. ICE when expanding unknown builtin. * config/aarch64/aarch64-simd-builtins.def (sqdmlal_lane): Use TERNOP_LANE qualifiers. (sqdmlsl_lane): Likewise. (sqdmlal_laneq): Likewise. (sqdmlsl_laneq): Likewise. (sqdmlal2_lane): Likewise. (sqdmlsl2_lane): Likewise. (sqdmlal2_laneq): Likewise. (sqdmlsl2_laneq): Likewise. 2014-09-05 Kyrylo Tkachov PR target/61749 * gcc.target/aarch64/vqdml_lane_intrinsics-bad_1.c: New test. commit 796f7ec499411034d5eb7441b51d0493d6299327 Author: Kyrylo Tkachov Date: Wed Aug 6 16:47:29 2014 +0100 [AArch64] PR target/61749 Fix ICE when passing non-literal lane to some intrinsics diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c index ba58a99..16c9329 100644 --- a/gcc/config/aarch64/aarch64-builtins.c +++ b/gcc/config/aarch64/aarch64-builtins.c @@ -178,10 +178,10 @@ aarch64_types_ternopu_qualifiers[SIMD_MAX_BUILTIN_ARGS] #define TYPES_TERNOPU (aarch64_types_ternopu_qualifiers) static enum aarch64_type_qualifiers -aarch64_types_quadop_qualifiers[SIMD_MAX_BUILTIN_ARGS] +aarch64_types_ternop_lane_qualifiers[SIMD_MAX_BUILTIN_ARGS] = { qualifier_none, qualifier_none, qualifier_none, - qualifier_none, qualifier_none }; -#define TYPES_QUADOP (aarch64_types_quadop_qualifiers) + qualifier_none, qualifier_immediate }; +#define TYPES_TERNOP_LANE (aarch64_types_ternop_lane_qualifiers) static enum aarch64_type_qualifiers aarch64_types_getlane_qualifiers[SIMD_MAX_BUILTIN_ARGS] @@ -907,8 +907,11 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval, case SIMD_ARG_CONSTANT: if (!(*insn_data[icode].operand[argc + have_retval].predicate) (op[argc], mode[argc])) + { error_at (EXPR_LOCATION (exp), "incompatible type for argument %d, " "expected %", argc + 1); + return const0_rtx; + } break; case SIMD_ARG_STOP: @@ -975,7 +978,7 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval, } if (!pat) - return 0; + return NULL_RTX; emit_insn (pat); @@ -1071,8 +1074,9 @@ aarch64_crc32_expand_builtin (int fcode, tree exp, rtx target) op1 = copy_to_mode_reg (mode1, op1); pat = GEN_FCN (icode) (target, op0, op1); - if (! pat) - return 0; + if (!pat) + return NULL_RTX; + emit_insn (pat); return target; } @@ -1124,7 +1128,7 @@ aarch64_expand_builtin (tree exp, else if (fcode >= AARCH64_CRC32_BUILTIN_BASE && fcode <= AARCH64_CRC32_BUILTIN_MAX) return aarch64_crc32_expand_builtin (fcode, exp, target); - return NULL_RTX; + gcc_unreachable (); } tree diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def b/gcc/config/aarch64/aarch64-simd-builtins.def index 4f3bd12..94b81a8 100644 --- a/gcc/config/aarch64/aarch64-simd-builtins.def +++ b/gcc/config/aarch64/aarch64-simd-builtins.def @@ -157,16 +157,16 @@ BUILTIN_VSDQ_I (UNOP, sqabs, 0) BUILTIN_VSDQ_I (UNOP, sqneg, 0) - BUILTIN_VSD_HSI (QUADOP, sqdmlal_lane, 0) - BUILTIN_VSD_HSI (QUADOP, sqdmlsl_lane, 0) - BUILTIN_VSD_HSI (QUADOP, sqdmlal_laneq, 0) - BUILTIN_VSD_HSI (QUADOP, sqdmlsl_laneq, 0) + BUILTIN_VSD_HSI (TERNOP_LANE, sqdmlal_lane, 0) + BUILTIN_VSD_HSI (TERNOP_LANE, sqdmlsl_lane, 0) + BUILTIN_VSD_HSI (TERNOP_LANE, sqdmlal_laneq, 0) + BUILTIN_VSD_HSI (TERNOP_LANE, sqdmlsl_laneq, 0) BUILTIN_VQ_HSI (TERNOP, sqdmlal2, 0) BUILTIN_VQ_HSI (TERNOP, sqdmlsl2, 0) - BUILTIN_VQ_HSI (QUADOP, sqdmlal2_lane, 0) - BUILTIN_VQ_HSI (QUADOP, sqdmlsl2_lane, 0) - BUILTIN_VQ_HSI (QUADOP, sqdmlal2_laneq, 0) - BUILTIN_VQ_HSI (QUADOP, sqdmlsl2_laneq, 0) + BUILTIN_VQ_HSI (TERNOP_LANE, sqdmlal2_lane, 0) + BUILTIN_VQ_HSI (TERNOP_LANE, sqdmlsl2_lane, 0) + BUILTIN_VQ_HSI (TERNOP_LANE, sqdmlal2_laneq, 0) + BUILTIN_VQ_HSI (TERNOP_LANE, sqdmlsl2_laneq, 0) BUILTIN_VQ_HSI (TERNOP, sqdmlal2_n, 0) BUILTIN_VQ_HSI (TERNOP, sqdmlsl2_n, 0) /* Implemented by aarch64_sqdmll. */ diff --git a/gcc/testsuite/gcc.target/aarch64/vqdml_lane_intrinsics-bad_1.c b/gcc/testsuite/gcc.target/aarch64/vqdml_lane_intrinsics-bad_1.c new file mode 100644 index 0000000..314a624 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/vqdml_lane_intrinsics-bad_1.c @@ -0,0 +1,54 @@ +/* { dg-do compile } */ + +#include "arm_neon.h" + +int32x4_t +foo (int32x4_t a, int16x4_t b, int16x4_t c, int d) +{ + return vqdmlal_lane_s16 (a, b, c, d); +} + +int32x4_t +foo1 (int32x4_t a, int16x4_t b, int16x8_t c, int d) +{ + return vqdmlal_laneq_s16 (a, b, c, d); +} + +int32x4_t +foo2 (int32x4_t a, int16x4_t b, int16x4_t c, int d) +{ + return vqdmlsl_lane_s16 (a, b, c, d); +} + +int32x4_t +foo3 (int32x4_t a, int16x4_t b, int16x8_t c, int d) +{ + return vqdmlsl_laneq_s16 (a, b, c, d); +} + +int32x4_t +foo4 (int32x4_t a, int16x8_t b, int16x4_t c, int d) +{ + return vqdmlal_high_lane_s16 (a, b, c, d); +} + +int32x4_t +foo5 (int32x4_t a, int16x8_t b, int16x4_t c, int d) +{ + return vqdmlsl_high_lane_s16 (a, b, c, d); +} + +int32x4_t +foo6 (int32x4_t a, int16x8_t b, int16x8_t c, int d) +{ + return vqdmlal_high_laneq_s16 (a, b, c, d); +} + +int32x4_t +foo7 (int32x4_t a, int16x8_t b, int16x8_t c, int d) +{ + return vqdmlsl_high_laneq_s16 (a, b, c, d); +} + + +/* { dg-excess-errors "incompatible type for argument" } */