[PATCH, rs6000] Add support for int versions of vec_addec
Segher Boessenkool
segher@kernel.crashing.org
Mon May 9 16:26:00 GMT 2016
Hi Bill,
On Fri, May 06, 2016 at 09:29:11AM -0500, Bill Seurer wrote:
> 2016-05-06 Bill Seurer <seurer@linux.vnet.ibm.com>
>
> * config/rs6000/rs6000-builtin.def (vec_addec): Change vec_addec to a
> special case builtin.
> * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Add
> support for ALTIVEC_BUILTIN_VEC_ADDEC.
> * config/rs6000/rs6000.c (altivec_init_builtins): Add definition
> for __builtin_vec_addec.
>
> [gcc/testsuite]
>
> 2016-05-06 Bill Seurer <seurer@linux.vnet.ibm.com>
>
> * gcc.target/powerpc/vec-addec.c: New test.
> * gcc.target/powerpc/vec-addec-int128.c: New test.
> + /* All 3 arguments must be vectors of (signed or unsigned) (int or
> + __int128) and the types must match. */
> + if ((arg0_type != arg1_type) || (arg1_type != arg2_type))
> + goto bad;
Superfluous parens; trailing space. Please fix (throughout).
> + /* For {un}signed ints,
> + vec_addec (va, vb, carryv) == vec_or (vec_addc (va, vb),
> + vec_addc(vec_add(va, vb),
> + vec_and (carryv, 0x1))). */
"0x1" looks really silly btw ;-)
> + /* Use save_expr to ensure that operands used more than once
> + that may have side effects (like calls) are only evaluated
> + once. */
> + arg0 = save_expr(arg0);
> + arg1 = save_expr(arg1);
> + vec<tree, va_gc> *params = make_tree_vector();
Space before function arg opening parenthesis.
> + vec_safe_push (params, arg0);
> + vec_safe_push (params, arg1);
> + tree call1 = altivec_resolve_overloaded_builtin
> + (loc, rs6000_builtin_decls[ALTIVEC_BUILTIN_VEC_ADDC], params);
That's not how you're supposed to break long lines. Probably easiest if you
first assign the decl to a new temporary?
> + tree const1 = build_vector_from_val (arg0_type,
> + build_int_cstu(TREE_TYPE (arg0_type), 1));
That's not right either.
> + tree and_expr = fold_build2_loc (loc, BIT_AND_EXPR,
> + arg0_type, arg2, const1);
Nor this. A continuation line should start at the same indent as the
thing it continues (so "arg0_type" should line up with "loc" in this case).
Segher
More information about the Gcc-patches
mailing list