This is the mail archive of the
gcc-bugs@gcc.gnu.org
mailing list for the GCC project.
[Bug optimization/15265] New: delete_output_reload deletes necessary insn
- From: "matz at suse dot de" <gcc-bugzilla at gcc dot gnu dot org>
- To: gcc-bugs at gcc dot gnu dot org
- Date: 3 May 2004 15:21:19 -0000
- Subject: [Bug optimization/15265] New: delete_output_reload deletes necessary insn
- Reply-to: gcc-bugzilla at gcc dot gnu dot org
With HEAD (and 3.3 for that matter) the following testcase generates wrong
code on amd64 with -O1 (!). With -O2 it's hidden.
------------------------------
extern float getcard();
extern void abort();
float def;
float f()
{
float card_bound = getcard();
if (card_bound == -9)
card_bound = def;
card_bound = ((card_bound) <= (def) ? (card_bound) : (def));
return card_bound;
}
float getcard() { return -9; }
int main()
{
def = 42;
if (f() != 42)
abort();
return 0;
}
-------------------------------
The problem is, that f() implements the compares like so:
call _Z7getcardv
movaps %xmm0, %xmm1
movss def(%rip), %xmm2
cmpeqss .LC0(%rip), %xmm1
andps %xmm1, %xmm2
andnps %xmm0, %xmm1
orps %xmm2, %xmm1
here %xmm1 contains the card_bound value after the "if (card_bound==-9)", but
movss def(%rip), %xmm2
cmpnless %xmm2, %xmm1
here it is overwritten with the mask of the "<=" compare, and
andps %xmm1, %xmm2
andnps %xmm0, %xmm1
here the original orig_bound value in %xmm0 is used again (the value from
before the compare to -9).
The problem is in delete_output_reload which deletes a necessary move insn.
It goes like this:
First we start with these two compare insns:
(insn 71 11 30 0 0x2a958bfa80 (parallel [
(set (reg/v:SF 59 [ card_bound ])
(if_then_else:SF (eq (reg/v:SF 59 [ card_bound ])
(mem/u/f:SF (symbol_ref/u:DI ("*.LC0")) [0 S4 A32]))
(mem/f:SF (symbol_ref:DI ("def")) [0 def+0 S4 A32])
(reg/v:SF 59 [ card_bound ])))
(clobber (scratch:SF))
(clobber (reg:CC 17 flags))
]) 674 {sse_movsfcc_eq})
(expr_list:REG_UNUSED (reg:CC 17 flags)
(expr_list:REG_UNUSED (scratch:SF)
(nil))))
....
(insn 42 75 50 0 0x2a958bfa80 (parallel [
(set (reg/v:SF 59 [ card_bound ])
(if_then_else:SF (ungt (reg/v:SF 59 [ card_bound ])
(mem/f:SF (symbol_ref:DI ("def")) [0 def+0 S4 A32]))
(mem/f:SF (symbol_ref:DI ("def")) [0 def+0 S4 A32])
(reg/v:SF 59 [ card_bound ])))
(clobber (scratch:SF))
(clobber (reg:CC 17 flags))
]) 673 {sse_movsfcc} (insn_list 71 (nil))
(expr_list:REG_UNUSED (scratch:SF)
(expr_list:REG_UNUSED (reg:CC 17 flags)
(nil))))
The interesting reloads generated are the xmm0->xmm1 reloads in both insns:
Reloads for insn # 71
Reload 0: reload_in (SF) = (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
reload_out (SF) = (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
SSE_REGS, RELOAD_OTHER (opnum = 0)
reload_in_reg: (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
reload_out_reg: (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
reload_reg_rtx: (reg:SF 22 exmm1)
Reloads for insn # 42
Reload 0: reload_in (SF) = (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
reload_out (SF) = (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
SSE_REGS, RELOAD_OTHER (opnum = 0)
reload_in_reg: (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
reload_out_reg: (reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])
reload_reg_rtx: (reg:SF 22 exmm1)
Both are in-out reloads, hence for each insns there must be a xmm0-->xmm1
move before it and a xmm1-->xmm0 move after it. These are correctly done
initially, but when doing the input part for insn 42 it notices that
the output part of reload0 of #71 can be inherited (which is okay
as it contains indeed the correct value).
But insn #42 uses pseudo 59 (the one assigned to xmm0) more than once.
The reload is only for operand 0 (and by matching constraints for operand 2
which is the one in the compare). Hence after substitution #42 looks like so:
(insn 42 81 80 0 0x2a958bfa80 (parallel [
(set (reg:SF 22 exmm1)
(if_then_else:SF (ungt (reg:SF 22 exmm1)
(reg:SF 23 exmm2))
(reg:SF 23 exmm2)
(reg/v:SF 21 exmm0 [orig:59 card_bound ] [59])))
(clobber (reg:SF 23 exmm2))
(clobber (reg:CC 17 flags))
]) 673 {sse_movsfcc} (insn_list 71 (nil))
(nil))
Note how xmm0 here is still used (in operand 5 if I counted correct).
This is okay as per the constraints.
So, the output reload of #71 can be inherited but it can't be removed
because what it set up is still used by this insn (of course this makes
the inheritance useless).
There is code in delete_output_reload which I think should prevent this from
happening (the counting of n_inherited compared to n_occurences) but it
doesn't work here.
There are two occurences of 'xmm0' in non-destinations. But the way
n_inherited is counted leads also to 2 because the input and output part
of the reload in question are counted separately. I think this is non-sense.
Just because the same reload has in- _and_ out-part (and both are equal) does
not make it two occurences.
I have no idea why n_inherited is counted like it is, so am unsure how to
fix it. I verified my hypothesis with a simple
--------------------------
--- reload1.c 13 Apr 2004 23:27:43 -0000 1.433
+++ reload1.c 3 May 2004 15:12:17 -0000
@@ -7615,7 +7615,7 @@ delete_output_reload (rtx insn, int j, i
continue;
while (GET_CODE (reg2) == SUBREG)
reg2 = XEXP (reg2, 0);
- if (rtx_equal_p (reg2, reg))
+ if (0 && rtx_equal_p (reg2, reg))
n_inherited++;
}
else
---------------------------
which generates correct code for this example. I've CCed you, Joern,
because you seem to have last touched this function in a significant way
generally fiddling with reload inheritance. Unfortunately it was in 1998.
--
Summary: delete_output_reload deletes necessary insn
Product: gcc
Version: 3.5.0
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: optimization
AssignedTo: unassigned at gcc dot gnu dot org
ReportedBy: matz at suse dot de
CC: gcc-bugs at gcc dot gnu dot org,joern dot rennecke at
superh dot com
GCC build triplet: x86_64-suse-linux
GCC host triplet: x86_64-suse-linux
GCC target triplet: x86_64-suse-linux
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15265