Re: [PATCH, rs6000] Add support for int versions of vec_adde

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi Bill,

On Thu, Apr 07, 2016 at 04:04:30PM -0500, Bill Seurer wrote:
> On 04/05/16 21:27, David Edelsohn wrote:
> >On Tue, Apr 5, 2016 at 3:36 PM, Bill Seurer <seurer@linux.vnet.ibm.com> 
> >>         * config/rs6000/rs6000-c.c (altivec_overloaded_builtins,
> >>         altivec_resolve_overloaded_builtin): Remove 
> >>         ALTIVEC_BUILTIN_VEC_ADDE
> >>         from altivec_overloaded_builtins structure.  Add support for it 
> >>         to
> >>         altivec_resolve_overloaded_builtin function.

A total nit, but...  If you change one thing in one places, and another
in another, put them in the changelog separately.

> >>+         /* for {un}signed ints,
> >>+            vec_adde (va, vb, carryv) == vec_add (vec_add (va, vb), 
> >>carryv) */
> >
> >Same.

Also sentences start with a capital letter.

What is the definition of the builtin?  The machine insn does
va + vb + (carryv & 1); your implementation skips the AND.

> >>--- gcc/testsuite/gcc.target/powerpc/vec-adde-int128.c  (revision 0)
> >>+++ gcc/testsuite/gcc.target/powerpc/vec-adde-int128.c  (working copy)
> >>@@ -0,0 +1,78 @@
> >>+/* { dg-do run { target { powerpc64le-*-* } } } */

Will powerpc64 (i.e. BE) not work?


Segher