On Thu, Aug 13, 2020 at 06:46:05PM -0500, will schmidt wrote:
> > .../gcc.target/powerpc/int_128bit-runnable.c | 2254 +++++++++++++++++
>
> The path into the testsuite subdir looks strange there.
Git abbreviated this. It is autogenerated (git diffstat), so there is
nothing much you can do about it. The abbreviation is quite helpful
often (for moves for example), but not always, yup.
> > --- a/gcc/config/rs6000/altivec.h
> > +++ b/gcc/config/rs6000/altivec.h
> > @@ -183,7 +183,7 @@
> > #define vec_recipdiv __builtin_vec_recipdiv
> > #define vec_rlmi __builtin_vec_rlmi
> > #define vec_vrlnm __builtin_vec_rlnm
> > -#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((c)<<8)|(b)))
> > +#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((b)<<8)|(c)))
>
> per above. I don't see this change called out.
It looks like an accidental revert. Good catch :-)
commit e97929e20b2f52e6cfc046c1302324d1b24d95e3
Author: Carl Love <ca...@us.ibm.com>
Date: Wed Mar 25 18:33:37 2020 -0500
> > + /* The -mti-vector-ops option requires ISA 3.1 support and -maltivec for
> > + the 128-bit instructions. Currently, TARGET_POWER10 is sufficient to
> > + enable it by default. */
> > + if (TARGET_POWER10)
> > + {
> > + if (rs6000_isa_flags_explicit & OPTION_MASK_VSX)
> > + warning(0, ("%<-mno-altivec%> disables -mti-vector-ops (128-bit integer
> > vector register operations)."));
> > + else
> > + rs6000_isa_flags |= OPTION_MASK_TI_VECTOR_OPS;
> > + }
>
> It seems odd here that -maltivec is explicitly called out here. That
> should be default on for quite a while at this point.
And the actual check it for -mvsx anyway? Not sure I follow what this
does at all.
> > @@ -2305,6 +2306,7 @@ extern int frame_pointer_needed;
> > #define RS6000_BTM_P8_VECTOR MASK_P8_VECTOR /* ISA 2.07 vector. */
> > #define RS6000_BTM_P9_VECTOR MASK_P9_VECTOR /* ISA 3.0 vector. */
> > #define RS6000_BTM_P9_MISC MASK_P9_MISC /* ISA 3.0 misc. non-vector */
> > +#define RS6000_BTM_P10_128BIT MASK_POWER10 /* ISA P10 vector. */
>
> Should comment be 128-bit something? (not just P10 vector).
Yeah, or it should be called P10_VECTOR (and it is called ISA 3.1).
> > @@ -2436,6 +2438,7 @@ enum rs6000_builtin_type_index
> > RS6000_BTI_bool_V8HI, /* __vector __bool short */
> > RS6000_BTI_bool_V4SI, /* __vector __bool int */
> > RS6000_BTI_bool_V2DI, /* __vector __bool long */
> > + RS6000_BTI_bool_V1TI, /* __vector __bool long */
>
> Fix comment?
I wonder if the V2DI is correct even (should be "long long"?).
> > +mti-vector-ops
> > +Target Report Mask(TI_VECTOR_OPS) Var(rs6000_isa_flags)
> > +Use integer 128-bit instructions for a future architecture.
>
> 'future' can probably be adjusted.
Yes :-)
> > \ No newline at end of file
>
> diff error?
No, the file really should have a newline at the end. Not all editors
enforce that by default :-(
> > +(define_expand "vector_eqv1ti"
> > + [(set (match_operand:V1TI 0 "vlogical_operand")
> > + (eq:V1TI (match_operand:V1TI 1 "vlogical_operand")
> > + (match_operand:V1TI 2 "vlogical_operand")))]
> > + "TARGET_TI_VECTOR_OPS"
> > + "")
All the rest of this is in rs6000.md, won't "eqvv1ti3" work already?
> Since it's on all of the clauses, Maybe adjust the dg-require to
> include ppc_native_128bit for the whole test, unless there is more to
> follow.
Good plan :-)
Thanks for all the comments Will! Carl, could you fix things and resend
please? It's a rather big patch, we'll have to do it in stages :-/
Segher
