Hi! On Mon, Apr 26, 2021 at 09:36:12AM -0700, Carl Love wrote: > This patch adds the 128-bit integer support for divide, modulo, shift, > compare of 128-bit integers instructions and builtin support.
> (rs6000_gimple_fold_builtin) [P10V_BUILTIN_VCMPEQUT, > P10_BUILTIN_CMPNET, P10_BUILTIN_CMPGE_1TI, > P10_BUILTIN_CMPGE_U1TI, P10_BUILTIN_VCMPGTUT, > P10_BUILTIN_VCMPGTST, P10_BUILTIN_CMPLE_1TI, > P10_BUILTIN_CMPLE_U1TI]: New case statements. > (builtin_function_type)[P10_BUILTIN_128BIT_VMULEUD, > P10_BUILTIN_128BIT_VMULOUD, P10_BUILTIN_128BIT_DIVEU_V1TI, > P10_BUILTIN_128BIT_MODU_V1TI, P10_BUILTIN_CMPGE_U1TI, > P10_BUILTIN_VCMPGTUT, P10_BUILTIN_VCMPEQUT]: New case statements. All P10_ here should be P10V_. > * config/rs6000/r6000.c (rs6000_handle_altivec_attribute)[E_TImode, > E_V1TImode]: New case statements. Space between ) and [. > +(define_insn "altivec_eqv1ti" > + [(set (match_operand:V1TI 0 "altivec_register_operand" "=v") > + (eq:V1TI (match_operand:V1TI 1 "altivec_register_operand" "v") > + (match_operand:V1TI 2 "altivec_register_operand" "v")))] > + "TARGET_POWER10" > + "vcmpequq %0,%1,%2" > + [(set_attr "type" "veccmpfx")]) There already is (define_insn "altivec_eq<mode>" [(set (match_operand:VI2 0 "altivec_register_operand" "=v") (eq:VI2 (match_operand:VI2 1 "altivec_register_operand" "v") (match_operand:VI2 2 "altivec_register_operand" "v")))] "<VI_unit>" "vcmpequ<VI_char> %0,%1,%2" [(set_attr "type" "veccmpfx")]) so changing the iterator VI2 to also include V1TI (or making a new iterator VI3 or something that includes it) will make this much easier. (You also need the add something to VI_char; VI_unit already has it). There are multiple iterators that can be used already. Especially since we have the "isa" and "enabled" attributes now :-) So maybe we can come up with a good logical name for it. This can be done later. But in the future, have an eye out if you can just use existing patterns, it saves work :-) The shift type things have the amount to shift by in the wrong place, so that is a bit of a kink. All the builtin stuff... I didn't see mistakes, but I am so glad it will all be rewritten soon, that is soo hard to look at :-) > +(define_expand "vector_gtv1ti" > + [(set (match_operand:V1TI 0 "vlogical_operand") > + (gt:V1TI (match_operand:V1TI 1 "vlogical_operand") > + (match_operand:V1TI 2 "vlogical_operand")))] > + "TARGET_POWER10" > + "") So this will require extending VEC_C a bit if we merge patterns. > +;; Swap upper/lower 64-bit values in a 128-bit vector > +(define_insn "xxswapd_v1ti" > + [(set (match_operand:V1TI 0 "vsx_register_operand" "=v") > + (subreg:V1TI > + (vec_select:V2DI > + (subreg:V2DI > + (match_operand:V1TI 1 "vsx_register_operand" "v") 0 ) > + (parallel [(const_int 1)(const_int 0)])) > + 0))] There are spaces instead of tabs on most of these lines. Okay for trunk with the trivialities fixed. Also okay for GCC 11 once it has been tested on all CPUs and OSes (all we care about that it :-) ) Thanks! Segher