Hi!

On Wed, Sep 01, 2021 at 11:13:43AM -0500, Bill Schmidt wrote:
>       * config/rs6000/rs6000-call.c (rs6000_invalid_new_builtin):
>       Implement.

That fits on one line.  Don't wrap early, esp. not if that leaves a
colon without anything following it on that line: it looks like
something is missing.

>       (rs6000_expand_ldst_mask): Likewise.
>       (rs6000_init_builtins): Initialize altivec_builtin_mask_for_load.


>  static void
>  rs6000_invalid_new_builtin (enum rs6000_gen_builtins fncode)
>  {
> +  size_t uns_fncode = (size_t) fncode;

Like in the previous patch, the "uns_*" name made me think "you do not
need an explicit cast, the assignment will do that automatically".  But
of course it does not matter this is unsigned at all: the cast is
casting an enum to a number, which in C++ does require a cast.

So maybe you can think of some better name?  Something like "j" is fine
with me as well btw, it's nice and short, and it is clear you do not
want more meaning ;-)

> +  switch (rs6000_builtin_info_x[uns_fncode].enable)

> +    case ENB_P6:
> +      error ("%qs requires the %qs option", name, "-mcpu=power6");
> +      break;

> +    case ENB_CELL:
> +      error ("%qs is only valid for the cell processor", name);
> +      break;

Maybe  "%qs requires the %qs option", name, "-mcpu=cell"  ?  Boring is
good ;-)

> +    };

(This is  switch (...) { ... };  )
Stray semi.  Was there no warning?

>  rtx
>  rs6000_expand_ldst_mask (rtx target, tree arg0)
>   {
> +  int icode2 = BYTES_BIG_ENDIAN

You do not need a line break here.

> +    ? (int) CODE_FOR_altivec_lvsr_direct
> +    : (int) CODE_FOR_altivec_lvsl_direct;

You can align the ? and : just fine without it.

> +  rtx op, addr, pat;

Don't declare such things early.

Okay for trunk with those things fixed.  Thanks!


Segher

Reply via email to