Re: [Patch] PR21990

[Björn Haase <bjoern dot m dot haase at web dot de>]

You wrote Dienstag, 6. September 2005 07:22 :
> Björn Haase <bjoern.m.haase@web.de> writes:
> >
> > Denis Chertykov's older version using "frame_pointer_required_p ()" has
> > now vanished. A problem during register allocation has been removed by
> > Roger Sayle's patch. However, the old frame pointer bug has been
> > re-introduced.
>
> Are you have a testcase for this bug ?
> I'm interested in testcase for current GCC (not old).
No. Unfortunately. The only testcase I have is for an old gcc. I think that we 
are having a hidden problem that is very difficult to produce: The present 
code allows only Pmode in the Y register so one would probably need a test 
function with many many pointer variables. Everything depends very much on 
the register allocator.

Even if it is difficult to prove: I am now sure that we are having a 
regression to 3.4.x in both, mainline and 4.0.x .

IMO two things need to be done in avr_hard_regno_mode_ok:
1.) Check for the frame pointer difficulty that produced both, your earlier 
QImode spill register problem and PR21990,
2.) One needs to prevent that DImode or SImode variables eat up all the 
pointer registers, e.g. a DImode variable in r24..r31 so that reload does not 
know how to find a free a pointer register. Now only Pmode is allowed in Y. 
This could be realized also by forcing that only 1 and 2 byte modes could go 
into Y and Z. The present implementation for this problem (allow only Pmode 
in Y) is more restrictive than necessary, in my opinion.

My suggestion to proceed as follows:

1.) The risk to insert bugs by checking against (frame_pointer_needed) or 
frame_pointer_required_p () is effectively zero. We had at least shown for 
old revisions that we are having a hidden problem that could be fixed this 
way. And the testsuite run says that it's ok. So let's add such a check into 
the 4.0.x branch as a regression fix.
2.) For mainline I'd like to suggest to change also that allowing only Pmode 
in Y is overly restrictive. IMO it would be OK to allow both, QImode and 
HImode. Here also the testsuite run says that it's ok.

Yours,

Bjoern.