[Bug rtl-optimization/57878] Incorrect code: live register clobbered in split2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57878 Alexander Monakov changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED CC||amonakov at gcc dot gnu.org Resolution|--- |FIXED --- Comment #5 from Alexander Monakov --- This bug was originally fixed in July 2013 by r201036: https://gcc.gnu.org/ml/gcc-patches/2013-07/msg00732.html , but that patch changed the comparator in a way that made it non-transitive (PR 68988). In 2014, fixes for PR 60650 introduced a more robust fix for the original spill failure. This patch reverts the non_reload_pseudos check from the comparator to fix PR 68988 and also moves the fragmentation avoidance check earlier as suggested in comment #3 since it was found that it leads to smaller cc1 size without regressions on SPEC CPU 2000: https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01054.html
[Bug rtl-optimization/57878] Incorrect code: live register clobbered in split2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57878 --- Comment #4 from Alexander Monakov --- Author: amonakov Date: Tue Sep 19 10:16:20 2017 New Revision: 252972 URL: https://gcc.gnu.org/viewcvs?rev=252972=gcc=rev Log: lra: make reload_pseudo_compare_func a proper comparator PR rtl-optimization/57878 PR rtl-optimization/68988 * lra-assigns.c (reload_pseudo_compare_func): Remove fragmentation avoidance test involving non_reload_pseudos. Move frequency test below the general fragmentation avoidance test. Modified: trunk/gcc/ChangeLog trunk/gcc/lra-assigns.c
[Bug rtl-optimization/57878] Incorrect code: live register clobbered in split2
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57878
wmi at google dot com changed:
What|Removed |Added
CC||wmi at google dot com
--- Comment #3 from wmi at google dot com ---
Seems problem is at deciding the priority of assign hardreg for reload pseudos
.i.e the func reload_pseudo_compare_func.
This is the trace of 2nd iteration of reload pseudo assignments in
r.ii.209r.reload:
2nd iter for reload pseudo assignments:
Reload r196 assignment failure
Reload r199 assignment failure
Reload r204 assignment failure
Reload r204 assignment failure
Spill reload r194(hr=1, freq=1426)
Spill reload r195(hr=5, freq=1426)
Spill reload r197(hr=1, freq=1426)
Spill reload r198(hr=5, freq=1426)
Spill reload r202(hr=1, freq=1426)
Spill reload r203(hr=5, freq=1426)
Assigning to 194 (cl=GENERAL_REGS, orig=138, freq=1426, tfirst=190,
tfreq=4991)...
Assign 1 to reload r194 (freq=1426)
Assigning to 197 (cl=GENERAL_REGS, orig=138, freq=1426, tfirst=190,
tfreq=4991)...
Assign 1 to reload r197 (freq=1426)
Hard reg 1 is preferable by r222 with profit 3029
Hard reg 2 is preferable by r222 with profit 1425
Assigning to 202 (cl=GENERAL_REGS, orig=138, freq=1426, tfirst=190,
tfreq=4991)...
Assign 1 to reload r202 (freq=1426)
Assigning to 195 (cl=INDEX_REGS, orig=140, freq=1426, tfirst=191,
tfreq=4278)...
Assign 5 to reload r195 (freq=1426)
Assigning to 198 (cl=INDEX_REGS, orig=140, freq=1426, tfirst=191,
tfreq=4278)...
Assign 5 to reload r198 (freq=1426)
Assigning to 203 (cl=INDEX_REGS, orig=140, freq=1426, tfirst=191,
tfreq=4278)...
Assign 5 to reload r203 (freq=1426)
Assigning to 196 (cl=GENERAL_REGS, orig=196, freq=1426, tfirst=196,
tfreq=1426)...
Trying 2: spill 225(freq=1426)
Assigning to 199 (cl=GENERAL_REGS, orig=199, freq=1426, tfirst=199,
tfreq=1426)...
Trying 2: spill 225(freq=1426)
Assigning to 204 (cl=GENERAL_REGS, orig=204, freq=1426, tfirst=204,
tfreq=1426)...
Trying 2: spill 216(freq=2139)
Assign 0 to reload r196 (freq=1426)
Assign 0 to reload r199 (freq=1426)
Assign 0 to reload r204 (freq=1426)
Reassigning non-reload pseudos
Here is the dump after lra_constraints. These are the insns related with r194,
r195, r196:
(insn 200 120 201 6 (set (reg/f:SI 194 [orig:138 D.3281 ] [138])
(reg/f:SI 138 [ D.3281 ])) 1.ii:197 89 {*movsi_internal}
(nil))
(insn 201 200 121 6 (set (reg/f:SI 195 [orig:140 D.3282 ] [140])
(reg/f:SI 140 [ D.3282 ])) 1.ii:197 89 {*movsi_internal}
(nil))
(insn 121 201 202 6 (set (reg:DI 196)
(mem:DI (plus:SI (plus:SI (reg/f:SI 99 [ D.3281 ])
(reg/f:SI 126 [ D.3282 ]))
(const_int 8 [0x8])) [10 MEM[base: _1, index: _44, offset: 8]+0
S8 A64])) 1.ii:197 88 {*movdi_internal}
(expr_list:REG_DEAD (reg:DI 131 [ D.3287 ])
(nil)))
(insn 202 121 122 6 (set (mem:DI (plus:SI (plus:SI (reg/f:SI 194 [orig:138
D.3281 ] [138])
(reg/f:SI 195 [orig:140 D.3282 ] [140]))
(const_int 8 [0x8])) [10 MEM[base: _75, index: _77, offset:
8B]+0 S8 A64])
(reg:DI 196)) 1.ii:197 88 {*movdi_internal}
(nil))
From trace, r194 r195 are assigned hardreg before r196. Usually reload pseudos
will not conflict with each other except a special case: they are in the same
insn. r194,r195 and r196 just belong to such case. They are all in the insn
202.
In addition, r194, r195 and r196 are all reload pseudos, so once r194 and r195
are allocated, they will not be spilled for assigning hardreg for r196. In this
case, r194 and r195 get hardreg assigned before r196. So after r194 and r195
are assigned hardreg, r196 cannot find available hardreg because it has bigger
mode and require a consecutive hardreg pair. All pseudos which cannot find
hardreg after two iterations will be given ax simply, and report error. Trunk
report error but 4.8.1 doesn't report it because lra_assert is only enabled in
trunk but not in 4.8.1.
A possible fix is to give bigger mode pseudos higher priority in lra
assignment.
Index: lra-assigns.c
===
--- lra-assigns.c(revision 200944)
+++ lra-assigns.c(working copy)
@@ -194,15 +194,15 @@ reload_pseudo_compare_func (const void *
if ((diff = (ira_class_hard_regs_num[cl1]
- ira_class_hard_regs_num[cl2])) != 0)
return diff;
- if ((diff = (regno_assign_info[regno_assign_info[r2].first].freq
- - regno_assign_info[regno_assign_info[r1].first].freq)) != 0)
-return diff;
/* Allocate bigger pseudos first to avoid register file
[Bug rtl-optimization/57878] Incorrect code: live register clobbered in split2
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57878
--- Comment #2 from Easwaran Raman eraman at google dot com ---
After IRA, we have:
(insn 116 115 117 6 (set (reg:DI 130 [ D.3288 ])
(mem:DI (plus:SI (reg/v/f:SI 172 [orig:109 __first ] [109])
(const_int 4 [0x4])) [10 MEM[base: _1, index: _44, offset: 0]+0
S8 A64])) r.ii:197 88 {*movdi_internal}
(expr_list:REG_EQUIV (mem:DI (plus:SI (reg/v/f:SI 172 [orig:109 __first ]
[109])
(const_int 4 [0x4])) [10 MEM[base: _1, index: _44, offset: 0]+0
S8 A64])
(nil)))
(insn 117 116 118 6 (parallel [
(set (reg/f:SI 138 [ D.3281 ])
(minus:SI (reg/v/f:SI 173 [orig:110 __cur ] [110])
(reg/v/f:SI 103 [ __cur ])))
(clobber (reg:CC 17 flags))
]) 309 {*subsi_1}
(expr_list:REG_UNUSED (reg:CC 17 flags)
(nil)))
(insn 118 117 119 6 (parallel [
(set (reg/f:SI 140 [ D.3282 ])
(plus:SI (reg/v/f:SI 103 [ __cur ])
(const_int 4 [0x4])))
(clobber (reg:CC 17 flags))
]) 273 {*addsi_1}
(expr_list:REG_UNUSED (reg:CC 17 flags)
(nil)))
(insn 119 118 120 6 (set (mem:DI (plus:SI (reg/v/f:SI 173 [orig:110 __cur ]
[110])
(const_int 4 [0x4])) [10 MEM[base: _75, index: _77, offset:
0B]+0 S8 A64])
(reg:DI 130 [ D.3288 ])) r.ii:197 88 {*movdi_internal}
(expr_list:REG_DEAD (reg:DI 130 [ D.3288 ])
(nil)))
(insn 120 119 121 6 (set (reg:DI 131 [ D.3287 ])
(mem:DI (plus:SI (plus:SI (reg/f:SI 99 [ D.3281 ])
(reg/f:SI 126 [ D.3282 ]))
(const_int 8 [0x8])) [10 MEM[base: _1, index: _44, offset: 8]+0
S8 A64])) r.ii:197 88 {*movdi_internal}
(expr_list:REG_EQUIV (mem:DI (plus:SI (plus:SI (reg/f:SI 99 [ D.3281 ])
(reg/f:SI 126 [ D.3282 ]))
(const_int 8 [0x8])) [10 MEM[base: _1, index: _44, offset: 8]+0
S8 A64])
(nil)))
(insn 121 120 122 6 (set (mem:DI (plus:SI (plus:SI (reg/f:SI 138 [ D.3281 ])
(reg/f:SI 140 [ D.3282 ]))
(const_int 8 [0x8])) [10 MEM[base: _75, index: _77, offset:
8B]+0 S8 A64])
(reg:DI 131 [ D.3287 ])) r.ii:197 88 {*movdi_internal}
(expr_list:REG_DEAD (reg:DI 131 [ D.3287 ])
(nil)))
After reload,
1. insn 116 is deleted
2. In insn 117, the pseudo 138 is replaced with dx
3. dx is spilled into stack at offset -0x36 from bp.
4. For insn 119, first a new pseudo 193 is created which is equivalent to 130
and is loaded from memory. This 193 is in DI mode and is replaced by ax. This
would clobber edx, but that is ok since dx is now stored into stack at offset
-0x36.
5. This is followed by the the store of 193 (ax) into memory location.
6. Then dx is loaded from bp-0x36.
7. insn 120 is deleted
8. For insn 121, the pseudo 131 is replaced by 196 which is assigned the hard
reg ax.
In the sequence above, shouldn't the fill of dx from bp-0x36 at step 6 above
happen after step 8?
[Bug rtl-optimization/57878] Incorrect code: live register clobbered in split2
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57878 --- Comment #1 from Easwaran Raman eraman at google dot com --- Created attachment 30494 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=30494action=edit Disassembly of the compiled r.ii
