On Thu, Aug 04, 2016 at 10:03:36AM -0500, Segher Boessenkool wrote:
> Hi Mike,
>
> On Thu, Aug 04, 2016 at 12:33:44AM -0400, Michael Meissner wrote:
> > I built spec 2006 with these patches on a little endian power8 system, and
> > at
> > least 18 of the benchmarks had vector initializations replaced. Most
> > benchmarks only used the initialization in a few places, but gamess, dealII,
> > h264ref, and wrf each had over 100 initializations changed.
>
> Did performance change?
I ran a selected set of spec benchmarks, and I saw one regression (3.5% in
tonto). I'll run the full suite, and see if I can see what the slow down is.
> > I have tried these patches on a big endian power7 system (both 32-bit and
> > 64-bit targets), on a big endian power8 system (just 64-bit targets), and a
> > little endian power8 system (just 64-bit targets). There were no
> > regressions
> > on any of the systems. Can I install these patches to the trunk?
>
> Some questions below, okay for trunk with those taken care of. Thanks.
>
>
> > --- gcc/config/rs6000/rs6000.c
> > (.../svn+ssh://meiss...@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000)
> > (revision 239098)
> > +++ gcc/config/rs6000/rs6000.c (.../gcc/config/rs6000) (working copy)
> > @@ -6736,6 +6736,38 @@ rs6000_expand_vector_init (rtx target, r
> > return;
> > }
> >
> > + /* Special case initializing vector int if we are on 64-bit systems with
> > + direct move. This bug tickles a bug in reload for fortran's
> > + cray_pointers_2 test unless -mvsx-timode is enabled. */
>
> "This bug"? It's not clear to me what this says, could you rephrase?
> Just say what the code does, not what would happen without the code. Or
> say both.
Here is the comment I tried to reword to be clearer. Since the bug only occurs
if you do -mno-lra -mno-vsx-timode, I made the test to be those conditions. I
don't think it is worth the time at this point to track down what the reload
issue is.
/* Special case initializing vector int if we are on 64-bit systems with
direct move. This optimization tickles a bug in RELOAD for fortran's
cray_pointers_2 test unless -mvsx-timode is enabled (the register
allocator is trying to load up a V4SImode vector in GPRs with a TImode
address using a SUBREG). Since RELOAD is no longer the default register
allocator, just don't do the optimization. */
if (mode == V4SImode && TARGET_DIRECT_MOVE_64BIT
&& (TARGET_LRA || TARGET_VSX_TIMODE))
> > +static inline int
> > +regno_or_subregno (rtx op)
> > +{
> > + if (REG_P (op))
> > + return REGNO (op);
> > + else if (SUBREG_P (op))
> > + return subreg_regno (op);
> > + else
> > + gcc_unreachable ();
> > +}
>
> Maybe this should check the subreg is lowpart, too? For robustness.
No, subreg_regno already does that. Since this is just infrastructure for a
potential future change, I can remove this change until the next patch.
> > ;; Build a V2DF/V2DI vector from two scalars
> > (define_insn "vsx_concat_<mode>"
> > - [(set (match_operand:VSX_D 0 "vsx_register_operand" "=<VSr>,?<VSa>")
> > + [(set (match_operand:VSX_D 0 "gpc_reg_operand" "=<VSa>,we")
> > (vec_concat:VSX_D
> > - (match_operand:<VS_scalar> 1 "vsx_register_operand" "<VS_64reg>,<VSa>")
> > - (match_operand:<VS_scalar> 2 "vsx_register_operand"
> > "<VS_64reg>,<VSa>")))]
> > + (match_operand:<VS_scalar> 1 "gpc_reg_operand" "<VS_64reg>,r")
> > + (match_operand:<VS_scalar> 2 "gpc_reg_operand" "<VS_64reg>,r")))
> > + (clobber (match_scratch:DI 3 "=X,X"))]
>
> X,X? How is that useful?
Because I had a more elaborate version for vsx_concat_<mode> that did need a
base register for memory addresses. I missed deleting the useless clobber. I
deleted it in my source tree that will become the next iteration of the patch.
When I add support for vector insert to/from memory, I will probably need the
clobbers (and this BTW, was the reason to split off regno_or_subregno).
> > + (set_attr "length" "4")])
>
> One insn is the default.
Yep. I had been working on a larger change, and some of the other alternatives
did have other lengths.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
