This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RS6000] PR89271, gcc.target/powerpc/vsx-simode2.c


Hi!

On Fri, Mar 29, 2019 at 04:00:55PM +1030, Alan Modra wrote:
> On Thu, Mar 28, 2019 at 01:08:55PM -0500, Segher Boessenkool wrote:
> > Darwin doesn't do VSX at all...  But maybe there is something that can get
> > allocated to both FPRs and VRs, sure.  And GPRs.
> > 
> > This PREFERRED_RELOAD_CLASS, but it makes me worried if there are places
> > where we should care about this change for correctness.
> 
> For sure there are other places.  A new union register class trips all
> those places where union classes fail.  For example,
> ira.c:setup_class_translate_array doesn't give useful answers for
> GEN_OR_VSX_REGS, or GEN_OR_FLOAT_REGS for that matter.  That makes
> uses of ira_allocno_class_translate and ira_pressure_class_translate
> for pseudos suspect whenever one of the union classes is involved.

And not all of those places will fail in an obviouswayfail.  Well, stage 1,
we'll find them all in time :-)

> rs6000_ira_change_pseudo_allocno_class helps of course, in that
> GEN_OR_VSX_REGS mostly won't be used.

Heh.

> > > @@ -34966,22 +34970,56 @@ rs6000_register_move_cost (machine_mode mode,
> > >  			   reg_class_t from, reg_class_t to)
> > >  {
> > >    int ret;
> > > +  reg_class_t rclass;
> > >  
> > >    if (TARGET_DEBUG_COST)
> > >      dbg_cost_ctrl++;
> > >  
> > > +  /* If we have VSX, we can easily move between FPR or Altivec registers,
> > > +     otherwise we can only easily move within classes.
> > > +     Do this first so we give best-case answers for union classes
> > > +     containing both gprs and vsx regs.  */
> > > +  HARD_REG_SET to_vsx, from_vsx;
> > > +  COPY_HARD_REG_SET (to_vsx, reg_class_contents[to]);
> > > +  AND_HARD_REG_SET (to_vsx, reg_class_contents[VSX_REGS]);
> > > +  COPY_HARD_REG_SET (from_vsx, reg_class_contents[from]);
> > > +  AND_HARD_REG_SET (from_vsx, reg_class_contents[VSX_REGS]);
> > 
> > This is a bit expensive to run at every call of rs6000_register_move_cost.
> > Can it be precomputed easily?
> 
> register_move_cost tends to be cached by callers of this function, so
> I'm inclined to go for simple and correct rather than fast and
> complicated.

The main problem is that every op on HARD_REG_SET can be *very* expensive.
It isn't obvious to a reader how big it is.  But this is target code of
course, and for us it is only 128 bits these days.  Okay.

> > > +	{
> > > +	  if (TARGET_DIRECT_MOVE
> > > +	      && (rs6000_tune == PROCESSOR_POWER8
> > > +		  || rs6000_tune == PROCESSOR_POWER9))
> > 
> > TARGET_DIRECT_MOVE is always on for these CPUs.  Should this also use the
> > m*vsr* cost with say -mcpu=power7 -mtune=power9?
> 
> No, because if we don't generate m*vsr*, and we shouldn't, then that
> would be telling a lie.

Then should we have those "tune" things in this conditional?  Just do it
for any direct move target?

> > This list makes me think...  Should there be a GEN_OR_ALTIVEC as well?
> 
> That might make sense for pre-vsx processors, if you can find a
> testcase where the GENERAL_REGS cost is the same as ALTIVEC_REGS cost.
> Extending rs6000_ira_change_pseudo_allocno_class to handle
> GEN_OR_FLOAT_REGS and VSX_REGS for !TARGET_VSX might be an idea too.
> 
> Where the cost for moves between float and altivec is low, you'll find
> that VSX_REGS is always used as the allocno class whenever
> ALTIVEC_REGS is preferred.  So for power7 and later I expect
> GEN_OR_ALTIVEC_REGS wouldn't ever be used as an allocno class.

I was thinking for those instructions that work only on VRs.  But those
already had variants that work on float (like lxssp), or they are insn
for quad precision float (ieee128), and those don't go in GPRs anyway.

So nothing to worry about :-)


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]