[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