On Tue, Jul 27, 2021 at 04:07:22PM -0500, will schmidt wrote:
> On Thu, 2021-06-17 at 10:19 -0500, Bill Schmidt via Gcc-patches wrote:
> > +  else if (type == bool_V16QI_type_node)
> > +    return "vbc";
> > +  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";
> 
> I'd be strongly tempted to rearrange the order and put V16 after V8 in
> the list.  Similar to the order you previously used in
> rs6000_expand_new_builtin().     Same comment elsewhere.

These are ordered on return value.  It is hard to make some order of all
these disparate things based on the actual type, but the strings is a
neat way out ;-)

(A comment "ordered by return value" would be good to have).

> > +  /*
> >    if (TARGET_DEBUG_BUILTIN)
> >      fprintf (stderr, "rs6000_builtin, code = %4d, %s%s\n",
> >          (int)code, name, attr_string);
> > +  */
> 
> Could probably just drop that chunk, instead of commenting it out. 

Or fix up its spacing :-P

> > +      for (int i = 1; i < (int) RS6000_BIF_MAX; i++)

That is an good reason to *not* have the max as enum value, btw: you
need a cast to use it.  Make the max a macro, and then it can include
all casting you need right in there :-)


Segher

Reply via email to