[Bug target/13722] [3.4/3.5 regression] [ia64] ICE in push_secondary_reload
------- 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));
+ }
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
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
Reply to: