On Wed, Sep 01, 2021 at 11:13:50AM -0500, Bill Schmidt wrote: > * config/rs6000/rs6000-call.c (rs6000_debug_type): New function. > (def_builtin): Change debug formatting for easier parsing and > include more information. > (rs6000_init_builtins): Add dump of autogenerated builtins. > (altivec_init_builtins): Dump __builtin_altivec_mask_for_load for > completeness.
> /* Builtins. */ Maybe change this header now? It heads "def_builtin" before this patch, maybe it should just move back there? And as any function comment, it should describe the function arguments (and return value, but that is void there :-) ) > +/* Debug utility to translate a type node to a single token. */ It returns a text string, instead. The function name can be better: > +static > +const char *rs6000_debug_type (tree type) "debug_" suggests this function outputs something to stderr. > +{ > + if (type == void_type_node) > + return "void"; > + else if (type == long_integer_type_node) > + return "long"; > + else if (type == long_unsigned_type_node) > + return "ulong"; > + else if (type == long_long_integer_type_node) > + return "longlong"; > + else if (type == long_long_unsigned_type_node) > + return "ulonglong"; > + else if (type == bool_V2DI_type_node) > + return "vbll"; > + else if (type == bool_V4SI_type_node) > + return "vbi"; > + else if (type == bool_V8HI_type_node) > + return "vbs"; > + else if (type == bool_V16QI_type_node) > + return "vbc"; > + else if (type == bool_int_type_node) > + return "bool"; > + else if (type == dfloat64_type_node) > + return "_Decimal64"; > + else if (type == double_type_node) > + return "double"; > + else if (type == intDI_type_node) > + return "sll"; > + else if (type == intHI_type_node) > + return "ss"; > + else if (type == ibm128_float_type_node) > + return "__ibm128"; > + else if (type == opaque_V4SI_type_node) > + return "opaque"; > + else if (POINTER_TYPE_P (type)) > + return "void*"; > + else if (type == intQI_type_node || type == char_type_node) > + return "sc"; > + else if (type == dfloat32_type_node) > + return "_Decimal32"; > + else if (type == float_type_node) > + return "float"; > + else if (type == intSI_type_node || type == integer_type_node) > + return "si"; > + else if (type == dfloat128_type_node) > + return "_Decimal128"; > + else if (type == long_double_type_node) > + return "longdouble"; > + else if (type == intTI_type_node) > + return "sq"; > + else if (type == unsigned_intDI_type_node) > + return "ull"; > + else if (type == unsigned_intHI_type_node) > + return "us"; > + else if (type == unsigned_intQI_type_node) > + return "uc"; > + else if (type == unsigned_intSI_type_node) > + return "ui"; > + else if (type == unsigned_intTI_type_node) > + return "uq"; > + else if (type == unsigned_V1TI_type_node) > + return "vuq"; > + else if (type == unsigned_V2DI_type_node) > + return "vull"; > + else if (type == unsigned_V4SI_type_node) > + return "vui"; > + else if (type == unsigned_V8HI_type_node) > + return "vus"; > + else if (type == unsigned_V16QI_type_node) > + return "vuc"; > + else if (type == V16QI_type_node) > + return "vsc"; > + else if (type == V1TI_type_node) > + return "vsq"; > + else if (type == V2DF_type_node) > + return "vd"; > + else if (type == V2DI_type_node) > + return "vsll"; > + else if (type == V4SF_type_node) > + return "vf"; > + else if (type == V4SI_type_node) > + return "vsi"; > + else if (type == V8HI_type_node) > + return "vss"; > + else if (type == pixel_V8HI_type_node) > + return "vp"; > + else if (type == pcvoid_type_node) > + return "voidc*"; > + else if (type == float128_type_node) > + return "_Float128"; > + else if (type == vector_pair_type_node) > + return "__vector_pair"; > + else if (type == vector_quad_type_node) > + return "__vector_quad"; > + else > + return "unknown"; > +} Please use a switch statement for this. You can call the variable "type_node" then as well, which would be a good idea. > if (TARGET_DEBUG_BUILTIN) > - fprintf (stderr, "rs6000_builtin, code = %4d, %s%s\n", > - (int)code, name, attr_string); > + { > + tree t = TREE_TYPE (type); > + fprintf (stderr, "%s %s (", rs6000_debug_type (t), name); > + t = TYPE_ARG_TYPES (type); > + while (t && TREE_VALUE (t) != void_type_node) Can both 0 and void_type_node terminate a list here? > + { > + fprintf (stderr, "%s", > + rs6000_debug_type (TREE_VALUE (t))); This easily fits on one line. > + t = TREE_CHAIN (t); > + if (t && TREE_VALUE (t) != void_type_node) > + fprintf (stderr, ", "); It is easier to use a "bool first" extra var, you do not need to write the same condition twice that way. bool first = true; while (...) { if (!first) fprintf ...; first = false; rest of loop body; } > + } > + fprintf (stderr, "); %s [%4d]\n", attr_string, (int)code); > + } (space after cast) > > static const struct builtin_compatibility bdesc_compat[] = > @@ -16097,6 +16209,67 @@ rs6000_init_builtins (void) > /* Execute the autogenerated initialization code for builtins. */ > rs6000_init_generated_builtins (); > > + if (TARGET_DEBUG_BUILTIN) > + { (misindent) > + if (e == ENB_P10_64 && (!TARGET_POWER10 || !TARGET_POWERPC64)) if (e == ENB_P10_64 && !(TARGET_POWER10 && TARGET_POWERPC64)) It even is shorter in this case ;-) > + if (TARGET_DEBUG_BUILTIN) > + fprintf (stderr, "%s __builtin_altivec_mask_for_load (%s); [%4d]\n", > + rs6000_debug_type (TREE_TYPE (v16qi_ftype_pcvoid)), > + rs6000_debug_type (TREE_VALUE > + (TYPE_ARG_TYPES (v16qi_ftype_pcvoid))), Never start a line with a paren from a function call. Often using an extra variable is the best solution? Okay for trunk with those things touched up. Thanks! Segher