Re: regrename creates invalid insn
> Here, I think the problem is that we have an in-out operand whose chain > is closed prematurely due to a bogus REG_DEAD note which shouldn't be > there for a register set in the instruction. IIRC I didn't see a REG_DEAD note, but I might be misremembering. -- Eric Botcazou
Re: regrename creates invalid insn
On 03/26/2012 07:37 PM, Eric Botcazou wrote: >> Does 4.7 still have the failure at all? I've checked with the 4.6 >> branch, and regrename gets confused because there's a REG_DEAD note for >> the register, and another REG_UNUSED for the same reg. As far as I >> remember, it used to be the case that there should not be a REG_DEAD >> note for a register that gets set in the insn, but maybe df changed the >> rules? Or maybe it was a df bug in 4.6? > > My understanding is that the REG_UNUSED note causes the chain opened for a > dest > register operand to be immediately closed but, when you have multiple such > dest register operands, one would need to have the chain live "during the > instruction" or right after, so that you have a conflict with the other dest > register operands for the instruction. This looks awkward though. You're right on the behaviour of REG_UNUSED, but it only takes effect after all destinations have been opened. So they still conflict with each other, usually. Here, I think the problem is that we have an in-out operand whose chain is closed prematurely due to a bogus REG_DEAD note which shouldn't be there for a register set in the instruction. Bernd
Re: regrename creates invalid insn
> Does 4.7 still have the failure at all? I've checked with the 4.6 > branch, and regrename gets confused because there's a REG_DEAD note for > the register, and another REG_UNUSED for the same reg. As far as I > remember, it used to be the case that there should not be a REG_DEAD > note for a register that gets set in the insn, but maybe df changed the > rules? Or maybe it was a df bug in 4.6? My understanding is that the REG_UNUSED note causes the chain opened for a dest register operand to be immediately closed but, when you have multiple such dest register operands, one would need to have the chain live "during the instruction" or right after, so that you have a conflict with the other dest register operands for the instruction. This looks awkward though. -- Eric Botcazou
Re: regrename creates invalid insn
Bernd Schmidt writes: > Does 4.7 still have the failure at all? Yes, see PR52573. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: regrename creates invalid insn
On 03/13/2012 12:41 AM, Andreas Schwab wrote: > Andreas Schwab writes: > >> Ian Lance Taylor writes: >> >>> Andreas Schwab writes: >>> Ian Lance Taylor writes: > But it also looks like the pattern should use a match_scratch. It is also used as input in operand 2. >>> >>> Sorry, I missed that. >> >> That appears not to be an issue actually, there is already one use of >> match_scratch together with a matching constraint in *cmpdi_internal. >> But then, using match_scratch instead of match_operand doesn't really >> fix the bug either (it only helps a simplified test case, but not the >> original one). > > It doesn't actually change anything (I was confused because 4.7/4.8 no > longer generates the overlapping output for the simplified testcase). Does 4.7 still have the failure at all? I've checked with the 4.6 branch, and regrename gets confused because there's a REG_DEAD note for the register, and another REG_UNUSED for the same reg. As far as I remember, it used to be the case that there should not be a REG_DEAD note for a register that gets set in the insn, but maybe df changed the rules? Or maybe it was a df bug in 4.6? Bernd
Re: regrename creates invalid insn
Andreas Schwab writes: > Ian Lance Taylor writes: > >> Andreas Schwab writes: >> >>> Ian Lance Taylor writes: >>> But it also looks like the pattern should use a match_scratch. >>> >>> It is also used as input in operand 2. >> >> Sorry, I missed that. > > That appears not to be an issue actually, there is already one use of > match_scratch together with a matching constraint in *cmpdi_internal. > But then, using match_scratch instead of match_operand doesn't really > fix the bug either (it only helps a simplified test case, but not the > original one). It doesn't actually change anything (I was confused because 4.7/4.8 no longer generates the overlapping output for the simplified testcase). Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: regrename creates invalid insn
Ian Lance Taylor writes: > Andreas Schwab writes: > >> Ian Lance Taylor writes: >> >>> But it also looks like the pattern should use a match_scratch. >> >> It is also used as input in operand 2. > > Sorry, I missed that. That appears not to be an issue actually, there is already one use of match_scratch together with a matching constraint in *cmpdi_internal. But then, using match_scratch instead of match_operand doesn't really fix the bug either (it only helps a simplified test case, but not the original one). > This still seems like a bug in regrename to me, but it also seems like > an unusual case, so it is less surprising that it hasn't been seen > before. Since regrename is only auto-enabled with -funroll-loops it is probably not much tested anyway. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: regrename creates invalid insn
Andreas Schwab writes: > Ian Lance Taylor writes: > >> But it also looks like the pattern should use a match_scratch. > > It is also used as input in operand 2. Sorry, I missed that. This still seems like a bug in regrename to me, but it also seems like an unusual case, so it is less surprising that it hasn't been seen before. Ian
Re: regrename creates invalid insn
Ian Lance Taylor writes: > But it also looks like the pattern should use a match_scratch. It is also used as input in operand 2. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: regrename creates invalid insn
Andreas Schwab writes:
> When regrename runs it turns it into this:
>
> (insn 6 27 7 2 (parallel [
> (set (reg:SI 1 %d1 [35])
> (truncate:SI (lshiftrt:DI (mult:DI (sign_extend:DI (reg:SI 1
> %d1 [36]))
> (const_int -2004318071 [0x8889]))
> (const_int 32 [0x20]
> (clobber (reg:SI 1 %d1 [36]))
> ]) mulsl.c:4 192 {const_smulsi3_highpart}
> (expr_list:REG_UNUSED (reg:SI 1 %d1 [36])
> (nil)))
>
> Note that both output operands are allocated to the same register (which
> generates a muls.l producing undefined results). Is this transformation
> actually valid? I can stop regrename from doing this by marking operand
> 0 as earlyclobber, but the insn doesn't actually write the outputs
> before reading the inputs, the issue is the dependency between the
> outputs.
I assume there were two different hard register numbers before
regrename. That does not look like a valid transformation to me.
But it also looks like the pattern should use a match_scratch.
Ian
