Bug 18251 - unable to find a register to spill in class `POINTER_REGS'
Summary: unable to find a register to spill in class `POINTER_REGS'
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 3.4.3
: P2 normal
Target Milestone: 4.3.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
: 19822 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-10-31 12:51 UTC by Ralf Corsepius
Modified: 2021-11-05 23:32 UTC (History)
11 users (show)

See Also:
Host:
Target: avr-*-*
Build:
Known to work: 3.4.2, 4.1.2
Known to fail: 3.4.3, 4.0.0, 4.2.0
Last reconfirmed: 2007-05-30 20:54:54


Attachments
example code to reproduce the breakdown. (636 bytes, text/plain)
2004-10-31 12:53 UTC, Ralf Corsepius
Details
alternative testcase (from ethernut, pr19822) (4.43 KB, text/plain)
2005-02-10 21:56 UTC, dieter meier
Details
Patch solves problem (1.50 KB, patch)
2005-02-12 20:19 UTC, andy hutchinson
Details | Diff
zipped patch (1.82 KB, patch)
2005-02-20 21:05 UTC, andy hutchinson
Details | Diff
Similar patch for gcc 3.4 (1.68 KB, patch)
2005-02-22 23:57 UTC, andy hutchinson
Details | Diff
Test case which causes ice-on-valid-code on 4.0-HEAD (355 bytes, text/plain)
2005-04-22 16:07 UTC, Stephan Eisvogel
Details
another test case (143 bytes, text/x-csrc)
2007-08-29 10:00 UTC, Sean D'Epagnier
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Corsepius 2004-10-31 12:51:18 UTC
Compiling the file from the attachment with -Os or greater -O1 results into the
compiler producing this:

# avr-rtems4.7-gcc -mmcu=avr5  -Os -g -Wall -c -o termios.o termios.c
termios.c: In function `termios_open':
termios.c:97: error: unable to find a register to spill in class `POINTER_REGS'
termios.c:97: error: this is the insn:
(insn 66 63 120 3 termios.c:91 (parallel [
            (set (mem:BLK (reg/v/f:HI 28 r28 [orig:44 tty ] [44]) [0 A8])
                (mem:BLK (reg/v/f:HI 45 [ callbacks ]) [0 A8]))
            (use (reg:QI 24 r24 [55]))
            (use (const_int 1 [0x1]))
            (clobber (scratch:HI))
            (clobber (scratch:HI))
            (clobber (scratch:QI))
        ]) 16 {*movstrqi_insn} (insn_list 62 (insn_list 63 (nil)))
    (expr_list:REG_DEAD (reg:QI 24 r24 [55])
        (expr_list:REG_DEAD (reg/v/f:HI 45 [ callbacks ])
            (expr_list:REG_UNUSED (scratch:QI)
                (expr_list:REG_UNUSED (scratch:HI)
                    (expr_list:REG_UNUSED (scratch:HI)
                        (nil)))))))
termios.c:97: confused by earlier errors, bailing out

avr-rtems* basically is identical to the bare avr, so I'd expect this bug to be
reproducable with all avr-* GCC's

# avr-rtems4.7-gcc --version
avr-rtems4.7-gcc (GCC) 3.4.3 20041027 (prerelease)
Comment 1 Ralf Corsepius 2004-10-31 12:53:36 UTC
Created attachment 7441 [details]
example code to reproduce the breakdown.

The example code is a stripped down version of a much larger real world piece
of code triggering this breakdown
Comment 2 Andrew Pinski 2004-10-31 14:47:03 UTC
Confirmed on the mainline.
Comment 3 Andrew Pinski 2005-02-08 13:55:34 UTC
*** Bug 19822 has been marked as a duplicate of this bug. ***
Comment 4 Björn Haase 2005-02-09 20:27:39 UTC
The bug seems still to be present in head 4.0.0. 
 
The failing example could be stripped down a little bit further: 
 
 
// Begin of sample code 
unsigned long semaphore_create (unsigned long name, 
				      unsigned long count, 
				      unsigned long attribute_set, 
				      unsigned long *id 
				      ); 
 
struct termios 
{ 
  unsigned int c_iflag; 
  unsigned int c_oflag; 
  unsigned int c_cflag; 
  unsigned int c_lflag; 
  unsigned char c_cc[19]; 
}; 
 
typedef struct termios_callbacks 
{ 
  int (*firstOpen) (int major, int minor, void *arg); 
  int (*lastClose) (int major, int minor, void *arg); 
  int (*pollRead) (int minor); 
} termios_callbacks; 
 
void *calloc (unsigned int __nmemb, unsigned int __size); 
 
struct termios_rawbuf 
{ 
  char *theBuf; 
  int Head; 
  int Tail; 
  int Size; 
  int Semaphore; 
}; 
 
struct termios_tty 
{ 
  struct termios_tty *forw; 
  struct termios_tty *back; 
 
  unsigned long major; 
  unsigned long minor; 
  unsigned long isem; 
  unsigned long osem; 
 
  char *cbuf; 
  int ccount; 
  int cindex; 
 
  int column; 
  int read_start_column; 
 
  struct termios termios; 
  struct termios_rawbuf rawInBuf; 
  struct termios_rawbuf rawOutBuf; 
 
  termios_callbacks device; 
  unsigned int flow_ctrl; 
  unsigned int lowwater, highwater; 
 
  unsigned int rxTaskId; 
  unsigned int txTaskId; 
 
  int tty_rcvwakeup; 
}; 
 
unsigned long 
termios_open (unsigned long major, 
		    unsigned long minor, 
		    struct termios_tty *tty, 
                     const termios_callbacks * callbacks) 
{ 
  unsigned long sc; 
 
    { 
      tty = calloc (1,0);  
      if (tty == ((void *) 0)) 
	  return 0; 
      tty->rawInBuf.Size = 128; 
 
      tty->minor = minor; 
      tty->major = major; 
      sc = semaphore_create (0, 1, 0, 0 ); 
      tty->device = *callbacks; 
 
      tty->lowwater = tty->rawInBuf.Size * 1 / 2; 
    } 
} 
// End of sample code 
 
 
 
Comment 5 Björn Haase 2005-02-09 23:14:15 UTC
Maybe this will help to localize the origin of the bug a bit better: 
 
I have made the observation that the presence of the bug strongly depends on 
the size of the memory footprint of the structs. Maybe there is a problem when 
the stack frame starts to grow beyond the 128 Byte boundary. 
 
Yours, 
 
Björn 
Comment 6 dieter meier 2005-02-10 21:56:53 UTC
Created attachment 8165 [details]
alternative testcase (from ethernut, pr19822)

This one compiles OK on 3.4.2 and fails on 3.4.3 and 4.0.0
Comment 7 andy hutchinson 2005-02-12 13:50:27 UTC
A sub-optimal fix is to disable movmemhi expansion. Either delete it or the less
draconian:
(define_expand "movmemhi"
  [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
		   (match_operand:BLK 1 "memory_operand" ""))
	      (use (match_operand:HI 2 "const_int_operand" ""))
	      (use (match_operand:HI 3 "const_int_operand" ""))
	      (clobber (match_scratch:HI 4 ""))
	      (clobber (match_scratch:HI 5 ""))
	      (clobber (match_dup 6))])]
  "(0)"
etc

A better solution is currently being tested.

Comment 8 andy hutchinson 2005-02-12 20:19:39 UTC
Created attachment 8188 [details]
Patch solves problem

This patch expands memmovhi into RTL memory move loop. Assocaited asm loop
pattern instruction no longer required. gcc does the same job and avoids reload
difficulties.

Fixes testcase.

Tested on avr testsuite by Björn Haase <bjoern.m.haase@web.de> with no
regressions.
Comment 9 Giovanni Bajo 2005-02-13 17:25:04 UTC
Andy, the patch needs a ChangeLog and needs to be posted to gcc-patches. I 
suggest you to explicitly CC the AVR maintainers when you post the patch. 
Comment 10 andy hutchinson 2005-02-20 21:05:19 UTC
Created attachment 8241 [details]
zipped patch

bzipped patch so that my email cant mess with it.
Otherwise same as before but cleaned up and a check made to avoid potential div
0
Comment 11 dieter meier 2005-02-22 10:32:43 UTC
Andy's patch works great for HEAD, but I get

patching file avr.md
Hunk #1 FAILED at 344.
1 out of 1 hunk FAILED -- saving rejects to file avr.md.rej

when patching 3_4 branch.
Comment 12 andy hutchinson 2005-02-22 12:31:01 UTC
Subject: Re:  unable to find a register to spill in class
 `POINTER_REGS'

if you can wait 12hrs I'll create 3.4 version.

Alternatively cut n  paste from a 4.0 avr.md
the change is local to one area.




dieterbmeier at yahoo dot com wrote:

>------- Additional Comments From dieterbmeier at yahoo dot com  2005-02-22 10:32 -------
>Andy's patch works great for HEAD, but I get
>
>patching file avr.md
>Hunk #1 FAILED at 344.
>1 out of 1 hunk FAILED -- saving rejects to file avr.md.rej
>
>when patching 3_4 branch.
>
>  
>



Comment 13 Eric Weddington 2005-02-22 13:19:50 UTC
Subject: Re:  unable to find a register to spill in class
 `POINTER_REGS'

dieterbmeier at yahoo dot com wrote:

>------- Additional Comments From dieterbmeier at yahoo dot com  2005-02-22 10:32 -------
>Andy's patch works great for HEAD, but I get
>
>patching file avr.md
>Hunk #1 FAILED at 344.
>1 out of 1 hunk FAILED -- saving rejects to file avr.md.rej
>
>when patching 3_4 branch.
>
>  
>
That's usually expected: a patch for HEAD does not guarantee that the 
patch will work for an earlier branch (3.4.x). Remember, this patch 
fixes a bug found on HEAD.
Comment 14 andy hutchinson 2005-02-22 23:57:06 UTC
Created attachment 8259 [details]
Similar patch for gcc 3.4

Patch uploaded for gcc 3.4

avr.md	24 Sep 2004 15:20:57 -0000	1.42.4.1
(as used in gcc 3.4 gcc-ss-3_4-20050121).

The patterns changed name between 3.4 and 4.0 - otherwise its the same patch as
proposed 4.0 patch.
Comment 15 dieter meier 2005-03-05 09:33:28 UTC
18251gcc34.patch.bz2 makes termios.c and arpcache.i to compile again on
3_4-20050305.
Thank you!

Can we have this patch in 3.4.4?
Comment 16 Joel Sherrill 2005-03-10 20:50:10 UTC
Any chance of this getting committed to the 3.4, 4.0 and 4.1 branches so it
can be closed?
Comment 17 Eric Weddington 2005-03-10 21:30:31 UTC
Marek, can you review this bug, the attached patches, and possibly approve
committing the fix?

Thanks
Eric
Comment 18 Marek Michalkiewicz 2005-03-12 20:39:02 UTC
(In reply to comment #17) 
> Marek, can you review this bug, the attached patches, and possibly approve 
> committing the fix? 
 
I'm looking into it right now.  I'm not sure about one thing: should "movmemhi" 
handle only constant block sizes, or variable block sizes too?  If variable - 
is it safe to assume nonzero?  (now 0 means 65536) 
 
Operand 2 (block size) has the "const_int_operand" predicate - doesn't this 
mean that (GET_CODE(operands[2]) == CONST_INT) is always true? 
 
Thanks, 
Marek 
 
Comment 19 andy hutchinson 2005-03-12 21:20:16 UTC
(In reply to comment #18)
> (In reply to comment #17) 

I think it is always true but the original used the same predicate and test (so
I played safe).

The pattern only helps if it is a constant.  I also thought it should handle
variable block size. However,  I found gcc already produces optimal code for
that case without any help.



> > Marek, can you review this bug, the attached patches, and possibly approve 
> > committing the fix? 
>  
> I'm looking into it right now.  I'm not sure about one thing: should "movmemhi" 
> handle only constant block sizes, or variable block sizes too?  If variable - 
> is it safe to assume nonzero?  (now 0 means 65536) 
>  
> Operand 2 (block size) has the "const_int_operand" predicate - doesn't this 
> mean that (GET_CODE(operands[2]) == CONST_INT) is always true? 
>  
> Thanks, 
> Marek 
>  

Comment 20 Marek Michalkiewicz 2005-03-13 00:30:50 UTC
Subject: Re:  unable to find a register to spill in class `POINTER_REGS'

On Sat, Mar 12, 2005 at 09:20:18PM -0000, andrewhutchinson at cox dot net wrote:

> The pattern only helps if it is a constant.  I also thought it should handle
> variable block size. However,  I found gcc already produces optimal code for
> that case without any help.

See below for revised patch (currently for mainline):
 - FAIL if count is not a CONST_INT
 - handle count == 0 (nothing to do)
 - handle count > 32767 (negative in RTL, mask with 0xffff)
 - minor formatting fixes

But, I'm still concerned a little about the variable block size:
 - __tmp_reg__ will not be used (some other register will)
 - more importantly, can the problem from this PR (unable to find a register
   to spill in class POINTER_REGS) still occur in the variable size case?
   (only with a different, not yet known test case - this means we are
   perhaps trying to hide the real bug instead of fixing it...)

If we have to handle the variable count case too, one more insn will
be needed (initially jump to decrementing the counter; test for carry
instead of zero).  Some other targets handle this by calling a subroutine
in libgcc.S - smaller (but slower) than generating the loop inline.

Marek


2005-03-12  Andy Hutchinson  <HutchinsonAndy@netscape.net>

	PR target/18251
	* config/avr/avr.md (movmemhi): Rewrite as RTL loop.
	(*movmemqi_insn): Delete.
	(*movmemhi): Delete.

Index: avr.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/avr/avr.md,v
retrieving revision 1.50
diff -c -3 -p -r1.50 avr.md
*** avr.md	6 Mar 2005 21:50:36 -0000	1.50
--- avr.md	12 Mar 2005 23:51:57 -0000
***************
*** 346,421 ****
  
  ;;=========================================================================
  ;; move string (like memcpy)
  
  (define_expand "movmemhi"
    [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
! 		   (match_operand:BLK 1 "memory_operand" ""))
! 	      (use (match_operand:HI 2 "const_int_operand" ""))
! 	      (use (match_operand:HI 3 "const_int_operand" ""))
! 	      (clobber (match_scratch:HI 4 ""))
! 	      (clobber (match_scratch:HI 5 ""))
! 	      (clobber (match_dup 6))])]
    ""
    "{
!   rtx addr0, addr1;
!   int cnt8;
    enum machine_mode mode;
  
    if (GET_CODE (operands[2]) != CONST_INT)
      FAIL;
-   cnt8 = byte_immediate_operand (operands[2], GET_MODE (operands[2]));
-   mode = cnt8 ? QImode : HImode;
-   operands[6] = gen_rtx_SCRATCH (mode);
-   operands[2] = copy_to_mode_reg (mode,
-                                   gen_int_mode (INTVAL (operands[2]), mode));
-   addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
-   addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
  
!   operands[0] = gen_rtx_MEM (BLKmode, addr0);
!   operands[1] = gen_rtx_MEM (BLKmode, addr1);
  }")
  
- (define_insn "*movmemqi_insn"
-   [(set (mem:BLK (match_operand:HI 0 "register_operand" "e"))
- 	(mem:BLK (match_operand:HI 1 "register_operand" "e")))
-    (use (match_operand:QI 2 "register_operand" "r"))
-    (use (match_operand:QI 3 "const_int_operand" "i"))
-    (clobber (match_scratch:HI 4 "=0"))
-    (clobber (match_scratch:HI 5 "=1"))
-    (clobber (match_scratch:QI 6 "=2"))]
-   ""
-   "ld __tmp_reg__,%a1+
- 	st %a0+,__tmp_reg__
- 	dec %2
- 	brne .-8"
-   [(set_attr "length" "4")
-    (set_attr "cc" "clobber")])
- 
- (define_insn "*movmemhi"
-   [(set (mem:BLK (match_operand:HI 0 "register_operand" "e,e"))
- 	(mem:BLK (match_operand:HI 1 "register_operand" "e,e")))
-    (use (match_operand:HI 2 "register_operand" "!w,d"))
-    (use (match_operand:HI 3 "const_int_operand" ""))
-    (clobber (match_scratch:HI 4 "=0,0"))
-    (clobber (match_scratch:HI 5 "=1,1"))
-    (clobber (match_scratch:HI 6 "=2,2"))]
-   ""
-   "*{
-      if (which_alternative==0)
-        return (AS2 (ld,__tmp_reg__,%a1+) CR_TAB
- 	       AS2 (st,%a0+,__tmp_reg__)  CR_TAB
- 	       AS2 (sbiw,%A2,1) CR_TAB
- 	       AS1 (brne,.-8));
-      else
-        return (AS2 (ld,__tmp_reg__,%a1+) CR_TAB
- 	       AS2 (st,%a0+,__tmp_reg__)  CR_TAB
- 	       AS2 (subi,%A2,1) CR_TAB
- 	       AS2 (sbci,%B2,0) CR_TAB
- 	       AS1 (brne,.-10));
- }"
-   [(set_attr "length" "4,5")
-    (set_attr "cc" "clobber,clobber")])
- 
  ;; =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0
  ;; memset (%0, 0, %1)
  
--- 346,414 ----
  
  ;;=========================================================================
  ;; move string (like memcpy)
+ ;; implement as RTL loop
  
  (define_expand "movmemhi"
    [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
!           (match_operand:BLK 1 "memory_operand" ""))
!           (use (match_operand:HI 2 "const_int_operand" ""))
!           (use (match_operand:HI 3 "const_int_operand" ""))])]
    ""
    "{
!   int prob, count;
    enum machine_mode mode;
+   rtx label = gen_label_rtx ();
+   rtx loop_reg;
+   rtx jump;
+ 
+   /* Copy pointers into new psuedos - they will be changed.  */
+   rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
+   rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
+ 
+   /* Create rtx for tmp register - we use this as scratch.  */
+   rtx tmp_reg_rtx  = gen_rtx_REG (QImode, TMP_REGNO);
  
    if (GET_CODE (operands[2]) != CONST_INT)
      FAIL;
  
!   count = INTVAL (operands[2]) & 0xffff;
!   if (count == 0)
!     DONE;
! 
!   /* Work out branch probability for latter use.  */
!   prob = REG_BR_PROB_BASE - REG_BR_PROB_BASE / count;
! 
!   /* See if constant fit 8 bits.  */
!   mode = (count < 0x100) ? QImode : HImode;
!   /* Create loop counter register.  */
!   loop_reg = copy_to_mode_reg (mode, gen_int_mode (count, mode));
! 
!   /* Now create RTL code for move loop.  */
!   /* Label at top of loop.  */
!   emit_label (label);
! 
!   /* Move one byte into scratch and inc pointer.  */
!   emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, addr1));
!   emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));
! 
!   /* Move to mem and inc pointer.  */
!   emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg_rtx);
!   emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));
! 
!   /* Decrement count.  */
!   emit_move_insn (loop_reg, gen_rtx_PLUS (mode, loop_reg, constm1_rtx));
! 
!   /* Compare with zero and jump if not equal. */
!   emit_cmp_and_jump_insns (loop_reg, const0_rtx, NE, NULL_RTX, mode, 1,
!                            label);
!   /* Set jump probability based on loop count.  */
!   jump = get_last_insn ();
!   REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB,
!                     GEN_INT (prob),
!                     REG_NOTES (jump));
!   DONE;
  }")
  
  ;; =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0
  ;; memset (%0, 0, %1)
  

Comment 21 andy hutchinson 2005-03-13 01:19:53 UTC
Subject: Re:  unable to find a register to spill in class
 `POINTER_REGS'

The concerns have merit but can be discounted:.

The reload problem occurs because the original pattern demands two 
pointers in "parrallel" existence.(Actually two pointers and a 
counter!)  The current register allocation is imperfect and with this 
constraint (and only 3 pointers incl frame)  fails to find a solution.

The new RTL expansion does not demand both pointer registers at the same 
time - indeed they could be the same register in extreme circumstances. 
Breaking up the RTL reveals this to GCC and allows the  register 
allocator to find a solution. So that is why it works.

The SAME result can also be realised by deleting the offending pattern - 
in this situation GCC generates it's own solution which happens to be 
identical RTL to the proposed solution (with a 16 bit counter). And 
indeed there is no reload failure.

Since the proposed pattern "FAILS", the variable case, we will still end 
up with GCC's solution and we can conclude there will be no hidden 
reload issue. (It should also be noted that a variable count is also 
less retrictive on hard register use than a constant).

Now here is the neat bit!. Since GCC middle end generates the detailed 
RTL loop, for a variable count,  we can and should rely on it to 
consider any restriction on the variable (ie variable count=0). If not, 
its very very broken.

I was very tempted to submit a patch that just deletes the pattern, 
however, that would produce worse code for the very common case where 
"fixed" count<255.

I hope this clarifies things.



marekm at amelek dot gda dot pl wrote:

>------- Additional Comments From marekm at amelek dot gda dot pl  2005-03-13 00:30 -------
>Subject: Re:  unable to find a register to spill in class `POINTER_REGS'
>
>On Sat, Mar 12, 2005 at 09:20:18PM -0000, andrewhutchinson at cox dot net wrote:
>
>  
>
>>The pattern only helps if it is a constant.  I also thought it should handle
>>variable block size. However,  I found gcc already produces optimal code for
>>that case without any help.
>>    
>>
>
>See below for revised patch (currently for mainline):
> - FAIL if count is not a CONST_INT
> - handle count == 0 (nothing to do)
> - handle count > 32767 (negative in RTL, mask with 0xffff)
> - minor formatting fixes
>
>But, I'm still concerned a little about the variable block size:
> - __tmp_reg__ will not be used (some other register will)
> - more importantly, can the problem from this PR (unable to find a register
>   to spill in class POINTER_REGS) still occur in the variable size case?
>   (only with a different, not yet known test case - this means we are
>   perhaps trying to hide the real bug instead of fixing it...)
>
>If we have to handle the variable count case too, one more insn will
>be needed (initially jump to decrementing the counter; test for carry
>instead of zero).  Some other targets handle this by calling a subroutine
>in libgcc.S - smaller (but slower) than generating the loop inline.
>
>Marek
>
>
>2005-03-12  Andy Hutchinson  <HutchinsonAndy@netscape.net>
>
>	PR target/18251
>	* config/avr/avr.md (movmemhi): Rewrite as RTL loop.
>	(*movmemqi_insn): Delete.
>	(*movmemhi): Delete.
>
>Index: avr.md
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/config/avr/avr.md,v
>retrieving revision 1.50
>diff -c -3 -p -r1.50 avr.md
>*** avr.md	6 Mar 2005 21:50:36 -0000	1.50
>--- avr.md	12 Mar 2005 23:51:57 -0000
>***************
>*** 346,421 ****
>  
>  ;;=========================================================================
>  ;; move string (like memcpy)
>  
>  (define_expand "movmemhi"
>    [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
>! 		   (match_operand:BLK 1 "memory_operand" ""))
>! 	      (use (match_operand:HI 2 "const_int_operand" ""))
>! 	      (use (match_operand:HI 3 "const_int_operand" ""))
>! 	      (clobber (match_scratch:HI 4 ""))
>! 	      (clobber (match_scratch:HI 5 ""))
>! 	      (clobber (match_dup 6))])]
>    ""
>    "{
>!   rtx addr0, addr1;
>!   int cnt8;
>    enum machine_mode mode;
>  
>    if (GET_CODE (operands[2]) != CONST_INT)
>      FAIL;
>-   cnt8 = byte_immediate_operand (operands[2], GET_MODE (operands[2]));
>-   mode = cnt8 ? QImode : HImode;
>-   operands[6] = gen_rtx_SCRATCH (mode);
>-   operands[2] = copy_to_mode_reg (mode,
>-                                   gen_int_mode (INTVAL (operands[2]), mode));
>-   addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
>-   addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
>  
>!   operands[0] = gen_rtx_MEM (BLKmode, addr0);
>!   operands[1] = gen_rtx_MEM (BLKmode, addr1);
>  }")
>  
>- (define_insn "*movmemqi_insn"
>-   [(set (mem:BLK (match_operand:HI 0 "register_operand" "e"))
>- 	(mem:BLK (match_operand:HI 1 "register_operand" "e")))
>-    (use (match_operand:QI 2 "register_operand" "r"))
>-    (use (match_operand:QI 3 "const_int_operand" "i"))
>-    (clobber (match_scratch:HI 4 "=0"))
>-    (clobber (match_scratch:HI 5 "=1"))
>-    (clobber (match_scratch:QI 6 "=2"))]
>-   ""
>-   "ld __tmp_reg__,%a1+
>- 	st %a0+,__tmp_reg__
>- 	dec %2
>- 	brne .-8"
>-   [(set_attr "length" "4")
>-    (set_attr "cc" "clobber")])
>- 
>- (define_insn "*movmemhi"
>-   [(set (mem:BLK (match_operand:HI 0 "register_operand" "e,e"))
>- 	(mem:BLK (match_operand:HI 1 "register_operand" "e,e")))
>-    (use (match_operand:HI 2 "register_operand" "!w,d"))
>-    (use (match_operand:HI 3 "const_int_operand" ""))
>-    (clobber (match_scratch:HI 4 "=0,0"))
>-    (clobber (match_scratch:HI 5 "=1,1"))
>-    (clobber (match_scratch:HI 6 "=2,2"))]
>-   ""
>-   "*{
>-      if (which_alternative==0)
>-        return (AS2 (ld,__tmp_reg__,%a1+) CR_TAB
>- 	       AS2 (st,%a0+,__tmp_reg__)  CR_TAB
>- 	       AS2 (sbiw,%A2,1) CR_TAB
>- 	       AS1 (brne,.-8));
>-      else
>-        return (AS2 (ld,__tmp_reg__,%a1+) CR_TAB
>- 	       AS2 (st,%a0+,__tmp_reg__)  CR_TAB
>- 	       AS2 (subi,%A2,1) CR_TAB
>- 	       AS2 (sbci,%B2,0) CR_TAB
>- 	       AS1 (brne,.-10));
>- }"
>-   [(set_attr "length" "4,5")
>-    (set_attr "cc" "clobber,clobber")])
>- 
>  ;; =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0
>  ;; memset (%0, 0, %1)
>  
>--- 346,414 ----
>  
>  ;;=========================================================================
>  ;; move string (like memcpy)
>+ ;; implement as RTL loop
>  
>  (define_expand "movmemhi"
>    [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
>!           (match_operand:BLK 1 "memory_operand" ""))
>!           (use (match_operand:HI 2 "const_int_operand" ""))
>!           (use (match_operand:HI 3 "const_int_operand" ""))])]
>    ""
>    "{
>!   int prob, count;
>    enum machine_mode mode;
>+   rtx label = gen_label_rtx ();
>+   rtx loop_reg;
>+   rtx jump;
>+ 
>+   /* Copy pointers into new psuedos - they will be changed.  */
>+   rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
>+   rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
>+ 
>+   /* Create rtx for tmp register - we use this as scratch.  */
>+   rtx tmp_reg_rtx  = gen_rtx_REG (QImode, TMP_REGNO);
>  
>    if (GET_CODE (operands[2]) != CONST_INT)
>      FAIL;
>  
>!   count = INTVAL (operands[2]) & 0xffff;
>!   if (count == 0)
>!     DONE;
>! 
>!   /* Work out branch probability for latter use.  */
>!   prob = REG_BR_PROB_BASE - REG_BR_PROB_BASE / count;
>! 
>!   /* See if constant fit 8 bits.  */
>!   mode = (count < 0x100) ? QImode : HImode;
>!   /* Create loop counter register.  */
>!   loop_reg = copy_to_mode_reg (mode, gen_int_mode (count, mode));
>! 
>!   /* Now create RTL code for move loop.  */
>!   /* Label at top of loop.  */
>!   emit_label (label);
>! 
>!   /* Move one byte into scratch and inc pointer.  */
>!   emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, addr1));
>!   emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));
>! 
>!   /* Move to mem and inc pointer.  */
>!   emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg_rtx);
>!   emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));
>! 
>!   /* Decrement count.  */
>!   emit_move_insn (loop_reg, gen_rtx_PLUS (mode, loop_reg, constm1_rtx));
>! 
>!   /* Compare with zero and jump if not equal. */
>!   emit_cmp_and_jump_insns (loop_reg, const0_rtx, NE, NULL_RTX, mode, 1,
>!                            label);
>!   /* Set jump probability based on loop count.  */
>!   jump = get_last_insn ();
>!   REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB,
>!                     GEN_INT (prob),
>!                     REG_NOTES (jump));
>!   DONE;
>  }")
>  
>  ;; =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0 =0
>  ;; memset (%0, 0, %1)
>  
>
>
>
>  
>


Comment 22 Paul Schlie 2005-03-13 02:06:53 UTC
(In reply to comment #20)

with reference to the most recent patch:
- anding with 0xFFFF may turn negatives positive so it seems wrong.
- there's no need limit byte counts to below 0x100 for bytes, as 0xFF is a
   count as long is it was originally verifed that the integer value is positive.

And just as a heads up, from when I was fooling a differnt varant, discovered
that (use (match_operand:HI 2 "const_int_operand" "")) apparently
also matches variable operands when compiling avrlibc:

Apparently failing as no code is generated:

../../../../libc/stdlib/realloc.c:154: error: unrecognizable insn:
(insn 235 232 236 31 ../../../../libc/stdlib/realloc.c:151 (parallel [
            (set (mem:BLK (reg/v/f:HI 49 [ memp ]) [0 A8])
                 (mem:BLK (reg/v/f:HI 60 [ ptr ]) [0 A8]))
            (use (reg:HI 81 [ <variable>.sz ]))
            (use (const_int 1 [0x1]))
        ]) -1 (insn_list:REG_DEP_TRUE 232 (nil))
    (expr_list:REG_DEAD (reg:HI 81 [ <variable>.sz ])
        (expr_list:REG_DEAD (reg/v/f:HI 49 [ memp ])
            (nil))))

From the following yet another version of Andy's patch:

(and for the hell of it, enclosed at the end, a version which
 attempts to handle variable counts, but couldn't figure out
 how to get the conditional insertion of a forward branch
 label generated correctly:)

- won't emit code unless (count > 0).

- removes code for non-constant count moves; as it would have
  generated incorrect code for move count <= 0.

- allocates a temporary, rather than presuming r0 is safe to use.
  (and seems to generate just as good code, as a step to freeing r0)

-- def --

;; move string (like memcpy)
;; implement as RTL loop

(define_expand "movmemhi"
  [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
           (match_operand:BLK 1 "memory_operand" ""))
          (use (match_operand:HI 2 "const_int_operand" ""))
          (use (match_operand:HI 3 "const_int_operand" ""))] )]
  ""
  "{
  int cnt8, prob;
  enum machine_mode mode;

  rtx loop_reg;
  rtx label = gen_label_rtx ();
  
  /* Copy pointers into new psuedos - they will be changed.  */
  rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
  rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
  
  /* If loop count is constant, try to use QImode counter.  */
  if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) > 0))
  {
    /* See if constant fit 8 bits.  */
    cnt8 = byte_immediate_operand (operands[2], GET_MODE (operands[2]));
    mode = cnt8 ? QImode : HImode;

    /* Create loop counter register.  */
    loop_reg = copy_to_mode_reg (mode, gen_int_mode (INTVAL (operands[2]),
                                                        mode));

    /* Create RTL code for move loop, with label at top of loop.  */
    emit_label (label);
 
    /* Move one byte into scratch and inc pointer.  */
    rtx tmp_reg = copy_to_mode_reg (QImode, gen_rtx_MEM (QImode, addr1));
    emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));
 
    /* Move scratch into mem, and inc other pointer.  */
    emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg);
    emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));
 
    /* Decrement count.  */
    emit_move_insn (loop_reg, gen_rtx_PLUS (mode, loop_reg, constm1_rtx));
 
    /* Compare with zero and jump if not equal.  */
    emit_cmp_and_jump_insns (loop_reg, const0_rtx, NE, NULL_RTX, mode, 1,
                             label);
  
    /* Set jump probability based on loop count.  */
    rtx jump = get_last_insn ();
    prob = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / INTVAL (operands[2]));
    REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB, GEN_INT (prob),
                                          REG_NOTES (jump));
    DONE;
  }}")

This time attempting to handle variable counts:

;; move string (like memcpy)
;; implement as RTL loop

(define_expand "movmemhi"
  [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
		   (match_operand:BLK 1 "memory_operand" ""))
	      (use (match_operand:HI 2 "const_int_operand" ""))
	      (use (match_operand:HI 3 "const_int_operand" ""))] )]
  ""
  "{
  enum machine_mode mode = HImode;
  int prob = (REG_BR_PROB_BASE * 95) / 100;
  rtx test_label = 0; /* Initial no-test value.  */
  
  /* Specify default variable loop count initial value.  */
  rtx loop_cnt = copy_to_mode_reg (mode, operands[2]);

  /* Only generate code for variable, or constant counts != 0.  */
  if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))
    goto done;

  if (GET_CODE (operands[2]) == CONST_INT) /* A constant count.  */
  {
    /* Adjust loop count mode size as a function of const size.  */
    if (byte_immediate_operand (operands[2], GET_MODE (operands[2])))
    {
      mode = QImode;
    }
    /* Adjust jump probability based on known loop count.  */
    prob = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / INTVAL (operands[2]));
    /* Update intermediate loop_cnt value based on its mode size.  */
    loop_cnt = copy_to_mode_reg (mode, gen_int_mode (INTVAL (operands[2]),
                                                     mode));
  }
  else /* A variable count.  */
  {
#if 0 /* FIXME, should jump to test_label if loop_cnt is a variable.  */
    /* Specify first jump to test_label to verify loop_cnt != 0.  */
    test_label = gen_label_rtx ();
    emit_jump_insn (test_label);
#endif
  }

  /* Specify memory operand pointer registers.  */
  rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
  rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));

  /* Specify RLT loop, beginning with loop_label.  */
  rtx loop_label = gen_label_rtx ();
  emit_label (loop_label);
  
  /* Specify move one byte into scratch and inc pointer.  */
  rtx tmp_reg = copy_to_mode_reg (QImode, gen_rtx_MEM (QImode, addr1));
  emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));
  
  /* Specify move scratch into mem, and inc other pointer.  */
  emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg);
  emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));
  
  /* Decrement count.  */
  emit_move_insn (loop_cnt, gen_rtx_PLUS (mode, loop_cnt, constm1_rtx));
  
#if 0 /* FIXME, should specify initial variable loop_cnt test_label.  */
  /* Specify test label, conditionally defined if loop_cnt = variable.  */
  if (test_label) emit_label (test_label); should work as test_label.
#endif

  /* Compare with zero and loop if not-equal.  */
  emit_cmp_and_jump_insns (loop_cnt, const0_rtx, NE, NULL_RTX, mode, 1,
                           loop_label);
  
  /* Set jump probability based on loop count.  */
  rtx jump = get_last_insn ();
  REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB, GEN_INT (prob),
                                        REG_NOTES (jump));
  
  DONE;
  done:
  }")
Comment 23 andy hutchinson 2005-03-13 02:44:50 UTC
Subject: Re:  unable to find a register to spill in class
 `POINTER_REGS'

This is a define EXPAND.  predicates (such as const_int_operand) and 
pattern have no effect at all on generated code or matching. This 
pattern always emits DONE or FAIL.

That is why you need to test operand in body.

And with oxffff is wrong. As is trying to handle the variable count 
case. That is fixing something that is not broke.

So looks like my patch is ok?




schlie at comcast dot net wrote:

>------- Additional Comments From schlie at comcast dot net  2005-03-13 02:06 -------
>(In reply to comment #20)
>
>with reference to the most recent patch:
>- anding with 0xFFFF may turn negatives positive so it seems wrong.
>- there's no need limit byte counts to below 0x100 for bytes, as 0xFF is a
>   count as long is it was originally verifed that the integer value is positive.
>
>And just as a heads up, from when I was fooling a differnt varant, discovered
>that (use (match_operand:HI 2 "const_int_operand" "")) apparently
>also matches variable operands when compiling avrlibc:
>
>Apparently failing as no code is generated:
>
>../../../../libc/stdlib/realloc.c:154: error: unrecognizable insn:
>(insn 235 232 236 31 ../../../../libc/stdlib/realloc.c:151 (parallel [
>            (set (mem:BLK (reg/v/f:HI 49 [ memp ]) [0 A8])
>                 (mem:BLK (reg/v/f:HI 60 [ ptr ]) [0 A8]))
>            (use (reg:HI 81 [ <variable>.sz ]))
>            (use (const_int 1 [0x1]))
>        ]) -1 (insn_list:REG_DEP_TRUE 232 (nil))
>    (expr_list:REG_DEAD (reg:HI 81 [ <variable>.sz ])
>        (expr_list:REG_DEAD (reg/v/f:HI 49 [ memp ])
>            (nil))))
>
>>From the following yet another version of Andy's patch:
>
>(and for the hell of it, enclosed at the end, a version which
> attempts to handle variable counts, but couldn't figure out
> how to get the conditional insertion of a forward branch
> label generated correctly:)
>
>- won't emit code unless (count > 0).
>
>- removes code for non-constant count moves; as it would have
>  generated incorrect code for move count <= 0.
>
>- allocates a temporary, rather than presuming r0 is safe to use.
>  (and seems to generate just as good code, as a step to freeing r0)
>
>-- def --
>
>;; move string (like memcpy)
>;; implement as RTL loop
>
>(define_expand "movmemhi"
>  [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
>           (match_operand:BLK 1 "memory_operand" ""))
>          (use (match_operand:HI 2 "const_int_operand" ""))
>          (use (match_operand:HI 3 "const_int_operand" ""))] )]
>  ""
>  "{
>  int cnt8, prob;
>  enum machine_mode mode;
>
>  rtx loop_reg;
>  rtx label = gen_label_rtx ();
>  
>  /* Copy pointers into new psuedos - they will be changed.  */
>  rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
>  rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
>  
>  /* If loop count is constant, try to use QImode counter.  */
>  if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) > 0))
>  {
>    /* See if constant fit 8 bits.  */
>    cnt8 = byte_immediate_operand (operands[2], GET_MODE (operands[2]));
>    mode = cnt8 ? QImode : HImode;
>
>    /* Create loop counter register.  */
>    loop_reg = copy_to_mode_reg (mode, gen_int_mode (INTVAL (operands[2]),
>                                                        mode));
>
>    /* Create RTL code for move loop, with label at top of loop.  */
>    emit_label (label);
> 
>    /* Move one byte into scratch and inc pointer.  */
>    rtx tmp_reg = copy_to_mode_reg (QImode, gen_rtx_MEM (QImode, addr1));
>    emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));
> 
>    /* Move scratch into mem, and inc other pointer.  */
>    emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg);
>    emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));
> 
>    /* Decrement count.  */
>    emit_move_insn (loop_reg, gen_rtx_PLUS (mode, loop_reg, constm1_rtx));
> 
>    /* Compare with zero and jump if not equal.  */
>    emit_cmp_and_jump_insns (loop_reg, const0_rtx, NE, NULL_RTX, mode, 1,
>                             label);
>  
>    /* Set jump probability based on loop count.  */
>    rtx jump = get_last_insn ();
>    prob = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / INTVAL (operands[2]));
>    REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB, GEN_INT (prob),
>                                          REG_NOTES (jump));
>    DONE;
>  }}")
>
>This time attempting to handle variable counts:
>
>;; move string (like memcpy)
>;; implement as RTL loop
>
>(define_expand "movmemhi"
>  [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
>		   (match_operand:BLK 1 "memory_operand" ""))
>	      (use (match_operand:HI 2 "const_int_operand" ""))
>	      (use (match_operand:HI 3 "const_int_operand" ""))] )]
>  ""
>  "{
>  enum machine_mode mode = HImode;
>  int prob = (REG_BR_PROB_BASE * 95) / 100;
>  rtx test_label = 0; /* Initial no-test value.  */
>  
>  /* Specify default variable loop count initial value.  */
>  rtx loop_cnt = copy_to_mode_reg (mode, operands[2]);
>
>  /* Only generate code for variable, or constant counts != 0.  */
>  if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))
>    goto done;
>
>  if (GET_CODE (operands[2]) == CONST_INT) /* A constant count.  */
>  {
>    /* Adjust loop count mode size as a function of const size.  */
>    if (byte_immediate_operand (operands[2], GET_MODE (operands[2])))
>    {
>      mode = QImode;
>    }
>    /* Adjust jump probability based on known loop count.  */
>    prob = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / INTVAL (operands[2]));
>    /* Update intermediate loop_cnt value based on its mode size.  */
>    loop_cnt = copy_to_mode_reg (mode, gen_int_mode (INTVAL (operands[2]),
>                                                     mode));
>  }
>  else /* A variable count.  */
>  {
>#if 0 /* FIXME, should jump to test_label if loop_cnt is a variable.  */
>    /* Specify first jump to test_label to verify loop_cnt != 0.  */
>    test_label = gen_label_rtx ();
>    emit_jump_insn (test_label);
>#endif
>  }
>
>  /* Specify memory operand pointer registers.  */
>  rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
>  rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));
>
>  /* Specify RLT loop, beginning with loop_label.  */
>  rtx loop_label = gen_label_rtx ();
>  emit_label (loop_label);
>  
>  /* Specify move one byte into scratch and inc pointer.  */
>  rtx tmp_reg = copy_to_mode_reg (QImode, gen_rtx_MEM (QImode, addr1));
>  emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));
>  
>  /* Specify move scratch into mem, and inc other pointer.  */
>  emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg);
>  emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));
>  
>  /* Decrement count.  */
>  emit_move_insn (loop_cnt, gen_rtx_PLUS (mode, loop_cnt, constm1_rtx));
>  
>#if 0 /* FIXME, should specify initial variable loop_cnt test_label.  */
>  /* Specify test label, conditionally defined if loop_cnt = variable.  */
>  if (test_label) emit_label (test_label); should work as test_label.
>#endif
>
>  /* Compare with zero and loop if not-equal.  */
>  emit_cmp_and_jump_insns (loop_cnt, const0_rtx, NE, NULL_RTX, mode, 1,
>                           loop_label);
>  
>  /* Set jump probability based on loop count.  */
>  rtx jump = get_last_insn ();
>  REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB, GEN_INT (prob),
>                                        REG_NOTES (jump));
>  
>  DONE;
>  done:
>  }")
>
>
>  
>


Comment 24 Paul Schlie 2005-03-13 03:39:40 UTC
(In reply to comment #23)
> This is a define EXPAND.  predicates (such as const_int_operand) and 
> pattern have no effect at all on generated code or matching. This 
> pattern always emits DONE or FAIL.
> 
> That is why you need to test operand in body.

- thanks, understood.

> And with oxffff is wrong. As is trying to handle the variable count 
> case. That is fixing something that is not broke.
> 
> So looks like my patch is ok?

- how about in the case of a variable count = 0 ?
  (or since only constants are handled, it falls back to letting gcc figure it out?)
Comment 25 andy hutchinson 2005-03-13 04:05:13 UTC
Subject: Re:  unable to find a register to spill in class
 `POINTER_REGS'

You answered your own question. GCC handles variable moves just like 
anything else. Dealing with range of possible values and size etc and 
construct appropriate RTL.

GCC does not need this backend  define or expand. It is quite happy 
working out moves by itself.

The pattern is *only* defined when the target can  do a better job - 
i.e.  when we have a constant  byte count - but not otherwise.  I have 
found it's a really bad idea to second guess compiler optimisations.

>- how about in the case of a variable count = 0 ?
>  (or since only constants are handled, it falls back to letting gcc figure it out?)
>
>
>  
>


Comment 26 Paul Schlie 2005-03-13 04:17:56 UTC
(In reply to comment #25)
> Subject: Re:  unable to find a register to spill in class
>  `POINTER_REGS'
> ...
> GCC does not need this backend  define or expand. It is quite happy 
> working out moves by itself.

- thanks again, sound's good.
Comment 27 GCC Commits 2005-03-13 10:10:17 UTC
Subject: Bug 18251

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	marekm@gcc.gnu.org	2005-03-13 10:09:56

Modified files:
	gcc            : ChangeLog 
	gcc/config/avr : avr.md 

Log message:
	PR target/18251
	* config/avr/avr.md (movmemhi): Rewrite as RTL loop.
	(*movmemqi_insn): Delete.
	(*movmemhi): Delete.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7811&r2=2.7812
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.md.diff?cvsroot=gcc&r1=1.50&r2=1.51

Comment 28 Marek Michalkiewicz 2005-03-13 10:10:52 UTC
Subject: Re:  unable to find a register to spill in class `POINTER_REGS'

On Sun, Mar 13, 2005 at 02:44:51AM -0000, andrewhutchinson at cox dot net wrote:

> And with oxffff is wrong. As is trying to handle the variable count 
> case. That is fixing something that is not broke.

Does this look OK?  Now "movmemhi" will only handle positive constant
counts, and FAIL for the other cases (zero, negative or variable) to
let GCC handle them by itself.

Hopefully this is the final version, committed to mainline - 4.0 and
3.4 will follow soon if there are no problems, please test.

Thanks,
Marek

;; move string (like memcpy)
;; implement as RTL loop

(define_expand "movmemhi"
  [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
          (match_operand:BLK 1 "memory_operand" ""))
          (use (match_operand:HI 2 "const_int_operand" ""))
          (use (match_operand:HI 3 "const_int_operand" ""))])]
  ""
  "{
  int prob;
  HOST_WIDE_INT count;
  enum machine_mode mode;
  rtx label = gen_label_rtx ();
  rtx loop_reg;
  rtx jump;

  /* Copy pointers into new psuedos - they will be changed.  */
  rtx addr0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0));
  rtx addr1 = copy_to_mode_reg (Pmode, XEXP (operands[1], 0));

  /* Create rtx for tmp register - we use this as scratch.  */
  rtx tmp_reg_rtx  = gen_rtx_REG (QImode, TMP_REGNO);

  if (GET_CODE (operands[2]) != CONST_INT)
    FAIL;

  count = INTVAL (operands[2]);
  if (count <= 0)
    FAIL;

  /* Work out branch probability for latter use.  */
  prob = REG_BR_PROB_BASE - REG_BR_PROB_BASE / count;

  /* See if constant fit 8 bits.  */
  mode = (count < 0x100) ? QImode : HImode;
  /* Create loop counter register.  */
  loop_reg = copy_to_mode_reg (mode, gen_int_mode (count, mode));

  /* Now create RTL code for move loop.  */
  /* Label at top of loop.  */
  emit_label (label);

  /* Move one byte into scratch and inc pointer.  */
  emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, addr1));
  emit_move_insn (addr1, gen_rtx_PLUS (Pmode, addr1, const1_rtx));

  /* Move to mem and inc pointer.  */
  emit_move_insn (gen_rtx_MEM (QImode, addr0), tmp_reg_rtx);
  emit_move_insn (addr0, gen_rtx_PLUS (Pmode, addr0, const1_rtx));

  /* Decrement count.  */
  emit_move_insn (loop_reg, gen_rtx_PLUS (mode, loop_reg, constm1_rtx));

  /* Compare with zero and jump if not equal. */
  emit_cmp_and_jump_insns (loop_reg, const0_rtx, NE, NULL_RTX, mode, 1,
                           label);
  /* Set jump probability based on loop count.  */
  jump = get_last_insn ();
  REG_NOTES (jump) = gen_rtx_EXPR_LIST (REG_BR_PROB,
                    GEN_INT (prob),
                    REG_NOTES (jump));
  DONE;
}")

Comment 29 GCC Commits 2005-03-19 15:44:35 UTC
Subject: Bug 18251

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	marekm@gcc.gnu.org	2005-03-19 15:44:26

Modified files:
	gcc            : ChangeLog 
	gcc/config/avr : avr.md 

Log message:
	PR target/18251
	* config/avr/avr.md (movmemhi): Rewrite as RTL loop.
	(*movmemqi_insn): Delete.
	(*movmemhi): Delete.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.62&r2=2.7592.2.63
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.md.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.49.8.1&r2=1.49.8.2

Comment 30 GCC Commits 2005-03-19 15:45:55 UTC
Subject: Bug 18251

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	marekm@gcc.gnu.org	2005-03-19 15:45:41

Modified files:
	gcc            : ChangeLog 
	gcc/config/avr : avr.md 

Log message:
	PR target/18251
	* config/avr/avr.md (movstrhi): Rewrite as RTL loop.
	(*movstrqi_insn): Delete.
	(*movstrhi): Delete.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.821&r2=2.2326.2.822
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/avr/avr.md.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.42.4.3&r2=1.42.4.4

Comment 31 Stephan Eisvogel 2005-04-22 16:07:46 UTC
Created attachment 8711 [details]
Test case which causes ice-on-valid-code on 4.0-HEAD
Comment 32 Stephan Eisvogel 2005-04-22 16:10:02 UTC
This bug is AFAICT not fully resolved in 4.0 HEAD as of today. The test case
compiles under 3.4.3, though. The code snippet used to be part of an MD5
transform function but I have stripped it down to the point where it is still
valid C code but does not do anything useful any more. In this state removing
any of the remaining lines usually prevents the ICE from occurring, most notably
the LPM macro (taken from avr-libc HEAD).

$ avr-gcc -Os -mmcu=atmega162 -c test.c
test.c: In function 'test':
test.c:46: error: unable to find a register to spill in class 'POINTER_REGS'
test.c:46: error: this is the insn:
(insn 18 115 20 0 (set (reg/v:SI 56 [ a ])
        (mem:SI (post_inc:HI (reg/f:HI 22 r22 [orig:59 D.1226 ] [59])) [2 S4
A8])) 14 {*movsi} (insn_list:REG_DEP_TRUE 115 (nil))
    (expr_list:REG_INC (reg/f:HI 22 r22 [orig:59 D.1226 ] [59])
        (nil)))
test.c:46: internal compiler error: in spill_failure, at reload1.c:1897
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

Compiler version:

$ avr-gcc -v
Using built-in specs.
Target: avr
Configured with: ../configure --target=avr --prefix=/home/eisvogel/avr
--enable-languages=c,c++ --disable-nls
Thread model: single
gcc version 4.1.0 20050422 (experimental)
Comment 33 berndtrog 2005-12-05 18:53:32 UTC
Compiling test.c with 4.1.0 20051202 or 4.2.0 20051202 works OK.

Compiling test.c with 4.0.3 20051123 still fails:

test.c: In function 'test':
test.c:46: error: unable to find a register to spill in class 'POINTER_REGS'
test.c:46: error: this is the insn:
(insn 18 116 20 0 (set (reg/v:SI 56 [ a ])
        (mem:SI (post_inc:HI (reg/f:HI 22 r22 [orig:59 D.1125 ] [59])) [2 S4 A8])) 14 {*movsi} (insn_list:REG_DEP_TRUE 116 (nil))
    (expr_list:REG_INC (reg/f:HI 22 r22 [orig:59 D.1125 ] [59])
        (nil)))
test.c:46: confused by earlier errors, bailing out
Comment 34 Joerg Wunsch 2007-04-29 06:59:56 UTC
Works with GCC 4.1.2.
Comment 35 Ralf Corsepius 2007-05-02 14:07:33 UTC
Now, bootstrapping 4.2.0 fails with a similar error

/builddir/build/BUILD/rtems-4.8-avr-rtems4.8-gcc-4.2.0/build/./gcc/xgcc -B/builddir/build/BUILD/rtems-4.8-avr-rtems4.8-gcc-4.2.0/b
../../../../../../gcc-4.2.0-20070430/newlib/libc/misc/init.c: In function '__libc_fini_array':
../../../../../../gcc-4.2.0-20070430/newlib/libc/misc/init.c:59: error: unable to find a register to spill in class 'BASE_POINTER_
../../../../../../gcc-4.2.0-20070430/newlib/libc/misc/init.c:59: error: this is the insn:
(insn 84 55 56 4 ../../../../../../gcc-4.2.0-20070430/newlib/libc/misc/init.c:56 (set (mem/c:HI (plus:HI (reg/f:HI 28 r28)
                (const_int 1 [0x1])) [5 S2 A8])
        (reg:HI 24 r24)) 12 {*movhi} (nil)
    (nil))
../../../../../../gcc-4.2.0-20070430/newlib/libc/misc/init.c:59: confused by earlier errors, bailing out
Comment 36 Ralf Corsepius 2007-05-02 14:16:30 UTC
(In reply to comment #35)
> Now, bootstrapping 4.2.0 fails with a similar error
Filed as http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31786
Comment 37 Eric Weddington 2007-05-30 20:54:54 UTC
test.c fails in 4.2.0 for -Os only, -O[0123] works.
test.c succeeds for 4.3-20070525 for all -O[0123s].
Comment 38 Eric Weddington 2007-05-30 20:59:33 UTC
termios.c successful for 4.2.0, 4.3-20070525, for all -Ox.
arpcache.i successful for 4.2.0, 4.3-20070525, for all -Ox.
Comment 39 Sean D'Epagnier 2007-08-29 10:00:59 UTC
Created attachment 14131 [details]
another test case

This test case only has problems when gcc is invoked with the -ftree-pre flag which is part of -O2 by default.
Comment 40 Eric Weddington 2007-08-29 18:06:40 UTC
(In reply to comment #39)
> Created an attachment (id=14131) [edit]
> another test case
> 
> This test case only has problems when gcc is invoked with the -ftree-pre flag
> which is part of -O2 by default.
> 

This is not a good test case as "float *a" is being used uninitialized. Please provide a better test case. FWIW, this test case works for me on:
4.2.1 -O[0123s]
4.3.0 20070824 snapshot -O[0123s]
4.1.2 (WinAVR 20070525) -O[0123s]

Please provide which version of GCC, and which target that you are using to make it fail, including how GCC is configured. I would also suggest opening up a new bug report for this until it is determined that this is the same bug.
Comment 41 Richard Biener 2008-03-14 16:39:01 UTC
Fixed for 4.3.0, please open a new bugreport for further issues.
Comment 42 Jackie Rosen 2014-02-16 13:14:03 UTC Comment hidden (spam)
Comment 43 Tim Turner 2021-11-05 23:18:39 UTC Comment hidden (spam)