New gcc port (ATMEL AVR) and question

New gcc port (ATMEL AVR) and question

[Denis Chertykov]

Hello !

I'm write the gcc port for ATMEL AVR at90s8515 micro controller.
at90s8515 have 512 bytes RAM and 8Kb embedded flash program memory.

My work pretty down (avr-gcc (with some hacks) can properly compile
vsprintf (I'm get it from Linux kernel) and fp-bit.c)
Are anybody interested ?

But I have a problem.

ATMEL AVR at90s8515 have 32 8bit registers (r0,r1,r2...r31).
The 26:27 28:29 30:31 registers pair can be 16bit pointers.
Pointer names:
26:27 - X
28:29 - Y
30:31 - Z

Y and Z pointers can be used with small displacement (<=63) but X not.
I'm define:
	POINTER_REGS - register class containing X,Y and Z.
	BASE_POINTER_REGS - register class containing Y and Z.
And then:
	#define BASE_REG_CLASS POINTER_REGS /* X,Y,Z */
	#define INDEX_REG_CLASS NO_REGS /* no index registers */

GO_IF_LEGITIMATE_ADDRESS macro in strict mode disable to using
X pointer with displacement -
(plus:HI (reg:HI 26 r26) (const_int any-number)) - not allowed.

movqi,movhi,movsi looks like:
(define_expand "movqi"
  [(set (match_operand:QI 0 "nonimmediate_operand" "")
	(match_operand:QI 1 "general_operand" ""))]
  ""
  "
{
  /* Don't generate memory->memory moves, go through a register */
  if (!(push_operand (operands[0],QImode)
	&& memory_operand (operands[1], QImode)))
    if ((reload_in_progress | reload_completed) == 0
        && GET_CODE (operands[0]) == MEM
        && (GET_CODE (operands[1]) == MEM || CONSTANT_P(operands[1])))
      operands[1] = force_reg(QImode, operands[1]);
 }")

(define_insn "*movqi_insn"
  [(set (match_operand:QI 0 "general_operand" "=r,!r,d,m,r")
	(match_operand:QI 1 "general_operand"  "r,!L,i,r,m"))]
  "(register_operand (operands[0],QImode)
    || register_operand (operands[1], QImode))"
  "@
	mov %0,%1
	clr %0
	ldi %0,low(%1)
	st%K0 %0,%1
	ld%K1 %0,%1"
  [(set_attr "length" "1,1,1,2,2")
   (set_attr "cc" "none,clobber,none,none,none")])

In the simple tests this work.
But in some cases (in difficult tests - `fp-bit.c') compiler produce
the pattern (inside the movM insn) like:
(mem:ANY-MODE (plus:HI (reg:HI 26 r26)
                       (const_int some-number-lower-then-63))
I'm confused. May be something wrong ?
IMHO: reload do this, but why ?

Denis.

Re: New gcc port (ATMEL AVR) and question

[Michael Hayes]

Denis Chertykov writes:
 > But in some cases (in difficult tests - `fp-bit.c') compiler produce
 > the pattern (inside the movM insn) like:
 > (mem:ANY-MODE (plus:HI (reg:HI 26 r26)
 >                        (const_int some-number-lower-then-63))
 > I'm confused. May be something wrong ?
 > IMHO: reload do this, but why ?

Have you always noticed this behaviour?  Which version of egcs
are you patching against?

I have been seeing similar problems with the c4x port where reload on
the second pass substitutes pseudo registers with locations on the
stack frame that have invalid displacements.  I'm still struggling
with a large testcase to find what triggers the problem.

Michael.

Re: New gcc port (ATMEL AVR) and question

[Denis Chertykov]

Hello !

> Denis Chertykov writes:
>  > But in some cases (in difficult tests - `fp-bit.c') compiler produce
>  > the pattern (inside the movM insn) like:
>  > (mem:ANY-MODE (plus:HI (reg:HI 26 r26)
>  >                        (const_int some-number-lower-then-63))
>  > I'm confused. May be something wrong ?
>  > IMHO: reload do this, but why ?
> 
> Have you always noticed this behaviour?
This depends from optimization level (-O3 produce wrong pattern more
frequently than -Os).
REG_ALLOC_ORDER and ORDER_REGS_FOR_LOCAL_ALLOC also influence to this.

> Which version of egcs are you patching against?

I'm patching egcs-1.1.1

Denis.

Re: New gcc port (ATMEL AVR) and question

[Jeffrey A Law]

  In message < m0zuMHO-000IfhC@medo.fov.uni-mb.si >you write:
  > 
  > GO_IF_LEGITIMATE_ADDRESS macro in strict mode disable to using
  > X pointer with displacement -
  > (plus:HI (reg:HI 26 r26) (const_int any-number)) - not allowed.
This is where you're going wrong.

You should have GO_IF_LEGITIMATE_ADDRESS return nonzero only for those
displacements which are always valid for each particular mode.

You can have special constraints which allow the larger displacements for
those register pairs which allow larger displacements.

Jeff

Re: New gcc port (ATMEL AVR) and question

[Nick Ing-Simmons]

Jeffrey A Law <law@cygnus.com> writes:
>  In message < m0zuMHO-000IfhC@medo.fov.uni-mb.si >you write:
>  > 
>  > GO_IF_LEGITIMATE_ADDRESS macro in strict mode disable to using
>  > X pointer with displacement -
>  > (plus:HI (reg:HI 26 r26) (const_int any-number)) - not allowed.
>This is where you're going wrong.
>
>You should have GO_IF_LEGITIMATE_ADDRESS return nonzero only for those
>displacements which are always valid for each particular mode.
>
>You can have special constraints which allow the larger displacements for
>those register pairs which allow larger displacements.

Hmm, easier said than done. When GO_IF_LEGITIMATE_ADDRESS is first called
the registers will be pseudos. If at that point the large offsets are 
permitted then allocate/reload must ensure that appropriate register
class gets used. On the other hand if large offsets are dis-allowed
then the index operation will be computed some other way
and put in a register. In either case it is most unlikely that "special 
constraints" will match.  You would need a CSE-after-reload to re-permute
the RTL.

I had similar issues trying to port gcc to a TI DSP where 
the subset of valid index registers depends on the (hard) reg used as
the base. In the end I gave up as it seemed impossible to allocate/reload
to an appropriate index register based on the associated base (or vice versa).

-- 
Nick Ing-Simmons <nik@tiuk.ti.com>
Via, but not speaking for: Texas Instruments Ltd.

Re: New gcc port (ATMEL AVR) and question

[Jeffrey A Law]

  In message < 199812301341.NAA29680@tiuk.ti.com >you write:
  > Hmm, easier said than done. When GO_IF_LEGITIMATE_ADDRESS is first called
  > the registers will be pseudos.
Right.  This is precisely why it must only accept displacemens that are always
valid.  ie, it must only accept the least common denominator addresses for each
mode.

That does not prevent one from having a constriaint letter that matches 
other address modes.  This is how the PA works.


jeff

Re: New gcc port (ATMEL AVR) and question

[Denis Chertykov]

Jeffrey A. Law writes:
> You should have GO_IF_LEGITIMATE_ADDRESS return nonzero only for those
> displacements which are always valid for each particular mode.
IMHO: in this case code quality will be low.
Code quality (size) is a very important for AVR because AVR is a
micro controller.
I'm resolve this problem (without rewriting GO_IF_LEGITIMATE_ADDRESS) as:
----------------------------------------------------------------------
(define_insn "*movqi_rm"
  [(set (match_operand:QI 0 "register_operand" "=r")
	(mem:QI (plus:HI (match_operand:HI 1 "register_operand" "b")
			 (match_operand:HI 2 "displacement_operand" "I"))))]
  ""
  "ldd %0,%a1+%2"
  [(set_attr "length" "1")])

(define_insn "*movqi_mr"
  [(set (mem:QI (plus:HI (match_operand:HI 0 "register_operand" "b")
			 (match_operand:HI 1 "displacement_operand" "I")))
	(match_operand:QI 2 "register_operand" "r"))]
  ""
  "std %a0+%1,%2"
  [(set_attr "length" "1")])


(define_insn "*movqi_insn"
  [(set (match_operand:QI 0 "general_operand" "=r,!r,d,m,r")
	(match_operand:QI 1 "general_operand"  "r,!L,i,r,m"))]
  "(register_operand (operands[0],QImode)
    || register_operand (operands[1], QImode))
   && !((GET_CODE (operands[0]) == MEM
         && GET_CODE (XEXP (operands[0],0)) == PLUS)
        || (GET_CODE (operands[1]) == MEM
            && GET_CODE (XEXP (operands[1],0)) == PLUS))"
  "@
	mov %0,%1
	clr %0
	ldi %0,low(%1)
	st%K0 %0,%1
	ld%K1 %0,%1"
  [(set_attr "length" "1,1,1,2,2")
   (set_attr "cc" "none,clobber,none,none,none")])
-------------------------------------------------------------------
Is this solution right ?
constraint `b' - register class which consists of
r28,r29,r30,r31 (Y and Z pointers) registers.
`I' - const_int >= 0 && <= 63

> You can have special constraints which allow the larger displacements for
> those register pairs which allow larger displacements.

-----------  gcc info fragment -------------------
`Q', `R', `S', ... `U'
     Letters in the range `Q' through `U' may be defined in a
     machine-dependent fashion to stand for arbitrary operand types.
     The machine description macro `EXTRA_CONSTRAINT' is passed the
     operand as its first argument and the constraint letter as its
     second operand.

     A typical use for this would be to distinguish certain types of
     memory references that affect other insn operands.

-->  Do not define these constraint letters to accept register
-->  references (`reg'); the reload pass does not expect this and would
-->  not handle it properly.
----------------- end fragment ---------------------
Conflict with info ?

Denis.

Re: New gcc port (ATMEL AVR) and question

[Michael Meissner]

On Fri, Jan 08, 1999 at 06:45:22PM +0000, Joern Rennecke wrote:
> > I noticed problems that were probably related to this with my efforts
> > for the i860 (had to stop in November).  It might not be a bed idea to
> > get reload to reset the insn id if it doesn't yet.
> 
> Indeed, it's not a bed idea, it's a bad idea.  reload has to be able to look
> at all alternatives of a mvoe insn, and if you split the insn into multiple
> patterns, this won't work.  If you want to go that route, you first have
> to devise some mechanism to group the patterns together and make reload
> handle the different patterns like one.

I disagree.  In theory, reload can do a general recog call just like any other
pass does.  It might slow things down a little, but it would allow better
tailoring of the move insns (after all, on most platforms, we already are out
of constraint letters, etc.).

-- 
Michael Meissner, Cygnus Solutions (Massachusetts office)
4th floor, 955 Massachusetts Avenue, Cambridge, MA 02139, USA
meissner@cygnus.com,	617-354-5416 (office),	617-354-7161 (fax)

Re: New gcc port (ATMEL AVR) and question

[Joern Rennecke]

> I noticed problems that were probably related to this with my efforts
> for the i860 (had to stop in November).  It might not be a bed idea to
> get reload to reset the insn id if it doesn't yet.

Indeed, it's not a bed idea, it's a bad idea.  reload has to be able to look
at all alternatives of a mvoe insn, and if you split the insn into multiple
patterns, this won't work.  If you want to go that route, you first have
to devise some mechanism to group the patterns together and make reload
handle the different patterns like one.

Re: New gcc port (ATMEL AVR) and question

[Joern Rennecke]

P.S.:  There was some talk before to make a new .md section to describe
the different forms of memory addresses.  Maybe that would be the best
solution for the problems you see.

Re: New gcc port (ATMEL AVR) and question

[Joern Rennecke]

> I disagree.  In theory, reload can do a general recog call just like any other
> pass does.  It might slow things down a little, but it would allow better
> tailoring of the move insns (after all, on most platforms, we already are out
> of constraint letters, etc.).

So how do you propose to handle cases when the rerecognition fails completely
because some operands still need reloading?

Re: New gcc port (ATMEL AVR) and question

[Michael Meissner]

On Thu, Dec 31, 1998 at 07:05:27PM +0300, Denis Chertykov wrote:
> Jeffrey A. Law writes:
> > You should have GO_IF_LEGITIMATE_ADDRESS return nonzero only for those
> > displacements which are always valid for each particular mode.
> IMHO: in this case code quality will be low.
> Code quality (size) is a very important for AVR because AVR is a
> micro controller.
> I'm resolve this problem (without rewriting GO_IF_LEGITIMATE_ADDRESS) as:
> ----------------------------------------------------------------------
> (define_insn "*movqi_rm"
>   [(set (match_operand:QI 0 "register_operand" "=r")
> 	(mem:QI (plus:HI (match_operand:HI 1 "register_operand" "b")
> 			 (match_operand:HI 2 "displacement_operand" "I"))))]
>   ""
>   "ldd %0,%a1+%2"
>   [(set_attr "length" "1")])
> 
> (define_insn "*movqi_mr"
>   [(set (mem:QI (plus:HI (match_operand:HI 0 "register_operand" "b")
> 			 (match_operand:HI 1 "displacement_operand" "I")))
> 	(match_operand:QI 2 "register_operand" "r"))]
>   ""
>   "std %a0+%1,%2"
>   [(set_attr "length" "1")])

In the past, it was not a good idea to have more than one pattern for moves,
due to misfeatures in reload (basically it would not reset the insn id # when
reload changed the move instruction, leaving later phases to call abort).
Whether or not it is safe now, I dunno, but I would err on the side of caution.

-- 
Michael Meissner, Cygnus Solutions (Massachusetts office)
4th floor, 955 Massachusetts Avenue, Cambridge, MA 02139, USA
meissner@cygnus.com,	617-354-5416 (office),	617-354-7161 (fax)

Re: New gcc port (ATMEL AVR) and question

[Bill Currie]

Michael Meissner wrote:
> 
> In the past, it was not a good idea to have more than one pattern for moves,
> due to misfeatures in reload (basically it would not reset the insn id # when
> reload changed the move instruction, leaving later phases to call abort).
> Whether or not it is safe now, I dunno, but I would err on the side of caution.

I noticed problems that were probably related to this with my efforts
for the i860 (had to stop in November).  It might not be a bed idea to
get reload to reset the insn id if it doesn't yet.

Bill
-- 
Leave others their otherness.