Re: PATCH: Simplify x86 load/store builtin expand

[Uros Bizjak <ubizjak at gmail dot com>]

H.J. Lu wrote:

> > >  x86 backend has many exceptions, for store, load, asymetric types, ...
> > >  when expanding x86 builtins. AVX has masked load/store builtins,
> > >  which need new exceptions. This patch removes bdesc_1arg,
> > >  ix86_expand_store_builtin and ix86_expand_unop_builtin.
> > >  ix86_expand_args_builtin can handle load/store builtins with
> > >  more than one argument. I am testing on Linux/ia32 and Linux/Intel64.
> > >  Ok for trunk if there are no regressions?
> > >       
> > IMO the best solution is to have all non-complicated const builtins
> > grouped in one table, perhaps nicely grouped by SSE level. All
> > non-const builtins should then be added explicitly using def_builtin,
> > hoping that there won't be many of them.
> >
> > Your proposed patch is a step in right direction (there are lots of
> > builtins...), but please leave out "memory" handling from the array
> > scanning loop. We can consider builtins that require "memory" flag to
> > be complicated, non-const ones.
> >
> >     
>
> Here is the updated patch. OK to install?
>   
For the moment, please leave builtins, defined explicitly with 
builtin_const () function where they are. I propose that const builtins 
only are converted in this patch, and non-const builtins to be handled 
in the last step, in a separate patch when all const builtins are 
converted. This way, they won't mix in any case.

BTW: I think that usign "... complex arguments" is not good choice as 
this combination usually refers to complex numbers.
> -/* SSE */
> -enum sse_builtin_type
> +/* Complex builtin types */
> +enum ix86_complex_builtin_type
>  {
> -  SSE_CTYPE_UNKNOWN,
> +  COMPLEX_FTYPE_UNKNOWN,
> +  V16QI_FTYPE_PCCHAR,
> +  V4SF_FTYPE_PCFLOAT,
> +  V2DF_FTYPE_PCDOUBLE,
> +  V2DI_FTYPE_PV2DI,
> +  VOID_FTYPE_PV2DI_V2DI,
> +  VOID_FTYPE_PCHAR_V16QI,
> +  VOID_FTYPE_PFLOAT_V4SF,
> +  VOID_FTYPE_PDOUBLE_V2DF,
> +  VOID_FTYPE_PDI_DI,
> +  VOID_FTYPE_PINT_INT,
> +};
>   
The part above should be committed as a separate patch to avoid mixing 
const and non-const builtins. (Also, we should use some other name than 
"complex" for the reason described above).

> -/* SSE builtins with variable number of arguments.  */
> -static const struct builtin_description bdesc_sse_args[] =
> +/* Builtins with variable number of complex arguments.  */
> +static const struct builtin_description bdesc_complex_args[] =
>   

This part should also be removed from the patch for now, together with 
the code that expands non-const builtins.

The "const" part of the patch is OK for mainline.

Thanks for taking this cleanup,
Uros.