Hi! On Wed, Dec 15, 2021 at 08:21:24AM -0600, Bill Schmidt wrote: > While replacing the built-in machinery, we agreed to defer some necessary > refactoring of the overload processing. This patch cleans it up considerably. > > I've put in one FIXME for an additional level of cleanup that should be done > independently. The various helper functions (resolve_VEC_*) can be simplified > if we move the argument processing in altivec_resolve_overloaded_builtin > earlier. But this requires making nontrivial changes to those functions that > will need careful review. Let's do that in a later patch.
All these names should be lower case. If you promise to do that in the aforementioned cleanup (and it happens before GCC 12), then okay for trunk. Thanks! Very minor stuff below. > * config/rs6000/rs6000-c.c (resolution): New enum. > (resolve_VEC_MUL): New function. > (resolve_VEC_CMPNE): Likewise. > (resolve_VEC_ADDE_SUBE): Likewise. > (resolve_VEC_ADDEC_SUBEC): Likewise. > (resolve_VEC_SPLATS): Likewise. > (resolve_VEC_EXTRACT): Likewise. > (resolve_VEC_INSERT): Likewise. > (resolve_VEC_STEP): Likewise. > (find_instance): Likewise. > (altivec_resolve_overloaded_builtin): Many cleanups: Call factored-out No two spaces after colon, no capital after colon. You probably want a full stop though? > functions. Move variable declarations closer to uses. Add commentary. > Remove unnecessary levels of braces. Avoid use of gotos. Change > misleading variable names. Use switches over if-else-if chains. > + /* Note: vec_nand also works but opt changes vec_nand's > + to vec_nor's anyway. */ Maybe there should be a vec_not? There is one at the RTL level (called one_cmpl<mode>2). > + decl= rs6000_builtin_decls[RS6000_OVLD_VEC_NOR]; Missing space btw. > - support Altivec's overloaded builtins. FIXME: This code needs > - to be brutally factored. */ Yay :-) > /* Return immediately if this isn't an overload. */ > + rs6000_gen_builtins fcode > + = (rs6000_gen_builtins) DECL_MD_FUNCTION_CODE (fndecl); > + > if (fcode <= RS6000_OVLD_NONE) > return NULL_TREE; The new code should be before this comment? I don't see how the comment makes much sense like this. > + /* Some overloads require special handling. */ > + /* FIXME: Could we simplify the helper functions if we gathered arguments > + and types into arrays first? */ A lot of the argument checking can be handled more generically. If there are many exceptions to that it will not be useful, but a bit of it would make sense. If you do that (and maybe similar things as well) this array question will swivel. > - if (TREE_CODE (arg0_type) != VECTOR_TYPE) > - goto bad; > - if (!lang_hooks.types_compatible_p (arg0_type, arg1_type)) > - goto bad; Yay, all gotos should go :-) Thanks again, Segher
