Hi Segher,
> On Apr 10, 2017, at 4:31 PM, Segher Boessenkool <seg...@kernel.crashing.org>
> wrote:
>
> 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 :-)
That's a nice idea. That would at least keep us from having to fix this in 2-3
dozen places.
Let me take a look.
Thanks!
Bill
>
>> 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).
OK.
>
>> --- 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
>
