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