On Mon, Jun 23, 2014 at 4:25 PM, Richard Henderson <r...@redhat.com> wrote:
> On 06/20/2014 01:42 PM, Marek Polacek wrote:
>> 2014-06-20 Marek Polacek <pola...@redhat.com>
>>
>> * genpreds.c (verify_rtx_codes): New function.
>> (main): Call it.
>> * rtl.h (RTX_FLD_WIDTH, RTX_HWINT_WIDTH): Define.
>> (struct rtx_def): Use them.
>
> Looks pretty good. Just a few nits.
>
>> +static void
>> +verify_rtx_codes (void)
>> +{
>> + unsigned int i, j;
>> +
>> + for (i = 0; i < NUM_RTX_CODE; i++)
>> + if (strchr (GET_RTX_FORMAT (i), 'w') == NULL)
>> + {
>> + if (strlen (GET_RTX_FORMAT (i)) > RTX_FLD_WIDTH)
>> + internal_error ("%s format %s longer than RTX_FLD_WIDTH %d\n",
>> + GET_RTX_NAME (i), GET_RTX_FORMAT (i),
>> + (int) RTX_FLD_WIDTH);
>> + }
>> + else
>> + {
>> + const size_t len = strlen (GET_RTX_FORMAT (i));
>
> The strlen result is used in both arms of the if. Tidier to hoist it, I
> think.
>
>> + for (j = 0; j < len; j++)
>> + if (GET_RTX_FORMAT (i)[j] != 'w')
>> + internal_error ("%s format %s should contain only w, but "
>> + "has %c\n", GET_RTX_NAME (i), GET_RTX_FORMAT (i),
>> + GET_RTX_FORMAT (i)[j]);
>
> The loop is strspn. Perhaps tidier as
>
> const size_t spn = strspn(GET_RTX_FORMAT (i), "w");
> if (spn != len)
> internal_error (...);
Note that RTX_HWINT_WIDTH is wrong because of CONST_WIDE_INT.
Also I don't like increasing the array sizes - this is wrong in the same
way as [1] is.
Can we instead refactor expmed.c to avoid allocating rtx_def directly?
Like by using rtx in init_expmed_rtl and allocating from an obstack
(or not care and GC-allocate anyway).
Richard.
>
> r~
>
