Hi Carl,
On Fri, Jun 30, 2017 at 01:32:01PM -0700, Carl Love wrote:
> vector unsigned __int128 vec_sube (vector unsigned __int128, vector
> unsigned __int128, vector unsigned __int128);
Many of the changelog lines are much too long.
> * gcc.target/powerpc/p8vector-builtin-8.c (foo): Add test cases for
> the new vec_subc, vec_sube, vec_subec built-ins. Add the missing test
> cases for vec_addc, adde and addec builtins.
Two spaces after a full stop.
> @@ -5855,13 +5864,14 @@ altivec_resolve_overloaded_builtin (location_t loc,
> tree fndecl,
> /* else, fall through and process the Power9 alternative below */
> }
>
> - if (fcode == ALTIVEC_BUILTIN_VEC_ADDE)
> + if ((fcode == ALTIVEC_BUILTIN_VEC_ADDE)
> + || (fcode == ALTIVEC_BUILTIN_VEC_SUBE))
> {
> /* vec_adde needs to be special cased because there is no instruction
> for the {un}signed int version. */
> if (nargs != 3)
> {
> - error ("vec_adde only accepts 3 arguments");
> + error ("vec_adde and vec_sube only accepts 3 arguments");
> return error_mark_node;
> }
Please only print the relevant name; see how this is handled for
ALTIVEC_BUILTIN_VEC_SPLATS and ALTIVEC_BUILTIN_VEC_PROMOTE, for example.
>
> @@ -5884,14 +5894,24 @@ altivec_resolve_overloaded_builtin (location_t loc,
> tree fndecl,
> {
> /* For {un}signed ints,
> vec_adde (va, vb, carryv) == vec_add (vec_add (va, vb),
> + vec_and (carryv, 0x1)).
> + vec_sube (va, vb, carryv) == vec_sub (vec_sub (va, vb),
> vec_and (carryv, 0x1)). */
s/0x1/1/ (yeah I realise this was here before, but let's stop the silliness).
> @@ -5919,13 +5951,14 @@ altivec_resolve_overloaded_builtin (location_t loc,
> tree fndecl,
> }
> }
>
> - if (fcode == ALTIVEC_BUILTIN_VEC_ADDEC)
> + if ((fcode == ALTIVEC_BUILTIN_VEC_ADDEC)
> + || (fcode == ALTIVEC_BUILTIN_VEC_SUBEC))
No superfluous parentheses please, they don't help readability.
> + if (fcode == ALTIVEC_BUILTIN_VEC_ADDEC)
> + {
> + tree VADDECUQ_bii =
> rs6000_builtin_decls[P8V_BUILTIN_VEC_VADDECUQ];
Line too long. Just name the local variable something shorter? You don't
need VADDECUQ in the name.
> +vector signed __int128 vec_sube (vector signed __int128,
> + vector signed __int128,
> + vector signed__int128);
Missing space in that last line.
Some of the above happen more than once, please check.
Okay for trunk with those issues fixed. Thanks!
Segher
