PR63633: May middle-end come up width hard regs for insn expanders?

[Vladimir Makarov]

On 20/04/15 06:27 PM, Steven Bosscher wrote:
> On Mon, Apr 20, 2015 at 10:11 PM, Vladimir Makarov wrote:
>> I might be wrong but I think you have a bloated code because you use
>> scratches.  I already told several times that usage of scratch is always a
>> bad idea.  It was a bad idea for an old RA and is still a bad idea for IRA.
>> The usage of scratches should be prohibited, probably we should write it
>> somewhere.  It is better to use just a regular pseudo instead.
> Thanks Vladimir, I didn't know this.
> Does this mean that, for example, extendsidi in i386.md would be
> better if it did not use match_scratch?
>
> The expander and the 32-bits version of the insn currently look like this:
>
> (define_expand "extendsidi2"
>    [(set (match_operand:DI 0 "register_operand")
>          (sign_extend:DI (match_operand:SI 1 "register_operand")))]
>    ""
> {
>    if (!TARGET_64BIT)
>      {
>        emit_insn (gen_extendsidi2_1 (operands[0], operands[1]));
>        DONE;
>      }
> })
>
> (define_insn "extendsidi2_1"
>    [(set (match_operand:DI 0 "nonimmediate_operand" "=*A,r,?r,?*o")
>          (sign_extend:DI (match_operand:SI 1 "register_operand" "0,0,r,r")))
>     (clobber (reg:CC FLAGS_REG))
>     (clobber (match_scratch:SI 2 "=X,X,X,&r"))]
>    "!TARGET_64BIT"
>    "#") // there is a post reg-alloc splitter
>
> If I understand your remark on SCRATCH correctly, then the expander
> should use gen_reg_rtx for operand 2 of extendsidi2_1; and the insn
> should use match_operand on operand 2 instead of match_scratch.
> Correct?
>
This case is more complicated than one for the AVR patch where scratch 
should always get a register.  I've should not have been too 
unconditional about prohibiting scratches.
> When is a scratch still OK?
There are some insns where scratch will rest as scratch after RA. The 
above insn is an example (constraint X).  If it is changed by pseudo in 
RA, the pseudo in such case might get a stack slot. It is not a big deal 
as scratch pseudo are short-live one and probability of stack space 
increase is small.  But for small stackless functions the penalty could 
be big.

For the above case it is also hard for me to say now will the pseudo use 
in IRA improve the allocation because we don't know yet what is the 
final insn alternative.  We might assign a hard reg in IRA and spill 
other pseudos for this, although the final alternative will use 
constraint X.

If scratch should always get a register, that what you describe above 
would be a right thing to do.  In such case using scratch is really a 
bad thing.
> The most common pattern in i386 is (clobber (match_scratch ...)))
> which probably always results in the need for a register that IRA
> doesn't see, so I assume that this is one of the cases where you would
> recommend using a pseudo instead...? (A quick grep on config/*.md
> shows 952 cases like this, and 113 match_scratch uses of a different
> form.)
>
> A match_scratch or (clobber (scratch)) in a define_peephole2 should
> also always be fine, because that's just a way to see if there is a
> suitable register available to perform the peephole code
> transformation (peephole2 runs after reg-alloc).
>
> (mem:BLK (scratch)) is always OK,
>
Right, there are cases where scratch is useful.
>> Why it is a bad idea?  Because IRA (or the old global RA) does not take
>> them into account *at all*.  It means that IRA might think that there are
>> enough registers for pseudos but in reality it is wrong because of
>> scratches in live range of the pseudos.
> Is there a reason why IRA doesn't replace scratches with pseudos, like
> LRA does (and IIRC reload does, also)?
>
As I remember reload does not do it but of course it takes scratches 
into account.  When reload processes insn, it looks what reload 
registers are needed for operands including scratches.  LRA process all 
pseudos not just reload ones having a bigger picture and of course it 
should looks at scratch too.  All scratches are changed by pseudos to 
simplify implementation of LRA and spilled pseudo are transformed back 
into scratches.

IRA could use the same approach with scratches as LRA but as I wrote 
above for some cases it is hard to say is it a right thing to do as we 
don't know the final insn alternative.  For LRA, changing scratches by 
pseudos is the best approach as we assign hard registers after deciding 
the final insn alternative.

So I should have written in my first email that usage of scratches 
should be deprecated when it has register constraints without X.