Message ID | 20140620204259.GE14420@redhat.com |
---|---|
State | New |
Headers | show |
On 06/20/2014 01:42 PM, Marek Polacek wrote: > 2014-06-20 Marek Polacek <polacek@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 (...); r~
On Mon, Jun 23, 2014 at 4:25 PM, Richard Henderson <rth@redhat.com> wrote: > On 06/20/2014 01:42 PM, Marek Polacek wrote: >> 2014-06-20 Marek Polacek <polacek@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~ >
diff --git gcc/genpreds.c gcc/genpreds.c index b14a4ac..7e62124 100644 --- gcc/genpreds.c +++ gcc/genpreds.c @@ -1471,6 +1471,36 @@ parse_option (const char *opt) return 0; } +/* Verify RTX codes. */ + +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)); + 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]); + if (len > RTX_HWINT_WIDTH) + internal_error ("%s format %s longer than RTX_HWINT_WIDTH %d\n", + GET_RTX_NAME (i), GET_RTX_FORMAT (i), + (int) RTX_HWINT_WIDTH); + } +} + /* Master control. */ int main (int argc, char **argv) @@ -1518,5 +1548,7 @@ main (int argc, char **argv) if (have_error || ferror (stdout) || fflush (stdout) || fclose (stdout)) return FATAL_EXIT_CODE; + verify_rtx_codes (); + return SUCCESS_EXIT_CODE; } diff --git gcc/rtl.h gcc/rtl.h index 6ec91a8..3f2e774 100644 --- gcc/rtl.h +++ gcc/rtl.h @@ -264,6 +264,12 @@ struct GTY((variable_size)) hwivec_def { #define CWI_PUT_NUM_ELEM(RTX, NUM) \ (RTL_FLAG_CHECK1("CWI_PUT_NUM_ELEM", (RTX), CONST_WIDE_INT)->u2.num_elem = (NUM)) +/* The maximum number of entries in the FLD array in rtx. */ +#define RTX_FLD_WIDTH 8 + +/* The maximum number of entries in the HWINT array in rtx. */ +#define RTX_HWINT_WIDTH (MAX (REAL_WIDTH, 3)) + /* RTL expression ("rtx"). */ struct GTY((chain_next ("RTX_NEXT (&%h)"), @@ -378,8 +384,8 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"), The number of operands and their types are controlled by the `code' field, according to rtl.def. */ union u { - rtunion fld[1]; - HOST_WIDE_INT hwint[1]; + rtunion fld[RTX_FLD_WIDTH]; + HOST_WIDE_INT hwint[RTX_HWINT_WIDTH]; struct block_symbol block_sym; struct real_value rv; struct fixed_value fv;