Bug 31786

This must be specific to RTEMS/newlib, as 4.2.0-20070430 (RC2) builds without error with:

CFLAGS=-D__USE_MINGW_ACCESS  \
../gcc-$version/configure \
    --prefix=$installdir \
    --target=avr \
    --enable-languages=c,c++ \
    --with-dwarf2 \
    --enable-win32-registry=WinAVR-$release \
    --disable-nls \
    --with-gmp=/usr/local \
    --with-mpfr=/usr/local \
    --enable-doc \
    --disable-libssp \
    2>&1 | tee $package-configure.log 

for host=mingw.

I can also reproduce the bug with 4.2.0-20070501.

It also is related to optimization levels. Newlib is being compiled with -O2, which triggers this breakdown. Using -O1 or -Os to build init.c lets the breakdown vanish.

Eric, due to bugzilla's current brokenness, I am sending you the *.i on PM.

Created attachment 13501 [details]
*.i of the breakdown above

Subject: RE:  error: unable to find a register to spill in
 class 'BASE_POINTER_REGS'

 

> ------- Comment #2 from ralf_corsepius at rtems dot org  
> 2007-05-03 10:27 -------
> I can also reproduce the bug with 4.2.0-20070501.
> 
> It also is related to optimization levels. Newlib is being 
> compiled with -O2,
> which triggers this breakdown. Using -O1 or -Os to build 
> init.c lets the
> breakdown vanish.
> 
> Eric, due to bugzilla's current brokenness, I am sending you 
> the *.i on PM.

Hi Ralf,
All public distros use avr-libc instead of newlib to build an AVR toolchain:
<http://savannah.nongnu.org/projects/avr-libc>
AFAIK, it is only RTEMS that attempts to use newlib. And because of this the
only testing that is done is when you do it, so I'm not surprised that
newlib is broken for the avr.

Eric

Subject: RE:  error: unable to find a register to spill
	in class 'BASE_POINTER_REGS'

On Thu, 2007-05-03 at 06:05 -0600, Eric Weddington wrote:
>  
> > ------- Comment #2 from ralf_corsepius at rtems dot org  
> > 2007-05-03 10:27 -------
> > I can also reproduce the bug with 4.2.0-20070501.
> > 
> > It also is related to optimization levels. Newlib is being 
> > compiled with -O2,
> > which triggers this breakdown. Using -O1 or -Os to build 
> > init.c lets the
> > breakdown vanish.
> > 
> > Eric, due to bugzilla's current brokenness, I am sending you 
> > the *.i on PM.
> 
> Hi Ralf,
> All public distros use avr-libc instead of newlib to build an AVR toolchain:
> <http://savannah.nongnu.org/projects/avr-libc>
> AFAIK, it is only RTEMS that attempts to use newlib. And because of this the
> only testing that is done is when you do it, so I'm not surprised that
> newlib is broken for the avr.
Though what you say is true, the file avr bombs out on is mere "c", i.e.
a bug in GCC.

Ralf

This is the minimised *.c having been extracted from newlib's file exposing the bug:
-- snip --
extern void (*array_start []) (void);
extern void (*array_end []) (void);

void
init_array (void)
{
  int count;
  int i;
  count = array_end - array_start;
  for (i = 0; i < count; i++)
    array_start[i] ();
}
-- snip --

Pretty trivial code, isn't it?

FYI: Vanilla gcc-4.1.2 without newlib also exposes this issue with the code fragment from comment #6:

./avr-gcc -o tmp.o -c init.c -O2
init.c: In function 'init_array':
init.c:12: error: unable to find a register to spill in class 'BASE_POINTER_REGS'
init.c:12: error: this is the insn:
(insn 61 30 31 2 (set (mem/c:HI (plus:HI (reg/f:HI 28 r28)
                (const_int 1 [0x1])) [4 S2 A8])
        (reg:HI 24 r24)) 12 {*movhi} (nil)
    (nil))
init.c:12: confused by earlier errors, bailing out

Confirmed. Code snippet fails for 4.1.2 when compiling for -O2 and -O3. Note that compiling with -O[0,1,s] is successful.

Changing target to all avr.

From Bjoern Haase:

Hi all,

I think that we could resolve this ICE by adding an unnamed pattern like

(define_insn "*strangeMovhi"
  [(set (mem:HI (plus:HI (reg:HI 28)
                         (const_int 1)))
        (match_operand:HI 0 "general_operand" "r"))]
  ""
  "st %A0,y+1 \n\t st %B0,y+2"
  [(set_attr "length" "2")])


to the machine description "avr.md" . That might cure the symptoms and does not resolve the underlying problem with reload, but this might be a fix for the immediate issue.

Note that above remark is untested pseudo-code.

Yours,

Bjoern.

P.S.: Eric, you might add this remark to the bugzilla entry.

Fails for 4.3-20070525

*** Bug 24894 has been marked as a duplicate of this bug. ***

According to a comment in duplicate bug #24894, bug #19636 may be related.

Ralf, can you try the test case using a 4.3 snapshot?

Problem occurs because 1 base pointer is used for function pointer and the other is used for frame pointer. (There are only 2). Backend LEGITIMIZE_RELOAD_ADDRESS tells reload to use a base pointer for reload pass of mem(SP+1). This fails.
Unfortunately reload does not figure out that it can use frame pointer and gives ICE. (Ironically the frame pointer would be eliminated in this testcase latter)

The problem potentially occurs whenever R30,R31 is tied up. (EEPROM macros are other example)

Reload calls LEGITIMIZE_RELOAD_ADDRESS when strict address check fails legitimate_address_p on SP+1 - since AVR has no way of indexing from stack pointer.

Potential fix is to stop LEGITIMIZE_RELOAD_ADDRESS from telling reload (push_reload)  to use BASE_POINTER class. POINTER CLASS is one wider and target macros have pattern to use register 'REG_X' as index (with addition). Alternatively don't suggest any substitution (ie not GOTO WIN) and reload finds answer.

The latter solution has been tested and caused ICE in GCC library building as 'REG_X' is not allowable with strict address checking (legitimate_address_p ) - so we get no pattern match ICE for MOVQI (REG_X+1),Rxx. However, backend does contain such patterns. Changing legitimate_address_p to accept REG_X solves problem.
In the testcase, the instruction that caused the ICE is eliminated - so the newly allowable pattern is never used.

The build ICE caused by first change does indicate that on occasion GCC will really use REG_X as index. Potentially producing sub-optimal code compared with REG_Y or REG_Z. However, the incidence is clearly low and may well be offset by other side effect improvements.

Created attachment 14862 [details]
Patch to current HEAD

This will patch avr.h and avr.c to allow REG_X as valid BASE_POINTER.

Adjusting target milestone.

Closing 4.1 branch.

Subject: Bug 31786

Author: aesok
Date: Sun Sep 14 12:50:10 2008
New Revision: 140360

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140360
Log:
	PR target/19636
	PR target/24894
	PR target/31644
	PR target/31786
	* config/avr/avr.c (legitimate_address_p): Fix problem where subreg
	is not recognized as a valid register usage. Allow REG_X to be used
	as a base pointer.
	* config/avr/avr.h (LEGITIMIZE_RELOAD_ADDRESS): Remove code that
	forces a reload when using a base register.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.h

Closing 4.2 branch.  Is the patch committed on 14 September 2008 a complete
fix?  If so, the 4.4 and 4.5 regression markers should be removed from the
summary.

GCC 4.3.4 is being released, adjusting target milestone.

I have in my list that this bug as fixed in 4.4.0. Closing.