[Bug bootstrap/13722] New: [3.4/3.5 regression] [ia64] Ada bootstrap failure

[Bug bootstrap/13722] New: [3.4/3.5 regression] [ia64] Ada bootstrap failure

[schwab at suse dot de]

Triggered by this change: 
 
2004-01-15  Zack Weinberg  <zack@codesourcery.com> 
 
	* config/ia64/ia64.md (*movti_internal): C output template 
	extracted to ia64.c. 
	(*movti_internal_reg): Delete. 
	(reload_inti, reload_outti): Use the correct mode on operand 2 
	in the first place, don't fix it up in the output template. 
	(movtf, reload_ointf, reload_outtf): New expanders. 
	(*movtf_internal): New define_insn_and_split. 
	* config/ia64/ia64.c (ia64_split_timode): Rename to ia64_split_tmode; 
	make static; do not hand TFmode CONST_DOUBLEs to split_double. 
	(ia64_split_tmode_move): New function, body mostly pulled 
	from ia64.md:*movti_internal. 
	(ia64_function_arg_words): New function, extracted common 
	logic from ia64_function_arg et seq. 
	(ia64_function_arg_offset): Likewise.  Handle correctly the 
	case of a scalar quantity 16 bytes wide with only 8-byte alignment. 
	(ia64_function_arg, ia64_function_arg_partial_nregs) 
	(ia64_function_arg_advance): Use ia64_function_arg_words and 
	ia64_function_arg_offset. 
	(ia64_function_value): TCmode does not go in float regs. 
	(ia64_secondary_reload_class): Also handle TFmode. 
	* config/ia64/ia64-protos.h: Remove prototype for 
	ia64_split_timode; add prototype for ia64_split_tmode_move. 
 
stage1/xgcc -Bstage1/ -B/usr/local/ia64-suse-linux/bin/ -c -g -O2      -gnatpg 
-gnata -I- -I. -Iada -I../../gcc/ada ../../gcc/ada/a-elchha.adb -o 
ada/a-elchha.o 
+===========================GNAT BUG DETECTED==============================+ 
| 3.5.0 20040117 (experimental) (ia64-suse-linux-gnu) GCC error:           | 
| in push_secondary_reload, at reload.c:427                                | 
| Error detected at a-elchha.adb:104:8                                     | 
 
(gdb) r 
Starting program: /tmp/cvs/gcc-test-2004011604/Build/gcc/stage1/gnat1 -I- -I. 
-Iada -I../../gcc/ada -quiet -dumpbase a-elchha.adb -g -gnatpg -gnata -O2 
-gnatO ada/a-elchha.o ../../gcc/ada/a-elchha.adb 
 
Breakpoint 1, fancy_abort (file=0x40000000013999a0 "../../gcc/reload.c",  
    line=427, function=0x40000000013999b8 "push_secondary_reload") 
    at ../../gcc/diagnostic.c:584 
584       internal_error ("in %s, at %s:%d", function, trim_filename (file), 
line); 
(gdb) up 
#1  0x4000000000e06a70 in push_secondary_reload (in_p=1, x=0x2000000000582da0,  
    opnum=1, optional=0, reload_class=GR_REGS, reload_mode=TImode,  
    type=RELOAD_FOR_INPUT_ADDRESS, picode=0x60000fffffff6ed8) 
    at ../../gcc/reload.c:427 
427         abort (); 
(gdb) l 
422          Allow this when a reload_in/out pattern is being used.  I.e. 
assume 
423          that the generated code handles this case.  */ 
424 
425       if (in_p && class == reload_class && icode == CODE_FOR_nothing 
426           && t_icode == CODE_FOR_nothing) 
427         abort (); 
428 
429       /* If we need a tertiary reload, see if we have one we can reuse or 
else 
430          make a new one.  */ 
431 
(gdb) p class 
$1 = GR_REGS 
(gdb) p x 
$2 = 0x2000000000582da0 
(gdb) pr 
(mem:TI (plus:DI (reg/f:DI 12 r12) 
        (const_int 736 [0x2e0])) [17 buff+0 S16 A128])

-- 
           Summary: [3.4/3.5 regression] [ia64] Ada bootstrap failure
           Product: gcc
           Version: 3.4.0
            Status: UNCONFIRMED
          Keywords: build
          Severity: normal
          Priority: P2
         Component: bootstrap
        AssignedTo: zack at codesourcery dot com
        ReportedBy: schwab at suse dot de
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: ia64-linux
  GCC host triplet: ia64-linux
GCC target triplet: ia64-linux


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical
            Summary|[3.4/3.5 regression] [ia64] |[3.4/3.5 regression] [ia64]
                   |Ada bootstrap failure       |Ada bootstrap failure
   Target Milestone|---                         |3.4.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-18 02:13 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap
 failure


I cannot debug Ada problems on ia64.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[charlet at act-europe dot fr]

------- Additional Comments From charlet at act-europe dot fr  2004-01-18 10:50 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap failure

> I cannot debug Ada problems on ia64.

Why ? A regular gdb understanding C can be used to debug Ada.
Alternatively you can use http://libre.act-europe.fr/GDB if an Ada-aware
gdb is really needed (for low level debugging of gdb, it usually isn't)

Let me know if you need to help (e.g. don't hesitate to send me
private emails) to get you started.

Arno


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-18 19:44 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap
 failure


I can't even *build* the ada compiler on ia64.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[charlet at act-europe dot fr]

------- Additional Comments From charlet at act-europe dot fr  2004-01-18 21:31 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap failure

> I can't even *build* the ada compiler on ia64.

Well yes, I guess this is what this PR is about, no ? :-)
You should still be able to debug the stage1/gnat1 that is failing during the
bootstrap.

Or ask Andreas about his set up if you can't get to this point (you didn't
give a clue as to why you can't build, so we're really playing
guessing games here).

Arno


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-18 21:42 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap
 failure


The only ia64 platform I have convenient access to is ia64-hpux11.x
(two point releases, which are not noticeably different).  I do not
have any Ada compiler for this target, therefore I cannot get as far
as building stage1/gnat1.  For all I know it isn't even supported.

I really need someone else to try to debug this.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[charlet at act-europe dot fr]

------- Additional Comments From charlet at act-europe dot fr  2004-01-18 21:48 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap failure

> The only ia64 platform I have convenient access to is ia64-hpux11.x
> (two point releases, which are not noticeably different).  I do not
> have any Ada compiler for this target, therefore I cannot get as far
> as building stage1/gnat1.  For all I know it isn't even supported.

You should be able to build a ia64-hpux Ada compiler (with no tasking, but
that doesn't matter for bootstrap issues).

Of course I have no idea whether the problem will show on this config.

Arno


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-18 22:29 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap
 failure


How do I build an ia64-hpux Ada compiler if I don't have one to begin
with?

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[charlet at act-europe dot fr]

------- Additional Comments From charlet at act-europe dot fr  2004-01-18 22:31 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap failure

> How do I build an ia64-hpux Ada compiler if I don't have one to begin
> with?

By building a cross compiler.

Arno


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-18 22:33 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap
 failure


I think it'll be easier for everyone involved if someone who already
sees this problem will debug it a bit more.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[charlet at act-europe dot fr]

------- Additional Comments From charlet at act-europe dot fr  2004-01-18 22:34 -------
Subject: Re:  [3.4/3.5 regression] [ia64] Ada bootstrap failure

> I think it'll be easier for everyone involved if someone who already
> sees this problem will debug it a bit more.

Yes, I agree this makes more sense for the time being.

Arno


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[debian-gcc at lists dot debian dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |debian-gcc at lists dot
                   |                            |debian dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-20 14:12 -------
Created an attachment (id=5535)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=5535&action=view)
Debugging dumps


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug bootstrap/13722] [3.4/3.5 regression] [ia64] Ada bootstrap failure

[rakdver at gcc dot gnu dot org]

------- Additional Comments From rakdver at gcc dot gnu dot org  2004-01-21 13:16 -------
Created an attachment (id=5543)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=5543&action=view)
C testcase for the problem

The bug can also be reproduced with this c testcase:

./cc1 tc.i

 ...
 init_iconv_desc

cppcharset.c: In function `init_iconv_desc':
cppcharset.c:704: internal compiler error: in push_secondary_reload, at
reload.c:427


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|bootstrap                   |target
     Ever Confirmed|                            |1
           Keywords|                            |ice-on-valid-code
   Last reconfirmed|0000-00-00 00:00:00         |2004-01-21 14:13:11
               date|                            |
            Summary|[3.4/3.5 regression] [ia64] |[3.4/3.5 regression] [ia64]
                   |Ada bootstrap failure       |ICE in push_secondary_reload


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[dhazeghi at yahoo dot com]

------- Additional Comments From dhazeghi at yahoo dot com  2004-01-21 16:53 -------
Reduces to:
struct cset_converter
{
  void * func;
  char cd;
};

extern void cpp_error (void *, int, char *,...);

static struct cset_converter
init_iconv_desc (void *to)
{
  struct cset_converter ret;

  if(to) return ret;

  cpp_error (to, 0x03, "foo");

     return ret;
}

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-22 02:13 -------
Subject: Re:  [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

See the MIME attachment.
I see Zack has left yet another IA-64 turd for me to clean up.  Sigh.

The main problem with Zack's patch is that he replaced a null predicate in the
reload_{in,out}* patterns with a memory_operand predicate.  This doesn't work.
push_secondary_reloads tries to verify that the operands are OK for the
secondary reload pattern.  However, for a MEM, this means that we are passing
an operand to the predicate that may appear to be invalid, but is valid
because it has already been reloaded.  memory_operand doesn't know this, and
returns false.  The IA-64 port deliberately used a null predicate to make this
work.  I see the Alpha port defines a special any_memory_operand predicate for
this case.  I blatantly copied it from the Alpha port.

By the way, the Alpha PREDICATE_CODES seems to be broken, because it says
any_memory_operand only accepts MEM, but it also accepts SUBREG.

This is enough to make the small C example work.

However, I think there are other problems.  Zack "fixed" the secondary reload
patterns to use DImode instead of TImode for operand2.  Anyone who knows how
these patterns work knows that you have to allocate 2 registers just in case
the scratch operand overlaps one of the other operands.  Practically all ports
define the patterns this way.  If this case happens, we can hit the abort in
ia64_split_tmode_move.  The original code was trying to avoid this problem.
It was doing it rather inexpertly, and perhaps incorretly, but the general
idea here was right.  Not surprisingly, this problem turns up during an Ada
build.  On multiple files.  I haven't written a patch to fix this yet.  This
will take a bit of thought, because we need to handle overlap both with the
input operand and the output operand.  We might actually need to use OImode
to make this safe.  Or maybe we can fix this by changing ia64_split_tmode
to decide based on overlaps whether it should use the scratch register for
out[0] or out[1].  The example I looked at had an insn
  [(set (reg:TI r14) (mem:TI (reg:DI r15)))
   (clobber (reg:DI r14))]
which resulted in
  (set (reg:DI r15) (plus:DI (reg:DI r14) (const_int 8)))
  (set (reg:DI r14) (mem:DI (reg:DI r15)))
  (set (reg:DI r15) (mem:DI (reg:DI r14)))
and ia64_split_tmode_move aborts because this fails no matter which order we
try to use for the last two instructions.  If we were smarter about the input
addresses, we could have copied r14 to r15, put the sum in r14, and then we
would have
  (set (reg:DI r15) (reg:DI r14))
  (set (reg:DI r14) (plus:DI (reg:DI r14) (const_int 8)))
  (set (reg:DI r14) (mem:DI (reg:DI r14)))
  (set (reg:DI r15) (mem:DI (reg:DI r15)))
which does work.  This is one instruction longer, but may be rare enough that
it isn't a problem.

I haven't looked at the rest of the patch in detail yet, so there could be
other problems too.
	
2004-01-21  James E Wilson  <wilson@specifixinc.com>

	* config/ia64/ia64.c (any_memory_operand): New.
	* config/ia64/ia64.h (PREDICATE_CODES): Add any_memory_operand.
	* config/ia64/ia64.md (reload_inti, reload_outti, reload_intf,
	reload_outtf): Use any_memory_operand.

Index: ia64.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/ia64/ia64.c,v
retrieving revision 1.265
diff -p -r1.265 ia64.c
*** ia64.c	16 Jan 2004 01:27:37 -0000	1.265
--- ia64.c	22 Jan 2004 00:28:08 -0000
*************** basereg_operand (rtx op, enum machine_mo
*** 961,966 ****
--- 961,980 ----
    return (register_operand (op, mode) &&
  	  REG_POINTER ((GET_CODE (op) == SUBREG) ? SUBREG_REG (op) : op));
  }
+ 
+ /* Return 1 if OP is any memory location.  During reload a pseudo matches.  */
+ 
+ int
+ any_memory_operand (rtx op, enum machine_mode mode ATTRIBUTE_UNUSED)
+ {
+   return (GET_CODE (op) == MEM
+ 	  || (GET_CODE (op) == SUBREG && GET_CODE (SUBREG_REG (op)) == REG)
+ 	  || (reload_in_progress && GET_CODE (op) == REG
+ 	      && REGNO (op) >= FIRST_PSEUDO_REGISTER)
+ 	  || (reload_in_progress && GET_CODE (op) == SUBREG
+ 	      && GET_CODE (SUBREG_REG (op)) == REG
+ 	      && REGNO (SUBREG_REG (op)) >= FIRST_PSEUDO_REGISTER));
+ }
  \f
  typedef enum
    {
Index: ia64.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/ia64/ia64.h,v
retrieving revision 1.163
diff -p -r1.163 ia64.h
*** ia64.h	6 Dec 2003 05:40:14 -0000	1.163
--- ia64.h	22 Jan 2004 00:28:11 -0000
*************** do {									\
*** 2249,2255 ****
  { "general_xfmode_operand", {SUBREG, REG, CONST_DOUBLE, MEM}},		\
  { "destination_xfmode_operand", {SUBREG, REG, MEM}},			\
  { "xfreg_or_fp01_operand", {REG, CONST_DOUBLE}},			\
! { "basereg_operand", {SUBREG, REG}},
  
  /* An alias for a machine mode name.  This is the machine mode that elements of
     a jump-table should have.  */
--- 2249,2256 ----
  { "general_xfmode_operand", {SUBREG, REG, CONST_DOUBLE, MEM}},		\
  { "destination_xfmode_operand", {SUBREG, REG, MEM}},			\
  { "xfreg_or_fp01_operand", {REG, CONST_DOUBLE}},			\
! { "basereg_operand", {SUBREG, REG}},					\
! { "any_memory_operand", {SUBREG, MEM}},
  
  /* An alias for a machine mode name.  This is the machine mode that elements of
     a jump-table should have.  */
Index: ia64.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/ia64/ia64.md,v
retrieving revision 1.119
diff -p -r1.119 ia64.md
*** ia64.md	16 Jan 2004 01:27:38 -0000	1.119
--- ia64.md	22 Jan 2004 00:28:17 -0000
***************
*** 614,626 ****
  
  (define_expand "reload_inti"
    [(parallel [(set (match_operand:TI 0 "register_operand" "=r")
! 		   (match_operand:TI 1 "memory_operand" "m"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
    "")
  
  (define_expand "reload_outti"
!   [(parallel [(set (match_operand:TI 0 "memory_operand" "=m")
  		   (match_operand:TI 1 "register_operand" "r"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
--- 614,626 ----
  
  (define_expand "reload_inti"
    [(parallel [(set (match_operand:TI 0 "register_operand" "=r")
! 		   (match_operand:TI 1 "any_memory_operand" "m"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
    "")
  
  (define_expand "reload_outti"
!   [(parallel [(set (match_operand:TI 0 "any_memory_operand" "=m")
  		   (match_operand:TI 1 "register_operand" "r"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
***************
*** 796,808 ****
  
  (define_expand "reload_intf"
    [(parallel [(set (match_operand:TF 0 "register_operand" "=r")
! 		   (match_operand:TF 1 "memory_operand" "m"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
    "")
  
  (define_expand "reload_outtf"
!   [(parallel [(set (match_operand:TF 0 "memory_operand" "=m")
  		   (match_operand:TF 1 "register_operand" "r"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
--- 796,808 ----
  
  (define_expand "reload_intf"
    [(parallel [(set (match_operand:TF 0 "register_operand" "=r")
! 		   (match_operand:TF 1 "any_memory_operand" "m"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""
    "")
  
  (define_expand "reload_outtf"
!   [(parallel [(set (match_operand:TF 0 "any_memory_operand" "=m")
  		   (match_operand:TF 1 "register_operand" "r"))
  	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
    ""


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-22 02:26 -------
Subject: Re:  [3.4/3.5 regression] [ia64] ICE in
 push_secondary_reload

"wilson at specifixinc dot com" <gcc-bugzilla@gcc.gnu.org> writes:

> I see Zack has left yet another IA-64 turd for me to clean up.  Sigh.

... excuse me for having bigger problems on my plate right now.  Not
to mention that none of this is reproducible on any target I can
conveniently test on.

> The main problem with Zack's patch is that he replaced a null
> predicate in the reload_{in,out}* patterns with a memory_operand
> predicate.  This doesn't work.  push_secondary_reloads tries to
> verify that the operands are OK for the secondary reload pattern.
> However, for a MEM, this means that we are passing an operand to the
> predicate that may appear to be invalid, but is valid because it has
> already been reloaded.

I ... see.

> However, I think there are other problems.  Zack "fixed" the
> secondary reload patterns to use DImode instead of TImode for
> operand2.  Anyone who knows how these patterns work knows that you
> have to allocate 2 registers just in case the scratch operand
> overlaps one of the other operands.  Practically all ports define
> the patterns this way.

This is insane.  Clearly the bug is that overlapping operands can
occur in the first place.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-22 05:21 -------
Subject: Re:  [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

zack at codesourcery dot com wrote:
> ... excuse me for having bigger problems on my plate right now.  Not
> to mention that none of this is reproducible on any target I can
> conveniently test on.

The simple fact is that you have broken the IA-64 compiler a number of 
times, and you have not always been responsive to fixing the problems 
you have caused.  This is very annoying.  There are many people using 
the IA-64 port, and you inconvience them everytime you break it.

I have important work to do also, but instead I am forced to do your 
work for you, and I am not happy about this.

If you aren't going to try to do IA-64 work properly, then you could at 
least post patches for review instead of just blindly checking them in 
and waiting to see what breaks.  Or you could voluntarily revert patches 
when it is proven that they don't work.  This is much more friendly to 
the rest of us, particularly those of us using the IA-64 gcc port for 
real work.

debian linux is and always has been free.  If you have an IA-64 machine, 
it should be easy to install debian on it.

>>However, for a MEM, this means that we are passing an operand to the
>>predicate that may appear to be invalid, but is valid because it has
>>already been reloaded.

Maybe the issue is that it hasn't been reloaded yet?  In any case, we 
are in the middle of reload, so we are guaranteed that a MEM is going to 
be OK whether it looks OK or not.

> This is insane.  Clearly the bug is that overlapping operands can
> occur in the first place.

It has always worked this way.  I know this is lame, but one should 
never underestimate the difficulty of trying to change how reload works. 
  It is much easier to change all md file than to change reload.  I 
think there is an important reason why it works this way, but it 
probably isn't possible to figure it out without spending an 
unreasonable amount of time messing with the code.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-22 08:22 -------
Subject: Re:  [3.4/3.5 regression] [ia64] ICE in
 push_secondary_reload

"wilson at specifixinc dot com" <gcc-bugzilla@gcc.gnu.org> writes:

> The simple fact is that you have broken the IA-64 compiler a number of 
> times, and you have not always been responsive to fixing the problems 
> you have caused.  This is very annoying.  There are many people using 
> the IA-64 port, and you inconvience them everytime you break it.
>
> I have important work to do also, but instead I am forced to do your 
> work for you, and I am not happy about this.
>
> If you aren't going to try to do IA-64 work properly, then you could at 
> least post patches for review instead of just blindly checking them in 
> and waiting to see what breaks.  Or you could voluntarily revert patches 
> when it is proven that they don't work.  This is much more friendly to 
> the rest of us, particularly those of us using the IA-64 gcc port for 
> real work.

The fact of the matter is that I *have* always tested these changes on
an ia64 machine.  It runs HP/UX, which (a) is big-endian, and (b) does
not (to my knowledge) have an Ada port yet.  Therefore testing on
ia64-linux is, unfortunately, going to show problems that my testing
cannot.  It is not feasible for me to change the operating system on
the machine.

The implication of what you are saying above is that you don't think
anyone without access to an ia64-linux box should be allowed to touch
the ia64 back end, which I think is bad policy.

It is certainly the case that I don't know as much about coding
machine descriptions as some people do, and if you would like, I will
stop checking backend patches in immediately, to give others a chance
to find problems - but my experience is that everyone ignores me when
I ask for a second opinion on my patches, so I don't expect this to
help much.

I *have* managed to reproduce the original bootstrap failure with an
i686-linux->ia64-hpux Ada cross compiler, for the record.

> It has always worked this way.  I know this is lame, but one should 
> never underestimate the difficulty of trying to change how reload works. 
>   It is much easier to change all md file than to change reload.  I 
> think there is an important reason why it works this way, but it 
> probably isn't possible to figure it out without spending an 
> unreasonable amount of time messing with the code.

RTH and I talked this over on IRC and came up with a clever plan,
which I intend to implement tonight and post for review, assuming it
works, tomorrow morning.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at gcc dot gnu dot org]

------- Additional Comments From zack at gcc dot gnu dot org  2004-01-22 20:14 -------
Several messages regarding this bug were sent to PR 13772 by mistake.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2004-01-21 14:13:11         |2004-01-22 20:14:54
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-22 20:20 -------
Subject: Re: PR 13722 candidate fix


[Note corrected bug number. 13722, not 13772.]

Richard Henderson <rth@redhat.com> writes:

> On Thu, Jan 22, 2004 at 02:07:12PM +0100, Andreas Schwab wrote:
>> (mem/s:TI (post_modify:DI (reg/f:DI 36 loc4 [400])
>>         (plus:DI (reg/f:DI 36 loc4 [400])
>>             (const_int 8 [0x8]))) [4 opt__R47s+0 S16 A128])
>
> Yep, looking at the patch, I was about to remind Zack that 
> (1) post-modify can take registers as well as constants, and
> (2) you have to check for overflowing the 9-bit post-modify
>     constant.

That shouldn't be hard... I suppose the thing to do is emit adddi3
instructions to fix up, if either condition is true.

> Hum, and, actually, that post-modify seems very improbable.
> addr += addr?  There's sure to be a typo somewhere.

Okay, what is the form of a post_modify expression supposed to be?
This is coming from

        switch (base) {
          ...
          case POST_MODIFY:
	    /* Extract and adjust the modification.  */
	    offset = XEXP (base, 1);
	    base = XEXP (base, 0);

	    out[0] = change_address (in, DImode,
				     gen_rtx_POST_INC (Pmode, base));
	    out[1] = change_address (in, DImode,
				     gen_rtx_POST_MODIFY
				     (Pmode, base, plus_constant (offset, -8)));
            break;

and it sure *looks* consistent with what it says in the manual about
the post_modify RTL, which gives

          (mem:SF (post_modify:SI (reg:SI 42) (plus (reg:SI 42)
                                                    (reg:SI 48))))

as an exemplar.

(Still looking for a clue about the differences among change_address,
adjust_address, offset_address, etc.)

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[rth at redhat dot com]

------- Additional Comments From rth at redhat dot com  2004-01-22 22:05 -------
Subject: Re: PR 13722 candidate fix

On Thu, Jan 22, 2004 at 12:20:10PM -0800, Zack Weinberg wrote:
> Okay, what is the form of a post_modify expression supposed to be?

My eyes weren't awake, sorry.  It's addr = addr + c, not +=.

> (Still looking for a clue about the differences among change_address,
> adjust_address, offset_address, etc.)

change_address can make arbitrary changes to a memory.  About
all we assume is that we're pointing to the same object.  But
we lose track of offset within the object, and all alignment
not inferrable from the mode of the memory.

adjust_address modifies a memory by adding a HWI to the address.
We get to keep a lot of data about alignment, offset within an
object.

offset_address is similar, except that it takes an rtx.

adjust_automodify_address is like adjust_address in that it
computes new MEM_ATTRS as if adding a HWI to a base address,
except that it allows you to give a completely new rtx for
the address at the same time.  The assumption being that the
new rtx is some automodify, but that's not actually checked.

This last is the one you want.


r~


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-22 22:17 -------
Subject: Re:  [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

zack at codesourcery dot com wrote:
> The implication of what you are saying above is that you don't think
> anyone without access to an ia64-linux box should be allowed to touch
> the ia64 back end, which I think is bad policy.

This is one way to solve the problem, but not the only way.  I mention 
it because I think it is the most expedient.  Another solution would be 
for you to build an ia64-hpux Ada compiler.  Another solution is for you 
to voluntarily revert patches when it is discovered that they don't 
work.  Another solution is for you to ask for help testing patches 
before checking them in.

> to find problems - but my experience is that everyone ignores me when
> I ask for a second opinion on my patches, so I don't expect this to
> help much.

I don't ignore IA-64 backend patches, though I am sometimes a week or so 
behind on reading gcc-patches.  I wouldn't mind if you cc'ed me on IA-64 
backend patches so that I would see them sooner.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[rth at redhat dot com]

------- Additional Comments From rth at redhat dot com  2004-01-23 08:24 -------
Subject: Re: PR 13722 candidate fix

On Thu, Jan 22, 2004 at 11:43:40PM -0800, Zack Weinberg wrote:
> +	    out[0] = adjust_automodify_address
> +	      (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
> +
> +	    if (GET_CODE (XEXP (offset, 1)) == REG)
> +	      {
> +		rtx reg = XEXP (offset, 1);
> +		/* Tweak the index register, then emit the first move as
> +		   a POST_INC and the second as the original POST_MODIFY.  */
> +		emit_insn (GEN_FCN (add_optab->handlers[GET_MODE (reg)].insn_code)
> +			   (reg, reg, GEN_INT (-8)));

You can't modify the POST_MODIFY operand.
You need to modify the address register a third time, ie.

	ld8	x = [a], 8
	ld8	y = [a], -8
	add	a = r, a
or
	ld8	x = [a], 8
	ld8	y = [a], r
	adds	a = -8, a



r~


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-23 08:35 -------
Subject: Re: PR 13722 candidate fix

Richard Henderson <rth@redhat.com> writes:

> On Thu, Jan 22, 2004 at 11:43:40PM -0800, Zack Weinberg wrote:
>> +	    out[0] = adjust_automodify_address
>> +	      (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
>> +
>> +	    if (GET_CODE (XEXP (offset, 1)) == REG)
>> +	      {
>> +		rtx reg = XEXP (offset, 1);
>> +		/* Tweak the index register, then emit the first move as
>> +		   a POST_INC and the second as the original POST_MODIFY.  */
>> +		emit_insn (GEN_FCN (add_optab->handlers[GET_MODE (reg)].insn_code)
>> +			   (reg, reg, GEN_INT (-8)));
>
> You can't modify the POST_MODIFY operand.
> You need to modify the address register a third time, ie.

Duh, because the index register value might be needed again.  This
isn't true for constant POST_MODIFY, though, right?

(Gets me out of having to dig around in add_optab, though, yay.)

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-23 21:32 -------
Subject: Re: PR 13722 candidate fix

"Zack Weinberg" <zack@codesourcery.com> writes:

> Re-revised patch.

Seems to miscompile the stage2 compiler.

$ stage2/xgcc -Bstage2/ -B/usr/local/ia64-suse-linux/bin/ -c -g -O2      -gnatpg -gnata -I- -I. -Iada -I../../gcc/ada ../../gcc/ada/ada.ads -o ada/ada.o -v
Reading specs from stage2/specs
Configured with: ../configure --host=ia64-suse-linux --enable-shared --enable-threads --with-system-zlib
Thread model: posix
gcc version 3.5.0 20040123 (experimental)
 stage2/gnat1 -I- -I. -Iada -I../../gcc/ada -quiet -dumpbase ada.ads -g -gnatpg -gnata -O2 -gnatO ada/ada.o ../../gcc/ada/ada.ads -o /tmp/ccR0NvnR.s
fatal error, run-time library not installed correctly
cannot locate file system.ads
compilation abandoned

strace shows that gnat1 wants to stat() "" three times, whereas the stage1
compiler searches for "./system.ads", "ada/system.ads",
"../../gcc/ada/system.ads" in that order.  Looking at the contents of the
empty strings shows that they actually point to "\0/system.ads",
"\0\0\0/system.ads" and "\0\0\0\0\0\0\0\0\0\0\0\0\0/system.ads", ie. the
characters before "/system.ads" are replaced by '\0'.

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-23 21:51 -------
Subject: Re: PR 13722 candidate fix

Andreas Schwab <schwab@suse.de> writes:

> "Zack Weinberg" <zack@codesourcery.com> writes:
>
>> Re-revised patch.
>
> Seems to miscompile the stage2 compiler.
>
> $ stage2/xgcc -Bstage2/ -B/usr/local/ia64-suse-linux/bin/ -c -g -O2      -gnatpg -gnata -I- -I. -Iada -I../../gcc/ada ../../gcc/ada/ada.ads -o ada/ada.o -v
> Reading specs from stage2/specs
> Configured with: ../configure --host=ia64-suse-linux --enable-shared --enable-threads --with-system-zlib
> Thread model: posix
> gcc version 3.5.0 20040123 (experimental)
>  stage2/gnat1 -I- -I. -Iada -I../../gcc/ada -quiet -dumpbase ada.ads -g -gnatpg -gnata -O2 -gnatO ada/ada.o ../../gcc/ada/ada.ads -o /tmp/ccR0NvnR.s
> fatal error, run-time library not installed correctly
> cannot locate file system.ads
> compilation abandoned

Could you repeat the failing command with the added option -gnatdl,
and send me the output?

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-23 22:05 -------
Subject: Re: PR 13722 candidate fix

"Zack Weinberg" <zack@codesourcery.com> writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> "Zack Weinberg" <zack@codesourcery.com> writes:
>>
>>> Re-revised patch.
>>
>> Seems to miscompile the stage2 compiler.
>>
>> $ stage2/xgcc -Bstage2/ -B/usr/local/ia64-suse-linux/bin/ -c -g -O2      -gnatpg -gnata -I- -I. -Iada -I../../gcc/ada ../../gcc/ada/ada.ads -o ada/ada.o -v
>> Reading specs from stage2/specs
>> Configured with: ../configure --host=ia64-suse-linux --enable-shared --enable-threads --with-system-zlib
>> Thread model: posix
>> gcc version 3.5.0 20040123 (experimental)
>>  stage2/gnat1 -I- -I. -Iada -I../../gcc/ada -quiet -dumpbase ada.ads -g -gnatpg -gnata -O2 -gnatO ada/ada.o ../../gcc/ada/ada.ads -o /tmp/ccR0NvnR.s
>> fatal error, run-time library not installed correctly
>> cannot locate file system.ads
>> compilation abandoned
>
> Could you repeat the failing command with the added option -gnatdl,
> and send me the output?

Unfortunately, there is no additional output.

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-23 22:18 -------
Subject: Re: PR 13722 candidate fix

Andreas Schwab <schwab@suse.de> writes:

>> Could you repeat the failing command with the added option -gnatdl,
>> and send me the output?
>
> Unfortunately, there is no additional output.

I'm at a bit of a loss trying to find the miscompilation.  It's going
wrong somewhere either in Name_Find or Load_Source_File or their
subroutines, and I don't know which.  Could you try to run it under
the debugger and find out where the broken code is?

zw



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-23 22:55 -------
Subject: Re: PR 13722 candidate fix

On Thu, 2004-01-22 at 23:43, Zack Weinberg wrote:
> Under what circumstances should REG_INC notes be tagged onto the insns
> emitted here?  It's currently done only for POST_MODIFYs, but this
> patch will generate POST_INC and POST_DEC as well.

I believe the answer is that we need it for all auto-inc addresses.  The
REG_INC note indicates that the instruction has another side-effect
besides the sets and clobbers.  Any routines that care about instruction
side-effects aren't going to notice the auto-inc address unless the
REG_INC note is there.

The fact that we only do this after reload reduces the chances of a
problem, but the rtlanal.c reg_set_p function needs it, and it is likely
that there is already some code somewhere after reload that directly or
indirectly calls reg_set_p.  Thus the REG_INC notes should be added to
be safe.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-23 22:58 -------
Subject: Re: PR 13722 candidate fix

"Zack Weinberg" <zack@codesourcery.com> writes:

> I'm at a bit of a loss trying to find the miscompilation.  It's going
> wrong somewhere either in Name_Find or Load_Source_File or their
> subroutines, and I don't know which.  Could you try to run it under
> the debugger and find out where the broken code is?

I tried, but unfortunately gdb (even the one from CVS) is unable to
properly interpret the debug information that gcc is generating, making it
pretty much impossible to track variables and their values.

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-23 23:22 -------
Subject: Re: PR 13722 candidate fix

On Fri, 2004-01-23 at 00:47, Zack Weinberg wrote:
> Re-revised patch.

I started a bootstrap as soon as I saw the patch.  It just failed.  I
will try taking a look at this, but I don't know if I will have enough
time to do anything useful.

I did take a look at the patch.

You mention reload nightmares.  There is a reload nightmare only if you
fight how reload works.  If we allocate an OImode scratch instead of a
DImode scratch, then I think we will always get at least one safe
scratch register.  The instruction can use at most 4 registers, 2 for
the register operand, 1 for the address of the memory operand, and
optionally one for the auto-inc operand in the memory operand.  In the
worst case, where we have complete overlap between the 4 scratch
registers and the 4 insn registers, it gets more complicated.  If we are
doing a load, then we can use the dest register that is loaded last as
the scratch register.  If we are doing a store, hmm, I guess in this
case we need a workaround, but this is just one very specific case that
needs a workaround and it is going to be very rare.

Keep in mind that Itanium2 has 4 memory ports.  It can do up to 2 loads
and 2 stores per cycle.  Using auto-inc addresses instead of a scratch
reg eliminates ILP, and will hurt performance.  The TImode stuff perhaps
doesn't occur often enough to worry about, but I think this would
seriously hurt ia64-hpux long double performance.  I think we should
consider whether this performance hit is acceptable.  It might be
acceptable to get the compiler working again, but at the very least I
think it makes sense to add some ??? comments pointing out that we have
an optimization problem.  At least this doesn't affect ia64-linux long
double performance.

SUBREG is valid anywhere where a REG is valid.  I see a number of places
where you handle REG but not SUBREG.  Also, I see one place where you
assume a value is a REG without checking first.  The address of a MEM
could be a SUBREG.  The operand to POST_MODIFY could be a SUBREG.  The
"register" operand could be a SUBREG.  When you check for overlap
between the "register" operand and a MEM, we need to check for SUBREGs.

This problem and the missing REG_INC notes may be enough to explain why
it doesn't work.  I'll try adding simple checks for SUBREGs to see if
this problem actually occurs during an Ada bootstrap.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-24 00:04 -------
Subject: Re: PR 13722 candidate fix

"Zack Weinberg" <zack@codesourcery.com> writes:

> So, if you wouldn't mind trying the appended revision (changes only
> ia64.c)?

Does that mean that the changes in ia64.md from pr13722.diff should be
backed out?

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-24 00:18 -------
Subject: Re: PR 13722 candidate fix

On Fri, 2004-01-23 at 00:47, Zack Weinberg wrote:
> Re-revised patch.

On the REG/SUBREG issue, I was looking at your latest patch.  In the
full context of the previous patch, I see you already handle most of
these issues by having a default switch case that aborts.  The only real
issue I see here is in the POST_MODIFY code where you have
+           if (GET_CODE (XEXP (offset, 1)) == REG)
...
+           else if (INTVAL (XEXP (offset, 1)) < -256 + 8)
which assumes without checking that the offset is a CONST_INT if it
isn't a REG.  I put in an abort to see if this ever happens, but I doubt
that it does.

There is also a possible issue where you call reg_overlap_mentioned_p,
and then assume that we have registers without checking.  However, since
we are already calling ia64_split_tmode, and that aborts for subregs, I
think this is actually safe optimization wise.  I think this can only
fail if you have rtl checking enabled, and we have a subreg, in which we
get the abort before calling ia64_split_tmode instead of inside
ia64_split_tmode which isn't a serious problem.

I see you have another modified patch.  I can try this when my current
build finishes.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[rth at redhat dot com]

------- Additional Comments From rth at redhat dot com  2004-01-24 00:29 -------
Subject: Re: PR 13722 candidate fix

On Fri, Jan 23, 2004 at 10:32:51PM +0100, Andreas Schwab wrote:
> strace shows that gnat1 wants to stat() "" three times, whereas the stage1
> compiler searches for "./system.ads", "ada/system.ads",
> "../../gcc/ada/system.ads" in that order.  Looking at the contents of the
> empty strings shows that they actually point to "\0/system.ads",
> "\0\0\0/system.ads" and "\0\0\0\0\0\0\0\0\0\0\0\0\0/system.ads", ie. the
> characters before "/system.ads" are replaced by '\0'.

Could this be a miscompilation of __builtin_memcpy or the like?


r~


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-24 01:28 -------
Subject: Re: PR 13722 candidate fix

Andreas Schwab <schwab@suse.de> writes:

> "Zack Weinberg" <zack@codesourcery.com> writes:
>
>> So, if you wouldn't mind trying the appended revision (changes only
>> ia64.c)?
>
> Does that mean that the changes in ia64.md from pr13722.diff should be
> backed out?

No, what I meant was, leave ia64.md alone and just re-patch ia64.c.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-24 01:35 -------
Subject: Re: PR 13722 candidate fix

Jim Wilson <wilson@specifixinc.com> writes:

> On Fri, 2004-01-23 at 00:47, Zack Weinberg wrote:
>> Re-revised patch.
>
> On the REG/SUBREG issue, I was looking at your latest patch.  In the
> full context of the previous patch, I see you already handle most of
> these issues by having a default switch case that aborts.

Yes.  I think there are good /a priori/ reasons to assume that we
won't ever get SUBREGs here, so I think this is adequate.  (A SUBREG
in the operand itself could only be paradoxical, and shouldn't survive
this far; a SUBREG in the MEM expression would either be paradoxical,
and again shouldn't have survived this far, or would take it out of
Pmode and therefore indicate a bug elsewhere.)

> The only real issue I see here is in the POST_MODIFY code where you
> have
> +           if (GET_CODE (XEXP (offset, 1)) == REG)
> ...
> +           else if (INTVAL (XEXP (offset, 1)) < -256 + 8)
> which assumes without checking that the offset is a CONST_INT if it
> isn't a REG.  I put in an abort to see if this ever happens, but I doubt
> that it does.

The theory here was that INTVAL would trigger an RTL checking abort if
offset wasn't a CONST_INT, so there was no need for an explicit
check.  I now remember that RTL checking isn't on by default, so an
explicit check would probably be wise.

> I see you have another modified patch.  I can try this when my current
> build finishes.

I would appreciate that.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-24 01:51 -------
Subject: Re: PR 13722 candidate fix

Jim Wilson <wilson@specifixinc.com> writes:

> You mention reload nightmares.  There is a reload nightmare only if you
> fight how reload works.  If we allocate an OImode scratch instead of a
> DImode scratch, then I think we will always get at least one safe
> scratch register.
[...]

I am more comfortable coding it this way, and I think it will be
easier to understand in six months even without benefit of lengthy
commentary.  Furthermore, I would like to eliminate OImode from the
ia64 back end (currently it's only used for STACK_SAVEAREA_MODE, which
should be handled differently) so I would prefer a solution that
doesn't need it.

[...]
> Keep in mind that Itanium2 has 4 memory ports.  It can do up to 2 loads
> and 2 stores per cycle.  Using auto-inc addresses instead of a scratch
> reg eliminates ILP, and will hurt performance.  The TImode stuff perhaps
> doesn't occur often enough to worry about, but I think this would
> seriously hurt ia64-hpux long double performance.  I think we should
> consider whether this performance hit is acceptable.  It might be
> acceptable to get the compiler working again, but at the very least I
> think it makes sense to add some ??? comments pointing out that we have
> an optimization problem.  At least this doesn't affect ia64-linux long
> double performance.

This, I think, can be addressed with peephole2 patterns, which have
the ability to allocate scratch registers without risk of overlap.  A
form like

(define_peephole2
  [(match_scratch:DI 3 "r")
   (set (match_operand:DI 0 "register_operand" "")
        (mem:DI (post_inc:DI (match_operand:DI 2 "register_operand" ""))))
   (set (match_operand:DI 1 "register_operand" "")
        (mem:DI (post_dec:DI (match_dup 2))))
   (match_dup 3)]
  "!optimize_size"
  [(set (match_dup 3)
        (add:DI (match_dup 2) (const_int 8)))
   (set (match_dup 0) (mem:DI (match_dup 2)))
   (set (match_dup 1) (mem:DI (match_dup 3)))]
  "")

will clean up the majority of these - note that it also gives us the
ability to *not* use a scratch pointer when optimizing for size, and
will just work in cases where there aren't scratch regs to be had.
(I am not claiming that the above is exactly the way it should be
written.  Perhaps it ought to be done by matching the MEMs and
changing them with adjust_automodify_address, or something like that.)

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-24 01:56 -------
Subject: Re: PR 13722 candidate fix

"Zack Weinberg" <zack@codesourcery.com> writes:

> So, if you wouldn't mind trying the appended revision (changes only
> ia64.c)?

Unfortunately that didn't help, still the same problem with finding
system.ads.

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-24 02:20 -------
Subject: Re: PR 13722 candidate fix

Andreas Schwab <schwab@suse.de> writes:

> "Zack Weinberg" <zack@codesourcery.com> writes:
>
>> So, if you wouldn't mind trying the appended revision (changes only
>> ia64.c)?
>
> Unfortunately that didn't help, still the same problem with finding
> system.ads.

Ugh.  I'm running out of ideas.

Just to be sure this isn't an unrelated bug, could you revert ia64.c
to version 1.264 and ia64.md to version 1.118, and see if it works
again?  (That is, backing out both these fixes and the original
problematic change.)

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-24 03:31 -------
Subject: Re: PR 13722 candidate fix

On Fri, 2004-01-23 at 17:50, Zack Weinberg wrote:
> This, I think, can be addressed with peephole2 patterns, which have
> the ability to allocate scratch registers without risk of overlap.  A
> form like

I forgot that you had mentioned peephole2 earlier.  I am not very
familiar with this code.  I looked at the docs and looked at the code a
bit, and I see that this does look like it will work nicely.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-24 03:35 -------
Subject: Re: PR 13722 candidate fix

Jim Wilson <wilson@specifixinc.com> writes:

> On Fri, 2004-01-23 at 17:50, Zack Weinberg wrote:
>> This, I think, can be addressed with peephole2 patterns, which have
>> the ability to allocate scratch registers without risk of overlap.  A
>> form like
>
> I forgot that you had mentioned peephole2 earlier.  I am not very
> familiar with this code.  I looked at the docs and looked at the code a
> bit, and I see that this does look like it will work nicely.

Ok.  I would prefer to implement that in a separate patch after we fix
the bootstrap failure, though.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-24 03:55 -------
Subject: Re: PR 13722 candidate fix

On Fri, 2004-01-23 at 19:34, Zack Weinberg wrote:
> Ok.  I would prefer to implement that in a separate patch after we fix
> the bootstrap failure, though.

Yes, that is fine.  The important thing is that you do have a sensible
plan, which I had forgotten about.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-24 14:10 -------
Subject: Re: PR 13722 candidate fix

"Zack Weinberg" <zack@codesourcery.com> writes:

> Just to be sure this isn't an unrelated bug, could you revert ia64.c
> to version 1.264 and ia64.md to version 1.118, and see if it works
> again?  (That is, backing out both these fixes and the original
> problematic change.)

The regression tester is already running here with the patches backed out
for some days.  But current mainline failed to bootstrap today, apparently
due to dereferencing freed memory in get_addr (3.4 didn't fail, but
probably only because of disabled checks).  Looks like an unrelated bug,
though.

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-24 17:38 -------
Subject: Re: PR 13722 candidate fix

On Fri, 2004-01-23 at 15:23, Zack Weinberg wrote:
> unadjustible POST_MODIFY were being emitted in the wrong place).  So,
> if you wouldn't mind trying the appended revision (changes only
> ia64.c)?

FYI you got the PR number wrong again in the attachment file name.

A -O0 c/ada bootstrap works.  A -O1 c/ada bootstrap does not.  So there
is the possibility here of doing a binary search to narrow down the
problem to one file, and with a little hackery, we could narrow down the
problem to one function.  This doesn't require any heavy lifting, but
will be time consuming.

I'm going to try disabling auto-inc addressing types, to see if maybe
only one of them is broken.

I think it would also be helpful to have debug_rtx() calls in
ia64_split_tmode_move, so I can see the actual before and after rtx
during the ada build.  I might be able to spot a problem that way.  I
may try doing this.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-24 20:17 -------
Subject: Re: PR 13722 candidate fix

I tried a bootstrap with all HAVE_POST_* macros turned off, and it still
failed.

I tried adding debug_rtx{_list,} calls to ia64_split_tmode_move and I
noticed a few things.

You are adding REG_INC notes when a MEM operand is a dest, but not when
a src.  An auto-inc address is still a side-effect even when it is used
in a source operand.  This is easy to fix by copying some code, I'll
have to try another bootstrap with this change.

I noticed that we are looking REG_*_P flag bits and alias info for
registers, because you are generating new ones with with gen_rtx_REG
without copying any info over from the old ones.  It isn't clear if this
is an optimization problem, I suspect not.  I think this is only a
potential performance loss.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-25 04:09 -------
Subject: Re: PR 13722 candidate fix

My last bootstrap worked.  I forgot to use the --enable-languages
option, so I ended up doing a full build plus gnatlib_and_tools.  I
didn't try running the testsuite.

I've got two changes on top of your patches, one to emit REG_INC notes
if in[0] or in[1] contain an auto-inc address, and one that sets all of
the HAVE_POST_* macros to zero which was added to help isolate the
problem.

I have removed the HAVE_POST_* changes, and I'm trying another
bootstrap.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-25 22:33 -------
Subject: Re: PR 13722 candidate fix

Jim Wilson <wilson@specifixinc.com> writes:

> My latest bootstrap succeeded.  I didn't run the testsuite, and I still
> had some debug code in there just to be safe, but I don't think either
> of those issues should matter.  With the following patch on top of your
> patches, the IA-64 gcc port bootstrap again including Ada.  This patch
> adds the REG_INC notes for auto-inc addresses in the IN operands.  There
> is also the potential problem I pointed out earlier where we use INTVAL
> without checking to see if we have a CONST_INT.  That isn't in this
> patch, but I think it should go in also.

Okay.  I have incorporated your changes into my patch, added the
CONST_INT check, and am running a bootstrap on ia64-hpux; if it
succeeds I'll repost the combined patch.

[...]
> So what I am asking here is that if you check in a patch that
> breaks the IA-64 compiler, and you are not able to solve the problem
> yourself, that you revert the patch until I am able to help you.  I am
> willing to help you with these kinds of things, but it isn't fair to
> others to let you monopolize my time, and it isn't fair to others to
> leave the IA-64 compiler broken for long periods of time.

I am willing to do this for most breaks.

However, it is my understanding that breaks that affect only Ada are
*not* considered critical, and in fact that there is no obligation for
anyone to attempt to build the Ada front end.  I normally do not
bother building it even on platforms that can handle it (ia64-hpux
cannot, as the runtime has not been ported).  This is why I did not
respond very fast to this bug report.  I am willing to change my
habits if the policy has changed, but not otherwise.

> If you want to be really helpful, you could try looking at some of the
> IA-64 gcc bug reports in bugzilla.  PR 7198 is on my list, and is one
> that you could handle.

Sure, I'll look at that.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

Re: [Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[Arnaud Charlet]

> However, it is my understanding that breaks that affect only Ada are
> *not* considered critical, and in fact that there is no obligation for
> anyone to attempt to build the Ada front end.  I normally do not
> bother building it even on platforms that can handle it (ia64-hpux
> cannot, as the runtime has not been ported).  This is why I did not

Even if the run time has not been tailored for ia64-hpux, you can still
build the generic part, which is more than enough for doing a bootstrap.

> respond very fast to this bug report.  I am willing to change my
> habits if the policy has changed, but not otherwise.

As shown by most of the Ada regressions recently, most of these regressions
actually show real bugs in the non Ada part of GCC, so I believe this is
in the interest of everyone to take Ada reports into consideration.

I believe the policy you are stating was fine as long as Ada was not
properly maintained, which is no longer the case.

I'd suggest reconsidering the state of Ada, given the efforts invested
recently on this front-end (as shown by the regular syncs and by the
handling of Ada PRs in bugzilla).

Arno

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[charlet at act-europe dot fr]

------- Additional Comments From charlet at act-europe dot fr  2004-01-26 08:14 -------
Subject: Re:  [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

> However, it is my understanding that breaks that affect only Ada are
> *not* considered critical, and in fact that there is no obligation for
> anyone to attempt to build the Ada front end.  I normally do not
> bother building it even on platforms that can handle it (ia64-hpux
> cannot, as the runtime has not been ported).  This is why I did not

Even if the run time has not been tailored for ia64-hpux, you can still
build the generic part, which is more than enough for doing a bootstrap.

> respond very fast to this bug report.  I am willing to change my
> habits if the policy has changed, but not otherwise.

As shown by most of the Ada regressions recently, most of these regressions
actually show real bugs in the non Ada part of GCC, so I believe this is
in the interest of everyone to take Ada reports into consideration.

I believe the policy you are stating was fine as long as Ada was not
properly maintained, which is no longer the case.

I'd suggest reconsidering the state of Ada, given the efforts invested
recently on this front-end (as shown by the regular syncs and by the
handling of Ada PRs in bugzilla).

Arno


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-26 18:03 -------
Subject: Hopefully final patch for PR 13722


This incorporates all bugfixes to date and gets good test results on
ia64-hpux (3.4 branch):

XPASS: gcc.c-torture/execute/990208-1.c execution,  -O3 -fomit-frame-pointer 
XPASS: gcc.c-torture/execute/990208-1.c execution,  -O3 -g 
FAIL: gcc.dg/compat/scalar-by-value-3 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/scalar-by-value-4 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-by-value-5 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/compat/struct-return-19 c_compat_x_tst.o-c_compat_y_tst.o execute 
FAIL: gcc.dg/20020219-1.c execution test
FAIL: gcc.dg/20021014-1.c execution test
FAIL: gcc.dg/20031202-1.c execution test
FAIL: gcc.dg/cleanup-5.c (test for excess errors)
FAIL: gcc.dg/nest.c execution test
FAIL: gcc.dg/pch/inline-3.c -O0 -g assembly comparison
FAIL: gcc.dg/pch/inline-3.c  -O0  assembly comparison
FAIL: gcc.dg/pch/inline-3.c  -O1  assembly comparison
FAIL: gcc.dg/pch/inline-3.c  -O2  assembly comparison
FAIL: gcc.dg/pch/inline-3.c  -O3 -fomit-frame-pointer  assembly comparison
FAIL: gcc.dg/pch/inline-3.c  -O3 -g  assembly comparison
FAIL: gcc.dg/pch/inline-3.c  -Os  assembly comparison

I believe all of these are preexisting.  All failures appear in both
-milp32 and -mlp64 modes.

zw

===================================================================
Index: config/ia64/ia64.c
--- config/ia64/ia64.c	16 Jan 2004 01:27:37 -0000	1.265
+++ config/ia64/ia64.c	26 Jan 2004 18:00:38 -0000
@@ -1362,62 +1362,37 @@ ia64_emit_cond_move (rtx op0, rtx op1, r
 }
 
 /* Split a post-reload TImode or TFmode reference into two DImode
-   components.  */
+   components.  This is made extra difficult by the fact that we do
+   not get any scratch registers to work with, because reload cannot
+   be prevented from giving us a scratch that overlaps the register
+   pair involved.  So instead, when addressing memory, we tweak the
+   pointer register up and back down with POST_INCs.  Or up and not
+   back down when we can get away with it.
+
+   REVERSED is true when the loads must be done in reversed order
+   (high word first) for correctness.  DEAD is true when the pointer
+   dies with the second insn we generate and therefore the second
+   address must not carry a postmodify.
+
+   May return an insn which is to be emitted after the moves.  */
 
 static rtx
-ia64_split_tmode (rtx out[2], rtx in, rtx scratch)
+ia64_split_tmode (rtx out[2], rtx in, bool reversed, bool dead)
 {
+  rtx fixup = 0;
+
   switch (GET_CODE (in))
     {
     case REG:
-      out[0] = gen_rtx_REG (DImode, REGNO (in));
-      out[1] = gen_rtx_REG (DImode, REGNO (in) + 1);
-      return NULL_RTX;
-
-    case MEM:
-      {
-	rtx base = XEXP (in, 0);
-
-	switch (GET_CODE (base))
-	  {
-	  case REG:
-	    out[0] = adjust_address (in, DImode, 0);
-	    break;
-	  case POST_MODIFY:
-	    base = XEXP (base, 0);
-	    out[0] = adjust_address (in, DImode, 0);
-	    break;
-
-	  /* Since we're changing the mode, we need to change to POST_MODIFY
-	     as well to preserve the size of the increment.  Either that or
-	     do the update in two steps, but we've already got this scratch
-	     register handy so let's use it.  */
-	  case POST_INC:
-	    base = XEXP (base, 0);
-	    out[0]
-	      = change_address (in, DImode,
-				gen_rtx_POST_MODIFY
-				(Pmode, base, plus_constant (base, 16)));
-	    break;
-	  case POST_DEC:
-	    base = XEXP (base, 0);
-	    out[0]
-	      = change_address (in, DImode,
-				gen_rtx_POST_MODIFY
-				(Pmode, base, plus_constant (base, -16)));
-	    break;
-	  default:
-	    abort ();
-	  }
-
-	if (scratch == NULL_RTX)
-	  abort ();
-	out[1] = change_address (in, DImode, scratch);
-	return gen_adddi3 (scratch, base, GEN_INT (8));
-      }
+      out[reversed] = gen_rtx_REG (DImode, REGNO (in));
+      out[!reversed] = gen_rtx_REG (DImode, REGNO (in) + 1);
+      break;
 
     case CONST_INT:
     case CONST_DOUBLE:
+      /* Cannot occur reversed.  */
+      if (reversed) abort ();
+      
       if (GET_MODE (in) != TFmode)
 	split_double (in, &out[0], &out[1]);
       else
@@ -1444,11 +1419,108 @@ ia64_split_tmode (rtx out[2], rtx in, rt
 	  out[0] = GEN_INT (p[0]);
 	  out[1] = GEN_INT (p[1]);
 	}
-      return NULL_RTX;
+      break;
+
+    case MEM:
+      {
+	rtx base = XEXP (in, 0);
+	rtx offset;
+
+	switch (GET_CODE (base))
+	  {
+	  case REG:
+	    if (!reversed)
+	      {
+		out[0] = adjust_automodify_address
+		  (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
+		out[1] = adjust_automodify_address
+		  (in, DImode, dead ? 0 : gen_rtx_POST_DEC (Pmode, base), 8);
+	      }
+	    else
+	      {
+		/* Reversal requires a pre-increment, which can only
+		   be done as a separate insn.  */
+		emit_insn (gen_adddi3 (base, base, GEN_INT (8)));
+		out[0] = adjust_automodify_address
+		  (in, DImode, gen_rtx_POST_DEC (Pmode, base), 8);
+		out[1] = adjust_address (in, DImode, 0);
+	      }
+	    break;
+
+	  case POST_INC:
+	    if (reversed || dead) abort ();
+	    /* Just do the increment in two steps.  */
+	    out[0] = adjust_automodify_address (in, DImode, 0, 0);
+	    out[1] = adjust_automodify_address (in, DImode, 0, 8);
+	    break;
+
+	  case POST_DEC:
+	    if (reversed || dead) abort ();
+	    /* Add 8, subtract 24.  */
+	    base = XEXP (base, 0);
+	    out[0] = adjust_automodify_address
+	      (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
+	    out[1] = adjust_automodify_address
+	      (in, DImode,
+	       gen_rtx_POST_MODIFY (Pmode, base, plus_constant (base, -24)),
+	       8);
+	    break;
+
+	  case POST_MODIFY:
+	    if (reversed || dead) abort ();
+	    /* Extract and adjust the modification.  This case is
+	       trickier than the others, because we might have an
+	       index register, or we might have a combined offset that
+	       doesn't fit a signed 9-bit displacement field.  We can
+	       assume the incoming expression is already legitimate.  */
+	    offset = XEXP (base, 1);
+	    base = XEXP (base, 0);
+
+	    out[0] = adjust_automodify_address
+	      (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
+
+	    if (GET_CODE (XEXP (offset, 1)) == REG)
+	      {
+		/* Can't adjust the postmodify to match.  Emit the
+		   original, then a separate addition insn.  */
+		out[1] = adjust_automodify_address (in, DImode, 0, 8);
+		fixup = gen_adddi3 (base, base, GEN_INT (-8));
+	      }
+	    else if (GET_CODE (XEXP (offset, 1)) != CONST_INT)
+	      abort ();
+	    else if (INTVAL (XEXP (offset, 1)) < -256 + 8)
+	      {
+		/* Again the postmodify cannot be made to match, but
+		   in this case it's more efficient to get rid of the
+		   postmodify entirely and fix up with an add insn. */
+		out[1] = adjust_automodify_address (in, DImode, base, 8);
+		fixup = gen_adddi3 (base, base,
+				    GEN_INT (INTVAL (XEXP (offset, 1)) - 8));
+	      }
+	    else
+	      {
+		/* Combined offset still fits in the displacement field.
+		   (We cannot overflow it at the high end.)  */
+		out[1] = adjust_automodify_address
+		  (in, DImode,
+		   gen_rtx_POST_MODIFY (Pmode, base,
+		     gen_rtx_PLUS (Pmode, base,
+				   GEN_INT (INTVAL (XEXP (offset, 1)) - 8))),
+		   8);
+	      }
+	    break;
+
+	  default:
+	    abort ();
+	  }
+	break;
+      }
 
     default:
       abort ();
     }
+
+  return fixup;
 }
 
 /* Split a TImode or TFmode move instruction after reload.
@@ -1456,39 +1528,54 @@ ia64_split_tmode (rtx out[2], rtx in, rt
 void
 ia64_split_tmode_move (rtx operands[])
 {
-  rtx adj1, adj2, in[2], out[2], insn;
-  int first;
-
-  adj1 = ia64_split_tmode (in, operands[1], operands[2]);
-  adj2 = ia64_split_tmode (out, operands[0], operands[2]);
-
-  first = 0;
-  if (reg_overlap_mentioned_p (out[0], in[1]))
-    {
-      if (reg_overlap_mentioned_p (out[1], in[0]))
-	abort ();
-      first = 1;
-    }
-
-  if (adj1 && adj2)
-    abort ();
-  if (adj1)
-    emit_insn (adj1);
-  if (adj2)
-    emit_insn (adj2);
-  insn = emit_insn (gen_rtx_SET (VOIDmode, out[first], in[first]));
-  if (GET_CODE (out[first]) == MEM
-      && GET_CODE (XEXP (out[first], 0)) == POST_MODIFY)
-    REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_INC,
-					  XEXP (XEXP (out[first], 0), 0),
-					  REG_NOTES (insn));
-  insn = emit_insn (gen_rtx_SET (VOIDmode, out[!first], in[!first]));
-  if (GET_CODE (out[!first]) == MEM
-      && GET_CODE (XEXP (out[!first], 0)) == POST_MODIFY)
-    REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_INC,
-					  XEXP (XEXP (out[!first], 0), 0),
-					  REG_NOTES (insn));
+  rtx in[2], out[2], insn;
+  rtx fixup[2];
+  bool dead = false;
+  bool reversed = false;
+
+  /* It is possible for reload to decide to overwrite a pointer with
+     the value it points to.  In that case we have to do the loads in
+     the appropriate order so that the pointer is not destroyed too
+     early.  Also we must not generate a postmodify for that second
+     load, or rws_access_regno will abort.  */
+  if (GET_CODE (operands[1]) == MEM
+      && reg_overlap_mentioned_p (operands[0], operands[1]))
+    {
+      rtx base = XEXP (operands[1], 0);
+      while (GET_CODE (base) != REG)
+	base = XEXP (base, 0);
+
+      if (REGNO (base) == REGNO (operands[0]))
+	reversed = true;
+      dead = true;
+    }
+
+  fixup[0] = ia64_split_tmode (in, operands[1], reversed, dead);
+  fixup[1] = ia64_split_tmode (out, operands[0], reversed, dead);
+
+#define MAYBE_ADD_REG_INC_NOTE(INSN, EXP)				\
+  if (GET_CODE (EXP) == MEM						\
+      && (GET_CODE (XEXP (EXP, 0)) == POST_MODIFY			\
+	  || GET_CODE (XEXP (EXP, 0)) == POST_INC			\
+	  || GET_CODE (XEXP (EXP, 0)) == POST_DEC))			\
+    REG_NOTES (INSN) = gen_rtx_EXPR_LIST (REG_INC,			\
+					  XEXP (XEXP (EXP, 0), 0),	\
+					  REG_NOTES (INSN))
+
+  insn = emit_insn (gen_rtx_SET (VOIDmode, out[0], in[0]));
+  MAYBE_ADD_REG_INC_NOTE (insn, in[0]);
+  MAYBE_ADD_REG_INC_NOTE (insn, out[0]);
+
+  insn = emit_insn (gen_rtx_SET (VOIDmode, out[1], in[1]));
+  MAYBE_ADD_REG_INC_NOTE (insn, in[1]);
+  MAYBE_ADD_REG_INC_NOTE (insn, out[1]);
+
+  if (fixup[0])
+    emit_insn (fixup[0]);
+  if (fixup[1])
+    emit_insn (fixup[1]);
 
+#undef MAYBE_ADD_REG_INC_NOTE
 }
 
 /* ??? Fixing GR->FR XFmode moves during reload is hard.  You need to go
@@ -4451,13 +4538,6 @@ ia64_secondary_reload_class (enum reg_cl
       /* This can happen when we take a BImode subreg of a DImode value,
 	 and that DImode value winds up in some non-GR register.  */
       if (regno >= 0 && ! GENERAL_REGNO_P (regno) && ! PR_REGNO_P (regno))
-	return GR_REGS;
-      break;
-
-    case GR_REGS:
-      /* Since we have no offsettable memory addresses, we need a temporary
-	 to hold the address of the second word.  */
-      if (mode == TImode || mode == TFmode)
 	return GR_REGS;
       break;
 
===================================================================
Index: config/ia64/ia64.md
--- config/ia64/ia64.md	16 Jan 2004 01:27:38 -0000	1.119
+++ config/ia64/ia64.md	26 Jan 2004 18:00:38 -0000
@@ -584,11 +584,12 @@
   [(set_attr "itanium_class" "ialu")])
 
 ;; With no offsettable memory references, we've got to have a scratch
-;; around to play with the second word.
+;; around to play with the second word.  However, in order to avoid a
+;; reload nightmare we lie, claim we don't need one, and fix it up
+;; in ia64_split_tmode_move.
 (define_expand "movti"
-  [(parallel [(set (match_operand:TI 0 "general_operand" "")
-		   (match_operand:TI 1 "general_operand" ""))
-	      (clobber (match_scratch:DI 2 ""))])]
+  [(set (match_operand:TI 0 "general_operand" "")
+	(match_operand:TI 1 "general_operand" ""))]
   ""
 {
   rtx op1 = ia64_expand_move (operands[0], operands[1]);
@@ -599,8 +600,7 @@
 
 (define_insn_and_split "*movti_internal"
   [(set (match_operand:TI 0 "nonimmediate_operand" "=r,r,m")
-	(match_operand:TI 1 "general_operand"      "ri,m,r"))
-   (clobber (match_scratch:DI 2 "=X,&r,&r"))]
+	(match_operand:TI 1 "general_operand"      "ri,m,r"))]
   "ia64_move_ok (operands[0], operands[1])"
   "#"
   "reload_completed"
@@ -612,20 +612,6 @@
   [(set_attr "itanium_class" "unknown")
    (set_attr "predicable" "no")])
 
-(define_expand "reload_inti"
-  [(parallel [(set (match_operand:TI 0 "register_operand" "=r")
-		   (match_operand:TI 1 "memory_operand" "m"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
-
-(define_expand "reload_outti"
-  [(parallel [(set (match_operand:TI 0 "memory_operand" "=m")
-		   (match_operand:TI 1 "register_operand" "r"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
-
 ;; Floating Point Moves
 ;;
 ;; Note - Patterns for SF mode moves are compulsory, but
@@ -764,13 +750,10 @@
   [(set_attr "itanium_class" "fmisc,fld,stf")])
 
 ;; Better code generation via insns that deal with TFmode register pairs
-;; directly.
-;; With no offsettable memory references, we've got to have a scratch
-;; around to play with the second word.
+;; directly.  Same concerns apply as for TImode.
 (define_expand "movtf"
-  [(parallel [(set (match_operand:TF 0 "general_operand" "")
-		   (match_operand:TF 1 "general_operand" ""))
-	      (clobber (match_scratch:DI 2 ""))])]
+  [(set (match_operand:TF 0 "general_operand" "")
+	(match_operand:TF 1 "general_operand" ""))]
   ""
 {
   rtx op1 = ia64_expand_move (operands[0], operands[1]);
@@ -781,8 +764,7 @@
 
 (define_insn_and_split "*movtf_internal"
   [(set (match_operand:TF 0 "nonimmediate_operand" "=r,r,m")
-	(match_operand:TF 1 "general_operand"      "ri,m,r"))
-   (clobber (match_scratch:DI 2 "=X,&r,&r"))]
+	(match_operand:TF 1 "general_operand"      "ri,m,r"))]
   "ia64_move_ok (operands[0], operands[1])"
   "#"
   "reload_completed"
@@ -794,19 +776,6 @@
   [(set_attr "itanium_class" "unknown")
    (set_attr "predicable" "no")])
 
-(define_expand "reload_intf"
-  [(parallel [(set (match_operand:TF 0 "register_operand" "=r")
-		   (match_operand:TF 1 "memory_operand" "m"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
-
-(define_expand "reload_outtf"
-  [(parallel [(set (match_operand:TF 0 "memory_operand" "=m")
-		   (match_operand:TF 1 "register_operand" "r"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
 \f
 ;; ::::::::::::::::::::
 ;; ::


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-27 00:08 -------
Subject: Re: Hopefully final patch for PR 13722

On Mon, 2004-01-26 at 10:02, Zack Weinberg wrote:
> This incorporates all bugfixes to date and gets good test results on
> ia64-hpux (3.4 branch):

Thanks.  It looks good to me also.  I'll add in your patches for 13722
and 7198 and do another ia64-linux bootstrap.  My IA-64 machine is idle
today anyways, so this is no bother for me.  I don't mind if you go
ahead and check in the patches first.  I'll let you know if something
goes wrong.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[ahs3 at fc dot hp dot com]

------- Additional Comments From ahs3 at fc dot hp dot com  2004-01-27 17:50 -------
Subject: Re: Hopefully final patch for PR 13722

I probably should have spoken up before but I was out of town
for a while.

I've got an ia64 machine that I try to run nightly bootstrap
and check on (it's sporadic right now).  I'd be glad to get 
it so it does run nightly and send out the results (and try
to at least isolate what breaks).  This would be for ia64-linux,
on Debian unstable.

Are there particular configuration parameters that should be
set?  I typically only build and test for C, C++ and g77, without
maintainer mode set.  Any particular version of the compiler
to be used for the bootstrap?

What should I do with the results?  Email them somewhere?  Post
them somewhere on gcc.gnu.org?  I've only done a real cursory
search of the site but didn't find much....

Let me know what I can do to help.


On Mon, 2004-01-26 at 17:08, Jim Wilson wrote:
> On Mon, 2004-01-26 at 10:02, Zack Weinberg wrote:
> > This incorporates all bugfixes to date and gets good test results on
> > ia64-hpux (3.4 branch):
> 
> Thanks.  It looks good to me also.  I'll add in your patches for 13722
> and 7198 and do another ia64-linux bootstrap.  My IA-64 machine is idle
> today anyways, so this is no bother for me.  I don't mind if you go
> ahead and check in the patches first.  I'll let you know if something
> goes wrong.
-- 
Ciao,
al
----------------------------
Al Stone
Linux & Open Source Lab
Hewlett-Packard Company
Phone:  970-898-0345
Telnet:     898-0345
Fax:    970-898-3804
E-mail: ahs3@fc.hp.com
----------------------------



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-27 18:13 -------
Subject: Re: Hopefully final patch for PR 13722

Al Stone <ahs3@fc.hp.com> writes:

> I probably should have spoken up before but I was out of town
> for a while.
>
> I've got an ia64 machine that I try to run nightly bootstrap
> and check on (it's sporadic right now).  I'd be glad to get 
> it so it does run nightly and send out the results (and try
> to at least isolate what breaks).  This would be for ia64-linux,
> on Debian unstable.

That would be really great.

> Are there particular configuration parameters that should be
> set?  I typically only build and test for C, C++ and g77, without
> maintainer mode set.

That should be fine.

> Any particular version of the compiler to be used for the bootstrap?

I've been using 3.3.2 for ia64 bootstrap base compiler, with good results.

> What should I do with the results?  Email them somewhere?  Post
> them somewhere on gcc.gnu.org?  I've only done a real cursory
> search of the site but didn't find much....

There is a mailing list, gcc-testresults, for posting these results.

If you set up an automatic nag script that reports regressions when
they happen, there is also gcc-regression.  Talk to Geoff Keating
and Phil Edwards about this.

zw


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[ahs3 at fc dot hp dot com]

------- Additional Comments From ahs3 at fc dot hp dot com  2004-01-27 18:43 -------
Subject: Re: Hopefully final patch for PR 13722

On Tue, 2004-01-27 at 11:13, Zack Weinberg wrote:
> Al Stone <ahs3@fc.hp.com> writes:
> > I've got an ia64 machine that I try to run nightly bootstrap
> > and check on (it's sporadic right now).  I'd be glad to get 
> > it so it does run nightly and send out the results (and try
> > to at least isolate what breaks).  This would be for ia64-linux,
> > on Debian unstable.
> 
> That would be really great.

Cool.  I'll try to get it running by this weekend.

> > Any particular version of the compiler to be used for the bootstrap?
> 
> I've been using 3.3.2 for ia64 bootstrap base compiler, with good results.

Hmm.  The Debian one is usually a _little_ bit different than
the officially released version.  I'll start with using the Debian
default (3.3.3 today) and we'll see what happens.

> > What should I do with the results?  Email them somewhere?  Post
> > them somewhere on gcc.gnu.org?  I've only done a real cursory
> > search of the site but didn't find much....
> 
> There is a mailing list, gcc-testresults, for posting these results.

Ah.  Thanks.  I knew there was something somewhere but just wasn't
seeing it.

> If you set up an automatic nag script that reports regressions when
> they happen, there is also gcc-regression.  Talk to Geoff Keating
> and Phil Edwards about this.

I sent a note off to Geoff; thanks for the pointer.  I'll try
to get this running in the next few days, too.

-- 
Ciao,
al
----------------------------
Al Stone
Linux & Open Source Lab
Hewlett-Packard Company
Phone:  970-898-0345
Telnet:     898-0345
Fax:    970-898-3804
E-mail: ahs3@fc.hp.com
----------------------------



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-01-27 20:00 -------
Subject: Re: Hopefully final patch for PR 13722

Al Stone <ahs3@fc.hp.com> writes:

>> > Any particular version of the compiler to be used for the bootstrap?
>> 
>> I've been using 3.3.2 for ia64 bootstrap base compiler, with good results.
>
> Hmm.  The Debian one is usually a _little_ bit different than
> the officially released version.  I'll start with using the Debian
> default (3.3.3 today) and we'll see what happens.

It shouldn't really matter much, as long as it is not too buggy.

Andreas.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at codesourcery dot com]

------- Additional Comments From zack at codesourcery dot com  2004-01-28 18:13 -------
Subject: Final patch for PR 13722


The only change from the previous patch is, ia64_split_tmode_move now
correctly handles a reg-reg move where the destination overlaps the
source by one register - e.g. (set (reg:TF 15) (reg:TF 14)).  This
cures the regression of compat/scalar-by-value-3.

[ This is not an ABI issue; it was a straight miscompilation, which
  escaped all other notice because such moves tend to get deleted in
  optimizing compilation, before they ever get this far.  Also, the
  compat tests involving complex numbers have some very strange code
  in them.  "long double complex n = (8.0,9.0);" sets n to the complex
  number 9+0i, *not* 8+9i.  I do not think this was the intent. ]

Bootstrapped ia64-hpux (C, C++); applied mainline and 3.4 branch.  I
am also checking the patch for scalar-by-value-4 onto the 3.4 branch
at this time.

zw

2004-01-28  Zack Weinberg  <zack@codesourcery.com>
            Jim Wilson  <wilson@specifixinc.com>

        * config/ia64/ia64.c (ia64_split_tmode, ia64_split_tmode_move): 
        Rewrite to use POST_INC/POST_DEC/POST_MODIFY instead of a
        scratch pointer.
        (ia64_secondary_reload_class): Delete case GR_REGS.
        * config/ia64/ia64.md (movti, *movti_internal, movtf, *movtf_internal):
        Do not allocate a scratch register.
        (reload_inti, reload_outti, reload_intf, reload_outtf): Delete.

===================================================================
Index: config/ia64/ia64.c
--- config/ia64/ia64.c	27 Jan 2004 22:48:11 -0000	1.267
+++ config/ia64/ia64.c	28 Jan 2004 17:45:39 -0000
@@ -1395,62 +1395,37 @@ ia64_emit_cond_move (rtx op0, rtx op1, r
 }
 
 /* Split a post-reload TImode or TFmode reference into two DImode
-   components.  */
+   components.  This is made extra difficult by the fact that we do
+   not get any scratch registers to work with, because reload cannot
+   be prevented from giving us a scratch that overlaps the register
+   pair involved.  So instead, when addressing memory, we tweak the
+   pointer register up and back down with POST_INCs.  Or up and not
+   back down when we can get away with it.
+
+   REVERSED is true when the loads must be done in reversed order
+   (high word first) for correctness.  DEAD is true when the pointer
+   dies with the second insn we generate and therefore the second
+   address must not carry a postmodify.
+
+   May return an insn which is to be emitted after the moves.  */
 
 static rtx
-ia64_split_tmode (rtx out[2], rtx in, rtx scratch)
+ia64_split_tmode (rtx out[2], rtx in, bool reversed, bool dead)
 {
+  rtx fixup = 0;
+
   switch (GET_CODE (in))
     {
     case REG:
-      out[0] = gen_rtx_REG (DImode, REGNO (in));
-      out[1] = gen_rtx_REG (DImode, REGNO (in) + 1);
-      return NULL_RTX;
-
-    case MEM:
-      {
-	rtx base = XEXP (in, 0);
-
-	switch (GET_CODE (base))
-	  {
-	  case REG:
-	    out[0] = adjust_address (in, DImode, 0);
-	    break;
-	  case POST_MODIFY:
-	    base = XEXP (base, 0);
-	    out[0] = adjust_address (in, DImode, 0);
-	    break;
-
-	  /* Since we're changing the mode, we need to change to POST_MODIFY
-	     as well to preserve the size of the increment.  Either that or
-	     do the update in two steps, but we've already got this scratch
-	     register handy so let's use it.  */
-	  case POST_INC:
-	    base = XEXP (base, 0);
-	    out[0]
-	      = change_address (in, DImode,
-				gen_rtx_POST_MODIFY
-				(Pmode, base, plus_constant (base, 16)));
-	    break;
-	  case POST_DEC:
-	    base = XEXP (base, 0);
-	    out[0]
-	      = change_address (in, DImode,
-				gen_rtx_POST_MODIFY
-				(Pmode, base, plus_constant (base, -16)));
-	    break;
-	  default:
-	    abort ();
-	  }
-
-	if (scratch == NULL_RTX)
-	  abort ();
-	out[1] = change_address (in, DImode, scratch);
-	return gen_adddi3 (scratch, base, GEN_INT (8));
-      }
+      out[reversed] = gen_rtx_REG (DImode, REGNO (in));
+      out[!reversed] = gen_rtx_REG (DImode, REGNO (in) + 1);
+      break;
 
     case CONST_INT:
     case CONST_DOUBLE:
+      /* Cannot occur reversed.  */
+      if (reversed) abort ();
+      
       if (GET_MODE (in) != TFmode)
 	split_double (in, &out[0], &out[1]);
       else
@@ -1477,11 +1452,108 @@ ia64_split_tmode (rtx out[2], rtx in, rt
 	  out[0] = GEN_INT (p[0]);
 	  out[1] = GEN_INT (p[1]);
 	}
-      return NULL_RTX;
+      break;
+
+    case MEM:
+      {
+	rtx base = XEXP (in, 0);
+	rtx offset;
+
+	switch (GET_CODE (base))
+	  {
+	  case REG:
+	    if (!reversed)
+	      {
+		out[0] = adjust_automodify_address
+		  (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
+		out[1] = adjust_automodify_address
+		  (in, DImode, dead ? 0 : gen_rtx_POST_DEC (Pmode, base), 8);
+	      }
+	    else
+	      {
+		/* Reversal requires a pre-increment, which can only
+		   be done as a separate insn.  */
+		emit_insn (gen_adddi3 (base, base, GEN_INT (8)));
+		out[0] = adjust_automodify_address
+		  (in, DImode, gen_rtx_POST_DEC (Pmode, base), 8);
+		out[1] = adjust_address (in, DImode, 0);
+	      }
+	    break;
+
+	  case POST_INC:
+	    if (reversed || dead) abort ();
+	    /* Just do the increment in two steps.  */
+	    out[0] = adjust_automodify_address (in, DImode, 0, 0);
+	    out[1] = adjust_automodify_address (in, DImode, 0, 8);
+	    break;
+
+	  case POST_DEC:
+	    if (reversed || dead) abort ();
+	    /* Add 8, subtract 24.  */
+	    base = XEXP (base, 0);
+	    out[0] = adjust_automodify_address
+	      (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
+	    out[1] = adjust_automodify_address
+	      (in, DImode,
+	       gen_rtx_POST_MODIFY (Pmode, base, plus_constant (base, -24)),
+	       8);
+	    break;
+
+	  case POST_MODIFY:
+	    if (reversed || dead) abort ();
+	    /* Extract and adjust the modification.  This case is
+	       trickier than the others, because we might have an
+	       index register, or we might have a combined offset that
+	       doesn't fit a signed 9-bit displacement field.  We can
+	       assume the incoming expression is already legitimate.  */
+	    offset = XEXP (base, 1);
+	    base = XEXP (base, 0);
+
+	    out[0] = adjust_automodify_address
+	      (in, DImode, gen_rtx_POST_INC (Pmode, base), 0);
+
+	    if (GET_CODE (XEXP (offset, 1)) == REG)
+	      {
+		/* Can't adjust the postmodify to match.  Emit the
+		   original, then a separate addition insn.  */
+		out[1] = adjust_automodify_address (in, DImode, 0, 8);
+		fixup = gen_adddi3 (base, base, GEN_INT (-8));
+	      }
+	    else if (GET_CODE (XEXP (offset, 1)) != CONST_INT)
+	      abort ();
+	    else if (INTVAL (XEXP (offset, 1)) < -256 + 8)
+	      {
+		/* Again the postmodify cannot be made to match, but
+		   in this case it's more efficient to get rid of the
+		   postmodify entirely and fix up with an add insn. */
+		out[1] = adjust_automodify_address (in, DImode, base, 8);
+		fixup = gen_adddi3 (base, base,
+				    GEN_INT (INTVAL (XEXP (offset, 1)) - 8));
+	      }
+	    else
+	      {
+		/* Combined offset still fits in the displacement field.
+		   (We cannot overflow it at the high end.)  */
+		out[1] = adjust_automodify_address
+		  (in, DImode,
+		   gen_rtx_POST_MODIFY (Pmode, base,
+		     gen_rtx_PLUS (Pmode, base,
+				   GEN_INT (INTVAL (XEXP (offset, 1)) - 8))),
+		   8);
+	      }
+	    break;
+
+	  default:
+	    abort ();
+	  }
+	break;
+      }
 
     default:
       abort ();
     }
+
+  return fixup;
 }
 
 /* Split a TImode or TFmode move instruction after reload.
@@ -1489,39 +1561,60 @@ ia64_split_tmode (rtx out[2], rtx in, rt
 void
 ia64_split_tmode_move (rtx operands[])
 {
-  rtx adj1, adj2, in[2], out[2], insn;
-  int first;
-
-  adj1 = ia64_split_tmode (in, operands[1], operands[2]);
-  adj2 = ia64_split_tmode (out, operands[0], operands[2]);
-
-  first = 0;
-  if (reg_overlap_mentioned_p (out[0], in[1]))
-    {
-      if (reg_overlap_mentioned_p (out[1], in[0]))
-	abort ();
-      first = 1;
-    }
-
-  if (adj1 && adj2)
-    abort ();
-  if (adj1)
-    emit_insn (adj1);
-  if (adj2)
-    emit_insn (adj2);
-  insn = emit_insn (gen_rtx_SET (VOIDmode, out[first], in[first]));
-  if (GET_CODE (out[first]) == MEM
-      && GET_CODE (XEXP (out[first], 0)) == POST_MODIFY)
-    REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_INC,
-					  XEXP (XEXP (out[first], 0), 0),
-					  REG_NOTES (insn));
-  insn = emit_insn (gen_rtx_SET (VOIDmode, out[!first], in[!first]));
-  if (GET_CODE (out[!first]) == MEM
-      && GET_CODE (XEXP (out[!first], 0)) == POST_MODIFY)
-    REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_INC,
-					  XEXP (XEXP (out[!first], 0), 0),
-					  REG_NOTES (insn));
+  rtx in[2], out[2], insn;
+  rtx fixup[2];
+  bool dead = false;
+  bool reversed = false;
+
+  /* It is possible for reload to decide to overwrite a pointer with
+     the value it points to.  In that case we have to do the loads in
+     the appropriate order so that the pointer is not destroyed too
+     early.  Also we must not generate a postmodify for that second
+     load, or rws_access_regno will abort.  */
+  if (GET_CODE (operands[1]) == MEM
+      && reg_overlap_mentioned_p (operands[0], operands[1]))
+    {
+      rtx base = XEXP (operands[1], 0);
+      while (GET_CODE (base) != REG)
+	base = XEXP (base, 0);
+
+      if (REGNO (base) == REGNO (operands[0]))
+	reversed = true;
+      dead = true;
+    }
+  /* Another reason to do the moves in reversed order is if the first
+     element of the target register pair is also the second element of
+     the source register pair.  */
+  if (GET_CODE (operands[0]) == REG && GET_CODE (operands[1]) == REG
+      && REGNO (operands[0]) == REGNO (operands[1]) + 1)
+    reversed = true;
+
+  fixup[0] = ia64_split_tmode (in, operands[1], reversed, dead);
+  fixup[1] = ia64_split_tmode (out, operands[0], reversed, dead);
+
+#define MAYBE_ADD_REG_INC_NOTE(INSN, EXP)				\
+  if (GET_CODE (EXP) == MEM						\
+      && (GET_CODE (XEXP (EXP, 0)) == POST_MODIFY			\
+	  || GET_CODE (XEXP (EXP, 0)) == POST_INC			\
+	  || GET_CODE (XEXP (EXP, 0)) == POST_DEC))			\
+    REG_NOTES (INSN) = gen_rtx_EXPR_LIST (REG_INC,			\
+					  XEXP (XEXP (EXP, 0), 0),	\
+					  REG_NOTES (INSN))
+
+  insn = emit_insn (gen_rtx_SET (VOIDmode, out[0], in[0]));
+  MAYBE_ADD_REG_INC_NOTE (insn, in[0]);
+  MAYBE_ADD_REG_INC_NOTE (insn, out[0]);
+
+  insn = emit_insn (gen_rtx_SET (VOIDmode, out[1], in[1]));
+  MAYBE_ADD_REG_INC_NOTE (insn, in[1]);
+  MAYBE_ADD_REG_INC_NOTE (insn, out[1]);
+
+  if (fixup[0])
+    emit_insn (fixup[0]);
+  if (fixup[1])
+    emit_insn (fixup[1]);
 
+#undef MAYBE_ADD_REG_INC_NOTE
 }
 
 /* ??? Fixing GR->FR XFmode moves during reload is hard.  You need to go
@@ -4489,13 +4582,6 @@ ia64_secondary_reload_class (enum reg_cl
       /* This can happen when we take a BImode subreg of a DImode value,
 	 and that DImode value winds up in some non-GR register.  */
       if (regno >= 0 && ! GENERAL_REGNO_P (regno) && ! PR_REGNO_P (regno))
-	return GR_REGS;
-      break;
-
-    case GR_REGS:
-      /* Since we have no offsettable memory addresses, we need a temporary
-	 to hold the address of the second word.  */
-      if (mode == TImode || mode == TFmode)
 	return GR_REGS;
       break;
 
===================================================================
Index: config/ia64/ia64.md
--- config/ia64/ia64.md	27 Jan 2004 17:42:59 -0000	1.120
+++ config/ia64/ia64.md	28 Jan 2004 17:45:40 -0000
@@ -584,11 +584,12 @@
   [(set_attr "itanium_class" "ialu")])
 
 ;; With no offsettable memory references, we've got to have a scratch
-;; around to play with the second word.
+;; around to play with the second word.  However, in order to avoid a
+;; reload nightmare we lie, claim we don't need one, and fix it up
+;; in ia64_split_tmode_move.
 (define_expand "movti"
-  [(parallel [(set (match_operand:TI 0 "general_operand" "")
-		   (match_operand:TI 1 "general_operand" ""))
-	      (clobber (match_scratch:DI 2 ""))])]
+  [(set (match_operand:TI 0 "general_operand" "")
+	(match_operand:TI 1 "general_operand" ""))]
   ""
 {
   rtx op1 = ia64_expand_move (operands[0], operands[1]);
@@ -599,8 +600,7 @@
 
 (define_insn_and_split "*movti_internal"
   [(set (match_operand:TI 0 "nonimmediate_operand" "=r,r,m")
-	(match_operand:TI 1 "general_operand"      "ri,m,r"))
-   (clobber (match_scratch:DI 2 "=X,&r,&r"))]
+	(match_operand:TI 1 "general_operand"      "ri,m,r"))]
   "ia64_move_ok (operands[0], operands[1])"
   "#"
   "reload_completed"
@@ -612,20 +612,6 @@
   [(set_attr "itanium_class" "unknown")
    (set_attr "predicable" "no")])
 
-(define_expand "reload_inti"
-  [(parallel [(set (match_operand:TI 0 "register_operand" "=r")
-		   (match_operand:TI 1 "memory_operand" "m"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
-
-(define_expand "reload_outti"
-  [(parallel [(set (match_operand:TI 0 "memory_operand" "=m")
-		   (match_operand:TI 1 "register_operand" "r"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
-
 ;; Floating Point Moves
 ;;
 ;; Note - Patterns for SF mode moves are compulsory, but
@@ -764,13 +750,10 @@
   [(set_attr "itanium_class" "fmisc,fld,stf")])
 
 ;; Better code generation via insns that deal with TFmode register pairs
-;; directly.
-;; With no offsettable memory references, we've got to have a scratch
-;; around to play with the second word.
+;; directly.  Same concerns apply as for TImode.
 (define_expand "movtf"
-  [(parallel [(set (match_operand:TF 0 "general_operand" "")
-		   (match_operand:TF 1 "general_operand" ""))
-	      (clobber (match_scratch:DI 2 ""))])]
+  [(set (match_operand:TF 0 "general_operand" "")
+	(match_operand:TF 1 "general_operand" ""))]
   ""
 {
   rtx op1 = ia64_expand_move (operands[0], operands[1]);
@@ -781,8 +764,7 @@
 
 (define_insn_and_split "*movtf_internal"
   [(set (match_operand:TF 0 "nonimmediate_operand" "=r,r,m")
-	(match_operand:TF 1 "general_operand"      "ri,m,r"))
-   (clobber (match_scratch:DI 2 "=X,&r,&r"))]
+	(match_operand:TF 1 "general_operand"      "ri,m,r"))]
   "ia64_move_ok (operands[0], operands[1])"
   "#"
   "reload_completed"
@@ -794,19 +776,6 @@
   [(set_attr "itanium_class" "unknown")
    (set_attr "predicable" "no")])
 
-(define_expand "reload_intf"
-  [(parallel [(set (match_operand:TF 0 "register_operand" "=r")
-		   (match_operand:TF 1 "memory_operand" "m"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
-
-(define_expand "reload_outtf"
-  [(parallel [(set (match_operand:TF 0 "memory_operand" "=m")
-		   (match_operand:TF 1 "register_operand" "r"))
-	      (clobber (match_operand:DI 2 "register_operand" "=&r"))])]
-  ""
-  "")
 \f
 ;; ::::::::::::::::::::
 ;; ::



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[doko at cs dot tu-berlin dot de]

------- Additional Comments From doko at cs dot tu-berlin dot de  2004-01-28 20:07 -------
Subject: Re: Hopefully final patch for PR 13722

Zack Weinberg writes:
> Al Stone <ahs3@fc.hp.com> writes:
> 
> > I probably should have spoken up before but I was out of town
> > for a while.
> >
> > I've got an ia64 machine that I try to run nightly bootstrap
> > and check on (it's sporadic right now).  I'd be glad to get 
> > it so it does run nightly and send out the results (and try
> > to at least isolate what breaks).  This would be for ia64-linux,
> > on Debian unstable.
> 
> That would be really great.
> 
> > Are there particular configuration parameters that should be
> > set?  I typically only build and test for C, C++ and g77, without
> > maintainer mode set.
> 
> That should be fine.
> 
> > Any particular version of the compiler to be used for the bootstrap?
> 
> I've been using 3.3.2 for ia64 bootstrap base compiler, with good results.

CVS 20040128 including your patch fails in the first ada file in
stage2 (Debian unstable):

stage1/xgcc -Bstage1/ -B/usr/lib/gcc-snapshot/ia64-linux/bin/   -O2  -DIN_GCC   -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -pedantic -Wno-long-long -Wold-style-definition     -DHAVE_CONFIG_H  gcov-dump.o version.o ../libiberty/libiberty.a   -o gcov-dump
stage1/xgcc -Bstage1/ -B/usr/lib/gcc-snapshot/ia64-linux/bin/ -c -O2       -gnatpg -gnata -I- -I. -Iada -I../../src/gcc/ada ../../src/gcc/ada/ada.ads -o ada/ada.o
xgcc: Internal error: Illegal instruction (program gnat1)
Please submit a full bug report.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
make[4]: *** [ada/ada.o] Error 1
make[4]: Leaving directory `/home/doko/gcc/snap/gcc-snapshot-20040128/build/gcc'
make[3]: *** [stage2_build] Error 2


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-28 21:27 -------
Subject: Re: Hopefully final patch for PR 13722

On Wed, 2004-01-28 at 12:02, Matthias Klose wrote:
> CVS 20040128 including your patch fails in the first ada file in
> stage2 (Debian unstable):

I couldn't reproduce this on debian testing using a gcc tree checked out
today.  It successfully compiled the stage2 ada/ada.o file.  It later
died because of a dwarf2out.c bug on ada/osint.o, but that is an
unrelated problem.

I have debian unstable on a second partition, so I could look at this
later if I have to.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[zack at gcc dot gnu dot org]

------- Additional Comments From zack at gcc dot gnu dot org  2004-01-28 21:39 -------
believed fixed, mainline and branch.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-28 22:07 -------
Subject: Re: Hopefully final patch for PR 13722

On Wed, 2004-01-28 at 12:02, Matthias Klose wrote:
> CVS 20040128 including your patch fails in the first ada file in
> stage2 (Debian unstable):

Keep in mind that since the gcc-3.4 branch was created 2 weeks ago,
mainline gcc is now a free for all, and it is likely that gcc will be
frequently broken.  If you want a stable compiler, use the gcc-3.4
branch instead of mainline.

I didn't reproduce the exact problem you saw, but I got an ICE compiling
c-decl.c when I tried a build without Ada.  This is failing in alias.c
get_addr() as called from the scheduler, because CSE_LIB_PTR returns
NULL, and then we try to dereference it.  This problem I just saw is
unrelated to Zack's patch.  This could be the same problem you saw.  The
cselib issue is one of accessing memory after it has been freed, and it
is known to be sensitive to the environment size, i.e. you get different
failures in different files depending on your environment size.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[doko at cs dot tu-berlin dot de]

------- Additional Comments From doko at cs dot tu-berlin dot de  2004-01-29 06:47 -------
Subject: Re: Hopefully final patch for PR 13722

Jim Wilson writes:
> On Wed, 2004-01-28 at 12:02, Matthias Klose wrote:
> > CVS 20040128 including your patch fails in the first ada file in
> > stage2 (Debian unstable):
> 
> Keep in mind that since the gcc-3.4 branch was created 2 weeks ago,
> mainline gcc is now a free for all, and it is likely that gcc will be
> frequently broken.  If you want a stable compiler, use the gcc-3.4
> branch instead of mainline.

sorry, I was a bit unspecific. this was the 3.4 branch. last checkin

2004-01-28  Zack Weinberg  <zack@codesourcery.com>

        * config/ia64/ia64.md (fetchadd_acq_si, fetchadd_acq_di)
        (cmpxchg_acq_si, cmpxchg_acq_di): Exchange match_dup and
        match_operand expressions so that all match_dups appear
        lexically after their corresponding match_operands.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722

[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-01-30 02:25 -------
Subject: Re: Hopefully final patch for PR 13722

On Wed, 2004-01-28 at 22:41, Matthias Klose wrote:
> sorry, I was a bit unspecific. this was the 3.4 branch. last checkin

I have a copy of the gcc-3.4 branch updated today.  make bootstrap
worked on debian testing (last updated Nov?), debian unstable (last
updated Dec?), and debian unstable (current as of today).  I ran apt-get
in between the last two tries.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13722