Hi Carl, On Tue, Oct 17, 2017 at 09:56:43AM -0700, Carl Love wrote: > gcc/ChangeLog: > > 2017-10-17 Carl Love <c...@us.ibm.com> > > * config/rs6000/rs6000-c.c (P8V_BUILTIN_VEC_REVB): > Add power 8 definitions for the builtin instances. > (P9V_BUILTIN_VEC_REVB): Remove the power 9 instance > definitions. > * config/rs6000/altivec.h (vec_revb): Change the > #define from power 9 to power 8. > * config/rs6000/r6000-protos.h (swap_selector_for_mode): Add extern > declaration. > * config/rs6000/rs6000.c (swap_selector_for_mode): Add > endian option to function. > * config/rs6000/rs6000-builtin.def (BU_P8V_VSX_1, > BU_P8V_OVERLOAD_1): Add power 8 macro expansions. > (BU_P9V_OVERLOAD_1): Remove power 9 overload expansion. > * config/rs6000/vsx.md (revb_<mode>): Add define_expand > to generate power 8 instructions for the vec_revb builtin. > > gcc/testsuite/ChangeLog: > > 2017-10-17 Carl Love <c...@us.ibm.com> > > * gcc.target/powerpc/builtins-revb-runnable.c: New > runnable test file for the vec_revb builtin.
You make all changelog lines really short... They are wrapped at 79 chars or so normally. > @@ -1853,6 +1853,13 @@ BU_P6_64BIT_2 (CMPB, "cmpb", CONST, cmpbdi3) > /* 1 argument VSX instructions added in ISA 2.07. */ > BU_P8V_VSX_1 (XSCVSPDPN, "xscvspdpn", CONST, vsx_xscvspdpn) > BU_P8V_VSX_1 (XSCVDPSPN, "xscvdpspn", CONST, vsx_xscvdpspn) > +BU_P8V_VSX_1 (REVB_V1TI, "revb_v1ti", CONST, revb_v1ti) > +BU_P8V_VSX_1 (REVB_V2DI, "revb_v2di", CONST, revb_v2di) > +BU_P8V_VSX_1 (REVB_V4SI, "revb_v4si", CONST, revb_v4si) > +BU_P8V_VSX_1 (REVB_V8HI, "revb_v8hi", CONST, revb_v8hi) > +BU_P8V_VSX_1 (REVB_V16QI, "revb_v16qi", CONST, revb_v16qi) > +BU_P8V_VSX_1 (REVB_V2DF, "revb_v2df", CONST, revb_v2df) > +BU_P8V_VSX_1 (REVB_V4SF, "revb_v4sf", CONST, revb_v4sf) The other entries have a tab instead of spaces between the last two columns; please fix. > +/* Return a constant vector for use as a big endian or little-endian > + permute control vector to reverse the order of elements of the > + given vector mode. */ > +rtx > +swap_selector_for_mode (machine_mode mode, bool endian) What does a bool "endian" mean? True if it is the one true endianness? :-) You probably should have a swap_endianness parameter, instead? So that most callers use "false". Or, better, make a separate function that swaps endianness, so that most callers can just use what they did before, and no magic boolean parameters. > + if ( endian == VECTOR_ELT_ORDER_BIG) No space after paren open. > +;; Iterator for xxperm types supported by VSX > +(define_mode_iterator XXBR_L [V16QI > + V8HI > + V4SI > + V2DI > + V4SF > + V2DF > + V1TI]) That's all normal vector types; is there no existing iterator you can use? > +;; Attribute for xxbr instructions > +(define_mode_attr VSX_XXBR [(V16QI "q_v16qi") > + (V8HI "h_v8hi") > + (V4SI "w_v4si") > + (V2DI "d_v2di") > + (V4SF "w_v4sf") > + (V2DF "d_v2df") > + (V1TI "q_v1ti")]) That's <wd>_<mode>? Well, except that needs entries for V4SF and V2DF as well then. > + > + DONE; > +} > + > +;; [(set_attr "type" "vecperm")]) > +) I think you forgot to finish this part? Rest looks good. Segher
