On Tue, Aug 25, 2015 at 07:09:48PM -0500, Segher Boessenkool wrote:
> On Wed, Aug 26, 2015 at 08:40:49AM +0930, Alan Modra wrote:
> > On Tue, Aug 25, 2015 at 10:08:54AM -0700, Segher Boessenkool wrote:
> > > -(define_insn_and_split "*and<mode>3_imm_dot_shifted"
> > > -  [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y")
> > > +(define_insn "*and<mode>3_imm_dot_shifted"
> > > +  [(set (match_operand:CC 3 "cc_reg_operand" "=x")
> > 
> > Is this really the best solution?  The operand predicate allows any
> > cr, but the constraint only cr0.  In the past we've seen this sort of
> > thing result in "insn does not satisfy its constraints" errors,
> 
> I don't think that can happen here.  It is the same situation as
> gpc_reg_operand with a "b" constraint.

Yeah, I'm not sure that it could happen.

> > and if
> > the operand is successfully reloaded you'll get slow mcrf insns.
> 
> What is slow about those?  It's not mfcr :-)

mcrf has a 3 cycle latency on p8, I believe.

> Also note that prior compilers (at least as far back as 4.7) generated
> it here, too (at -O2 etc. even).
> 
> >  At
> > -O1 the testcase generates:
> > 
> >         andi. 8,3,0x16
> >         mcrf 4,0
> > 
> > I started throwing together a patch yesterday, before you claimed the
> > bug.  With this patch, I see what looks to be better code despite it
> > being larger:
> > 
> >         li 9,22
> >         and 9,3,9
> >         cmpdi 4,9,0
> 
> The alternative that I was trying to avoid is
> 
>       andi. 8,3,0x16
>       cmpdi 4,8,0
> 
> which doesn't have such a long dependency chain.

But does destroy cr0.

> > +  [(set (match_dup 0) (match_dup 2))
> 
> This won't work for 0xa000 etc.

Oops, yes, you're right.

-- 
Alan Modra
Australia Development Lab, IBM

Reply via email to