Re: [named address] ice-on-valid: in postreload.c:reload_cse_simplify_operands

[Georg-Johann Lay <avr@gjlay.de>]

Ulrich Weigand wrote:
> Georg-Johann Lay wrote:
> 
>> Thanks, it works.
> 
> OK, thanks for testing!
> 
>> 	std Y+2,r31	 ;  30	*movphi/3	[length = 7]
>> 	std Y+1,r30
> 
> I'm actually not seeing those (maybe I'm using a different code
> base than you were using ...)
> 
> But I still see that the frame is created.  The problem is that IRA
> thinks it needs to allocate a pseudo on the stack, and creates a
> stack slot for it.  But then reload goes and just reloads the
> pseudo into a hard register anyway, and simply doesn't need the
> stack slot ... but it was already allocated and accounted for
> such that get_frame_size () no longer returns 0.

The stack slot is an IRA hog because there are plenty of hard regs.

>> I frequently see IRA doing a very bad job for small register classes
>> like here.  Maybe it's better to take it completely away from IRA
>> and write the load as
>>
>> (set (reg:HI)
>>      (unspec:HI (post_inc:PHI (reg:PHI Z))))
>>
>> Loading from __pgm is actually an unspec, i.e. reading two times from
>> the same address will yield the same result.
> 
> This really seems to be a problem in IRA somewhere, but I'd guess it
> would be better to fix in there instead of working around it.  Maybe
> you should open a bug an get in touch with the IRA maintainers ...

It surely is an IRA issue.  However, it's extremely unlikely that anyone
cares for SSA/RTL optimization flaws that just show up on AVR or are
reported against AVR.  And if a patch turns out to overcharge IRA
like, e.g. in
   http://gcc.gnu.org/ml/gcc-patches/2011-07/msg02655.html
the patch won't be integrated.

Moreover, this one is hard to reproduce because it needs you patch,
my patch and is for a "irrelevant" target.

Johann

> Bye,
> Ulrich