On Mon, Apr 10, 2017 at 12:08:50PM -0500, Bill Schmidt wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80376 shows an ICE on invalid
> code when using certain flavors of the vec_xxpermdi intrinsic. The root
> cause is that we were not checking the parameter for a legal value, and an
> illegal value was being provided by the user (only an integer constant is
> permissible for argument 3).
>
> However, this brings up a slightly larger problem, in that the invalid
> vec_xxpermdi call is nested within another call. We have a lot of cases
> in rs6000.c where we signal an error message and replace the offending
> construct with a const0_rtx. In this case, that const0_rtx was being used
> as a vector argument to another call, leading to a follow-up ICE that is
> not easy to parse.
i386 does this:
/* Errors in the source file can cause expand_expr to return const0_rtx
where we expect a vector. To avoid crashing, use one of the vector
clear instructions. */
static rtx
safe_vector_operand (rtx x, machine_mode mode)
{
if (x == const0_rtx)
x = CONST0_RTX (mode);
return x;
}
used in e.g. ix86_expand_binop_builtin as
if (VECTOR_MODE_P (mode0))
op0 = safe_vector_operand (op0, mode0);
We might want something similar :-)
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 246804)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -15586,6 +15586,11 @@ rs6000_expand_ternop_builtin (enum insn_code icode
> || icode == CODE_FOR_vsx_xxpermdi_v2di
> || icode == CODE_FOR_vsx_xxpermdi_v2df_be
> || icode == CODE_FOR_vsx_xxpermdi_v2di_be
> + || icode == CODE_FOR_vsx_xxpermdi_v1ti
> + || icode == CODE_FOR_vsx_xxpermdi_v4sf
> + || icode == CODE_FOR_vsx_xxpermdi_v4si
> + || icode == CODE_FOR_vsx_xxpermdi_v8hi
> + || icode == CODE_FOR_vsx_xxpermdi_v16qi
> || icode == CODE_FOR_vsx_xxsldwi_v16qi
> || icode == CODE_FOR_vsx_xxsldwi_v8hi
> || icode == CODE_FOR_vsx_xxsldwi_v4si
The existing code is indented with spaces only; please keep the style
consistent (fixing it is fine, but then the whole block or function).
> --- gcc/config/rs6000/vector.md (revision 246804)
> +++ gcc/config/rs6000/vector.md (working copy)
> @@ -109,6 +109,11 @@
> {
> if (CONSTANT_P (operands[1]))
> {
> + /* Handle cascading error conditions. */
> + if (VECTOR_MODE_P (<MODE>mode)
> + && !VECTOR_MODE_P (GET_MODE (operands[1])))
> + internal_error ("non-vector constant found where vector expected");
> +
> if (FLOAT128_VECTOR_P (<MODE>mode))
> {
> if (!easy_fp_constant (operands[1], <MODE>mode))
You might not need this with the suggestion above?
The patch is okay if you want that for now; it's an improvement over
what we have.
Segher
