This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[patch] reload1.c for incorrect code generation
- From: Hui-May Chang <hm dot chang at apple dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Tue, 24 Apr 2007 09:10:13 -0700
- Subject: [patch] reload1.c for incorrect code generation
We at Apple found a bug in reload1.c which caused incorrect codegen
for a movsd instruction.
The problem happened when it reloaded one vec_concat instruction to
five instructions.
dump from .lreg
(insn:HI 532 530 534 5 /Network/Servers/hills/Volumes/capanna/
iollmann/vImage/Source/Transform.c:18947 (set (reg:V2DF 512)
(vec_concat:V2DF (mem:DF (plus:SI (mult:SI (reg:SI 434)
(const_int 8 [0x8]))
(reg/f:SI 614)) [6 S8 A64])
(mem:DF (plus:SI (reg:SI 83 [ D.7147 ])
(reg/f:SI 614)) [0 S8 A8]))) 1062
{*vec_concatv2df} (nil)
(expr_list:REG_DEAD (reg:SI 434)
(expr_list:REG_DEAD (reg:SI 83 [ D.7147 ])
(nil))))
insn 532 does r512 = [ *((r434*8) + r614), *(r83 + r614)]
============================================================
dump from .greg
It is expanded to the following five instructions.
(insn 910 530 911 5 /Network/Servers/hills/Volumes/capanna/iollmann/
vImage/Source/Transform.c:18947 (set (reg:SI 1 dx)
(mem/c:SI (plus:SI (reg/f:SI 6 bp)
(const_int -332 [0xfffffffffffffeb4])) [39 S4 A8]))
40 {*movsi_1} (nil) (nil))
(insn 911 910 912 5 /Network/Servers/hills/Volumes/capanna/iollmann/
vImage/Source/Transform.c:18947 (set (reg:SI 1 dx)
(mem/c:SI (plus:SI (reg/f:SI 6 bp)
(const_int -120 [0xffffffffffffff88])) [23 D.7147+0
S4 A8])) 40 {*movsi_1} (nil)
(nil))
(insn 912 911 532 5 /Network/Servers/hills/Volumes/capanna/iollmann/
vImage/Source/Transform.c:18947 (set (reg:DF 21 xmm0)
(mem:DF (plus:SI (mult:SI (reg:SI 1 dx)
(const_int 8 [0x8]))
(reg:SI 0 ax)) [6 S8 A64])) 94 {*movdf_nointeger} (nil)
(nil))
(insn:HI 532 912 913 5 /Network/Servers/hills/Volumes/capanna/
iollmann/vImage/Source/Transform.c:18947 (set (reg:V2DF 21 xmm0)
(vec_concat:V2DF (reg:DF 21 xmm0)
(mem:DF (plus:SI (reg:SI 1 dx)
(reg:SI 0 ax)) [0 S8 A8]))) 1062
{*vec_concatv2df} (nil)
(nil))
(insn 913 532 534 5 /Network/Servers/hills/Volumes/capanna/iollmann/
vImage/Source/Transform.c:18947 (set (reg:V2DF 22 xmm1 [512])
(reg:V2DF 21 xmm0)) 917 {*movv2df_internal} (nil)
(nil))
The problem is both insn 910 and 911 were allocated the same register
(dx). The result from insn 910 is to be used by insn 912.
However, insn 910 is deleted as it becomes a dead insn.
The information of five reloaded insns are kept in rld[0], rld[1], rld
[2], rld[3], rld[4].
reload_reg_free_p and reloads_conflict find free registers for them
and decide if they can share a register.
One of the decision criteria is rld[i].when_needed. rld[0] and rld[2]
share a register as rld[0] is for RELOAD_FOR_OTHER_ADDRESS,
whereas rld[2] is for RELOAD_FOR_OPERAND_ADDRESS.
rld[0].in_reg == r434, rld[0].when_needed == RELOAD_FOR_OTHER_ADDRESS
rld[1].in_reg == r614, rld[1].when_needed == RELOAD_FOR_OTHER_ADDRESS
rld[2].in_reg == r83, rld[2].when_needed == RELOAD_FOR_OPERAND_ADDRESS
rld[3].in_reg == r614, rld[3].when_needed == RELOAD_FOR_OPERAND_ADDRESS
rld[4].in_reg == r434*8, rld[4].when_needed == RELOAD_OTHER
merge_assigned_reloads decides to merge rld[1] and rld[2].
rld[1].when_needed is changed to RELOAD_OTHER at line 6085 as rld
[1].reg_rtx is equal to rld[3].reg_rtx.
6075 if (j == n_reloads
6076 && max_input_address_opnum <=
min_conflicting_input_opnum)
6077 {
6078 for (j = 0; j < n_reloads; j++)
6079 if (i != j && rld[j].reg_rtx != 0
6080 && rtx_equal_p (rld[i].reg_rtx, rld[j].reg_rtx)
6081 && (! conflicting_input
6082 || rld[j].when_needed ==
RELOAD_FOR_INPUT_ADDRESS
6083 || rld[j].when_needed ==
RELOAD_FOR_OTHER_ADDRESS))
6085 rld[i].when_needed = RELOAD_OTHER;
6086 rld[j].in = 0;
6087 reload_spill_index[j] = -1;
6088 transfer_replacements (i, j);
6089 }
rld[2].when_needed is changed to RELOAD_OTHER at line 6114,
6101 if (rld[i].when_needed == RELOAD_OTHER)
6102 for (j = 0; j < n_reloads; j++)
6103 if (rld[j].in != 0
6104 && rld[j].when_needed != RELOAD_OTHER
6105 && rld[j].when_needed != RELOAD_FOR_OTHER_ADDRESS
6106 && (! conflicting_input
6107 || rld[j].when_needed ==
RELOAD_FOR_INPUT_ADDRESS
6108 || rld[j].when_needed ==
RELOAD_FOR_INPADDR_ADDRESS)
6109 && reg_overlap_mentioned_for_reload_p (rld[j].in,
6110 rld
[i].in))
6111 {
6112 int k;
6113
6114 rld[j].when_needed
6115 = ((rld[j].when_needed ==
RELOAD_FOR_INPUT_ADDRESS
6116 || rld[j].when_needed ==
RELOAD_FOR_INPADDR_ADDRESS)
6117 ? RELOAD_FOR_OTHER_ADDRESS : RELOAD_OTHER);
After rld[1] and rld[2] are merged, emit_reload_insns emits insns as
the comments below,
7046 /* Now write all the insns we made for reloads in the order
expected by
7047 the allocation functions. Prior to the insn being
reloaded, we write
7048 the following reloads:
7049
7050 RELOAD_FOR_OTHER_ADDRESS reloads for input addresses.
7051
7052 RELOAD_OTHER reloads.
7053
7054 For each operand, any RELOAD_FOR_INPADDR_ADDRESS reloads
followed
7055 by any RELOAD_FOR_INPUT_ADDRESS reloads followed by the
7056 RELOAD_FOR_INPUT reload for the operand.
7057
7058 RELOAD_FOR_OPADDR_ADDRS reloads.
7059
7060 RELOAD_FOR_OPERAND_ADDRESS reloads.
rld[0] belongs to RELOAD_FOR_OTHER_ADDRESS, rld[1] and rld[2] belong
to RELOAD_OTHER.
The emitted insns become rld[0], rld[2], rld[1], rld[3], rld[4].
The problem is rld[1] and rld[2] shouldn't have been merged.
We proposed the following patch which simply prevents the merge for
this case. Your comments and suggestions are welcomed.
The patch has been tested on x86 MacOS with "make all", "--enable-
languages=c,c++,objc,obj-c++", and regression tested with a top-level
"make check-gcc" with no regression.
The test case checked the number of instructions being deleted, which
should be one less with the patch.
ChangeLog:
2007-04-24 Hui-May Chang <hm.chang@apple.com>
* reload1.c (merge_assigned_reloads) : Do not merge a
RELOAD_OTHER
instruction with a RELOAD_FOR_OPERAND_ADDRESS instruction.
* gcc.target/i386/reload-1.c. New.
Attachment:
radar-patch.reload.txt
Description: Text document
OK for mainline?
Hui-May Chang