On Tue, Jul 02, 2019 at 07:09:20PM -0500, Segher Boessenkool wrote:
> On Tue, Jul 02, 2019 at 07:36:21PM -0400, Michael Meissner wrote:
> > On Mon, Jul 01, 2019 at 04:27:05PM -0500, Segher Boessenkool wrote:
> > > The entry before the 8 is split as well.  Maybe that should be "4", to
> > > stand out?  I don't know what works better; your choice.
> > 
> > I'll look into it.  Note, the length is used in two places.  One at the end 
> > to
> > generate the appropriate branches, but the other is in rs6000_insn_cost 
> > inside
> > rs6000.c.
> 
> We'll need to update our insn_cost for prefixed, sure, it currently does
>   int n = get_attr_length (insn) / 4;
> to figure out how many machine instructions a pattern is, and then uses
> "n" differently for the different types of insn.  We'll need to refine
> this a bit for prefixed instructions.

Yes, I have some plans with this regard.  In particular, I will be introducing
a "num_insns" RTL attribute, that if set is the number of instructions that
will be emitted.

If "num_insns" is not set, then it will use the length and look at the
"prefixed" attribute.

> > This occurs before the final passes, so it is important that even
> > though the insn will be split, that the length is still set.  However, 
> > things
> > are rather inconsistant, in that sometimes the length field is accurate, and
> > sometimes not.
> 
> It *has* to be correct; it is allowed to be pessimistic though.  This
> is used to determine if a B-form branch can reach, for example.  You get
> ICEs if it isn't correct.  Only on very few testcases, of course :-(

To be clear.  Yes it has to be correct when the label handling is done.

What I was talking about is I've found some insns that don't set the length,
and are split.  Using the insn cost mechanism will mean that these instructions
will be thought of being cheaper than they actually are.

> > I'm finding that the rs6000_insn_cost issue really muddies things up,
> > particularly if a vector load/store insn is done in gpr registers, where it 
> > can
> > be 4 insns.
> 
> Yeah, it doesn't handle vectors specially *at all* currently, not even
> for FP in vectors.  It is pretty much just a switch over the "type", and
> for everything vector it gets to
>     default:
>       cost = COSTS_N_INSNS (n);
> (with the above "n") which is a pretty coarse approximation to the truth ;-)
> 
> You could try #undef'ing TARGET_INSN_COST (in rs6000.c) for now, and hope
> that rs6000_rtx_costs does better for what you need right now.  In the end
> it will have to be fixed properly, insn_cost is quite important.
> 
> 
> Segher
> 

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797

Reply via email to