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