Emit more REG_EQUIV notes for function args (PR42235)

When moving arguments into pseudos, we are being very careful not to
emit any instructions that could possibly clobber other argument
registers.  Currently, we generate unnecessarily complicated sequences
of code for simple zero or sign extensions.

With this patch, we check can_extend_p and the necessary predicates to
see if an extend insn is available for the conversion we have to do.  If
so, we emit the insn directly, and create a REG_EQUIV note of the form
(sign_extend (mem)).  Reload can't really do anything with these yet,
but there's an optimization in the register allocator to move argument
loads directly before their use if there's only one use.  On Thumb-2
this happens already, we seem to generate better code than before.
Unfortunately Thumb-1, which the PR is about, has some other problems
which I'll try to fix later.

Bootstrapped and regression tested on i686-linux.  Also regression
tested on arm-linux, with my three usual sets of options.  Ok?


Bernd
PR target/42235
	* function.c (assign_parm_setup_reg): When an optab for extending
	exists, emit the insn directly and create a REG_EQUIV note.

On 07/14/10 05:14, Bernd Schmidt wrote:
> When moving arguments into pseudos, we are being very careful not to
> emit any instructions that could possibly clobber other argument
> registers.  Currently, we generate unnecessarily complicated sequences
> of code for simple zero or sign extensions.
>
> With this patch, we check can_extend_p and the necessary predicates to
> see if an extend insn is available for the conversion we have to do.
Right, but what guarantee do we have that the conversion insn doesn't 
clobber a function argument register?   ISTM that to be safe you 
actually have to scan the insns created by gen_extend_insn to ensure 
they don't clobber something important.

I'm not an expert on what ports do these days, but I did work on a port 
(mn10200) where conversion "insns" where implemented as special function 
calls under the hood.  I don't recall if we allowed those special 
function calls to have visible side effects, but if they did, they'd 
show up as clobbers/uses attached to the normal conversion insn.    Of 
course the mn102 is dead, but I think it's method for implementing 
conversions was valid and if another port were to do something similar 
it would likely not interact well with your change.


>   If
> so, we emit the insn directly, and create a REG_EQUIV note of the form
> (sign_extend (mem)).
Creating more REG_EQUIVs seems like a good idea to me.



>    Reload can't really do anything with these yet,
> but there's an optimization in the register allocator to move argument
> loads directly before their use if there's only one use.  On Thumb-2
> this happens already, we seem to generate better code than before.
> Unfortunately Thumb-1, which the PR is about, has some other problems
> which I'll try to fix later.
>    
In theory, given the REG_EQUIV note we ought to get an entry in 
reg_equiv_mem, which the code I'm working on knows it can use instead of 
shoving the pseudo into a stack slot.  Effectively, my code will 
rematerialize the argument from the equivalent memory location 
regardless of the number of uses.


Jeff

On 07/14/2010 08:54 PM, Jeff Law wrote:
> On 07/14/10 05:14, Bernd Schmidt wrote:
>> When moving arguments into pseudos, we are being very careful not to
>> emit any instructions that could possibly clobber other argument
>> registers.  Currently, we generate unnecessarily complicated sequences
>> of code for simple zero or sign extensions.
>>
>> With this patch, we check can_extend_p and the necessary predicates to
>> see if an extend insn is available for the conversion we have to do.
> Right, but what guarantee do we have that the conversion insn doesn't
> clobber a function argument register?   ISTM that to be safe you
> actually have to scan the insns created by gen_extend_insn to ensure
> they don't clobber something important.
> 
> I'm not an expert on what ports do these days, but I did work on a port
> (mn10200) where conversion "insns" where implemented as special function
> calls under the hood.  I don't recall if we allowed those special
> function calls to have visible side effects, but if they did, they'd
> show up as clobbers/uses attached to the normal conversion insn.    Of
> course the mn102 is dead, but I think it's method for implementing
> conversions was valid and if another port were to do something similar
> it would likely not interact well with your change.

Hmm, ok.  That's awful, but I kind of expected someone would say that.
Did this really happen for integer zero/sign extend, or only for
floating point stuff?

If necessary I can try to test for a single insn with single_set and
push it to the sequence otherwise.


Bernd

On 07/14/2010 08:54 PM, Jeff Law wrote:
> In theory, given the REG_EQUIV note we ought to get an entry in
> reg_equiv_mem, which the code I'm working on knows it can use instead of
> shoving the pseudo into a stack slot.  Effectively, my code will
> rematerialize the argument from the equivalent memory location
> regardless of the number of uses.

Reload can do that already, I think, but it's not prepared to handle a
(zero_extend (mem)) inside a REG_EQUIV note.  That should be reasonably
trivial to add if the reg is never set other than in the initializing insn.


Bernd

On 07/14/10 15:47, Bernd Schmidt wrote:
> On 07/14/2010 08:54 PM, Jeff Law wrote:
>    
>> In theory, given the REG_EQUIV note we ought to get an entry in
>> reg_equiv_mem, which the code I'm working on knows it can use instead of
>> shoving the pseudo into a stack slot.  Effectively, my code will
>> rematerialize the argument from the equivalent memory location
>> regardless of the number of uses.
>>      
> Reload can do that already, I think, but it's not prepared to handle a
> (zero_extend (mem)) inside a REG_EQUIV note.  That should be reasonably
> trivial to add if the reg is never set other than in the initializing insn.
>    
reload does, but it's code that can be problematical.  For example, 
replacement of an unallocated pseudo with its equivalent can trigger 
secondary reloads, spills, etc.

My code creates a new pseudo/allocno for the various ranges where the 
unallocated pseudo is live then asks IRA to color those split-range 
pseudos/allocnos.  What we end up presenting to reload is cleaner; 
though often they result in the same final code.  Handling this case 
falls out nicely from the infrastructure for splitting ranges of 
unallocated pseudos without equivalent forms (it's probably a couple 
dozen lines of new code).

jeff

On 07/14/10 15:30, Bernd Schmidt wrote:
> On 07/14/2010 08:54 PM, Jeff Law wrote:
>    
>> On 07/14/10 05:14, Bernd Schmidt wrote:
>>      
>>> When moving arguments into pseudos, we are being very careful not to
>>> emit any instructions that could possibly clobber other argument
>>> registers.  Currently, we generate unnecessarily complicated sequences
>>> of code for simple zero or sign extensions.
>>>
>>> With this patch, we check can_extend_p and the necessary predicates to
>>> see if an extend insn is available for the conversion we have to do.
>>>        
>> Right, but what guarantee do we have that the conversion insn doesn't
>> clobber a function argument register?   ISTM that to be safe you
>> actually have to scan the insns created by gen_extend_insn to ensure
>> they don't clobber something important.
>>
>> I'm not an expert on what ports do these days, but I did work on a port
>> (mn10200) where conversion "insns" where implemented as special function
>> calls under the hood.  I don't recall if we allowed those special
>> function calls to have visible side effects, but if they did, they'd
>> show up as clobbers/uses attached to the normal conversion insn.    Of
>> course the mn102 is dead, but I think it's method for implementing
>> conversions was valid and if another port were to do something similar
>> it would likely not interact well with your change.
>>      
> Hmm, ok.  That's awful, but I kind of expected someone would say that.
> Did this really happen for integer zero/sign extend, or only for
> floating point stuff?
>    
It was all the PSImode crap.

> If necessary I can try to test for a single insn with single_set and
> push it to the sequence otherwise.
>    
For the mn103, the conversions were single insns...

Ultimately,  I think you have to peek at the insn(s) and see what 
registers they set/clobber.

jeff