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  <kyrylo.tkac...@arm.com>

    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  <kyrylo.tkac...@arm.com>

    PR target/61749
    * gcc.target/aarch64/vqdml_lane_intrinsics-bad_1.c: New test.
commit 796f7ec499411034d5eb7441b51d0493d6299327
Author: Kyrylo Tkachov <kyrylo.tkac...@arm.com>
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 %<const int%>", 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_sqdml<SBINQOPS:as>l<mode>.  */
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" } */

Reply via email to