Hi Bill,

On Fri, May 06, 2016 at 09:29:11AM -0500, Bill Seurer wrote:
> 2016-05-06  Bill Seurer  <seu...@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  <seu...@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

Reply via email to