Re: Help with reload and naked constant sum causing ICE

[Richard Sandiford <rdsandiford at googlemail dot com>]

Andy H <hutchinsonandy@aim.com> writes:
>> If L_R_A does nothing with it,
>> the normal reload handling will first try:
>>
>>   (const:HI (plus:HI (symbol_ref:HI ("chk_fail_buf") (const_int 2))))
>>   
>
> This worked just as your described after I added test of 
> reg_equiv_constant[] inside L_R_A .
>
> So I guess that looks like  the fix for bug I posted.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34641
>
> To summarize
>
> LEGITIMIZE_RELOAD_ADDRESS should now always check reg_equiv_constant
> before it trying to do any push_reload of register.

TBH, I still think AVR is doing far too much in L_R_A.  To quote
the current version:

#define LEGITIMIZE_RELOAD_ADDRESS(X, MODE, OPNUM, TYPE, IND_LEVELS, WIN)    \
do {									    \
  if (1&&(GET_CODE (X) == POST_INC || GET_CODE (X) == PRE_DEC))	    \
    {									    \
      push_reload (XEXP (X,0), XEXP (X,0), &XEXP (X,0), &XEXP (X,0),	    \
	           POINTER_REGS, GET_MODE (X),GET_MODE (X) , 0, 0,	    \
		   OPNUM, RELOAD_OTHER);				    \
      goto WIN;								    \
    }									    \

Why does AVR need different POST_INC and PRE_DEC handling from
other auto-inc targets?  This at least deserves a comment.

But really, you should just let reload handle this case.
Does reload currently lack some support you need?

  if (GET_CODE (X) == PLUS						    \
      && REG_P (XEXP (X, 0))						    \
      && GET_CODE (XEXP (X, 1)) == CONST_INT				    \
      && INTVAL (XEXP (X, 1)) >= 1)					    \
    {									    \
      int fit = INTVAL (XEXP (X, 1)) <= (64 - GET_MODE_SIZE (MODE));	    \
      if (fit)								    \
	{								    \
          if (reg_equiv_address[REGNO (XEXP (X, 0))] != 0)		    \
	    {								    \
	      int regno = REGNO (XEXP (X, 0));				    \
	      rtx mem = make_memloc (X, regno);				    \
	      push_reload (XEXP (mem,0), NULL, &XEXP (mem,0), NULL,         \
		           POINTER_REGS, Pmode, VOIDmode, 0, 0,		    \
		           1, ADDR_TYPE (TYPE));			    \
	      push_reload (mem, NULL_RTX, &XEXP (X, 0), NULL,		    \
		           BASE_POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0, \
		           OPNUM, TYPE);				    \
	      goto WIN;							    \
	    }								    \
	  push_reload (XEXP (X, 0), NULL_RTX, &XEXP (X, 0), NULL,	    \
		       BASE_POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0,	    \
		       OPNUM, TYPE);					    \
          goto WIN;							    \
	}								    \
      else if (! (frame_pointer_needed && XEXP (X,0) == frame_pointer_rtx)) \
	{								    \
	  push_reload (X, NULL_RTX, &X, NULL,				    \
		       POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0,	    \
		       OPNUM, TYPE);					    \
          goto WIN;							    \
	}								    \
    }									    \
} while(0)

These too don't make much immediate sense to me.  What are they trying
to do that reload wouldn't do otherwise?

Rather than duplicating more of reload's checks in this macro, I think
it would be better to strip it down as far as possible.

There's a bit of self-interest here.  It makes it very hard to work on
reload if ports try to duplicate so much of the logic in target-specific
files.

Richard