23478 – [3.4 regression] Miscompilation due to reloading of a var that is also used in EH pad (original) (raw)

Description Jakub Jelinek 2005-08-19 11:08:06 UTC

The following testcase is miscompiled on x86_64-linux at -O2. Before global alloc/reload, the interesting part of code is: (reg:DI %rax) = (call _Znwm (1)) (reg:DI 81) = (reg:DI %rax) ... (reg:DI %rdi) = (reg:DI 81) (call ZN2C1C1ERK2C3S2_S2_RPS1 (%rdi, ...)) - may throw, EH pad .Leh ... (reg:DI %rdi) = (reg:DI 81) ... (barrier) .Leh: (reg:DI %rdi) = (reg:DI 81) (call _ZdlPv (%rdi))

Now, as the register preassure is pretty high, there don't appear to be any free usable call saved registers for pseudo 81, so global alloc assigns pseudo 81 into (reg:DI %r10), which is call clobbered. Then reload saves it into stack before the ZN2C1C1ERK2C3S2_S2_RPS1 call and restores it from the stack slot after the call. But doesn't restore it on the EH path as well. So we end up with: (reg:DI %rax) = (call _Znwm (1)) (reg:DI %r10 (81)) = (reg:DI %rax) ... (reg:DI %rdi) = (reg:DI %r10 (81)) (mem:DI %rsp+24) = (reg:DI %r10 (81)) (call ZN2C1C1ERK2C3S2_S2_RPS1 (%rdi, ...)) - may throw, EH pad .Leh (reg:DI %r10 (81)) = (mem:DI %rsp+24) ... (reg:DI %rdi) = (reg:DI %r10 (81)) ... (barrier) .Leh: (reg:DI %rdi) = (reg:DI %r10 (81)) (call _ZdlPv (%rdi))

As this is a reload bug, it is not reproduceable on != 3.4.x compilers I have tried, which doesn't mean the bug is present on 3.4.x only though.

Comment 2 Serge Belyshev 2005-08-19 11:35:06 UTC

Confirmed.

Comment 3 Jakub Jelinek 2005-08-19 13:36:28 UTC

caller-save.c inserts the restore insns after the can_throw_internal () CALL_INSN and as the rest of reload excepts fixup_abnormal_edges to fix the mess up. But, fixup_abnormal_edges only inserts the instructions on the fallthrough edge, not on the fallthrough edge and the EH edge. Is the bug in fixup_abnormal_edges then (which ought to insert the insns on all the edges rather than just one) or is the bug in caller-save.c which would need to take care of this and inserts the restore instructions not only after the call insn (awaiting fixup_abnormal_edges moving it to next resp. new bb), but also to the abnormal edge? It doesn't seem reload.c nor reload1.c (except fixup_abnormal_edges) bothers with this at all, so my guess would be that fixup_abnormal_edges needs to be changed, on the other side it surprises me this didn't cause (reported) problems for so long.

Comment 4 Richard Henderson 2005-08-19 18:32:43 UTC

I think it's caller-save's bug.

The use of fixup_abnormal_edges in reload and reg-stack is to move output reloads to the fallthru edge. Well, the output reloads are not used on the eh edge, because by definition the output was not generated.

Comment 5 Richard Henderson 2005-08-19 18:34:46 UTC

More, since you cannot insert insns on the abnormal EH edge, the fix to caller-save needs to be of the form "don't caller-save this variable".

Comment 6 Jakub Jelinek 2005-08-19 18:51:20 UTC

It can't be inserted just on abnormal critical edges: gcc_assert (!((e->flags & EDGE_ABNORMAL) && EDGE_CRITICAL_P (e))); So I guess we could insert it on the EH edges if !EDGE_CRITICAL_P and only only avoid caller-saving it if EDGE_CRITICAL_P.

Comment 7 Richard Henderson 2005-08-19 19:14:10 UTC

Maybe. I think you'll find that most of the time these edges are critical. I don't think it's worth bothering to make the distinction.

Comment 9 Steve Ellcey 2005-08-29 22:51:52 UTC

The patch for this bug appears to be triggering PR 23548.

Comment 13 Jakub Jelinek 2005-09-01 20:52:58 UTC

Should be fixed on 3.4/4.0/HEAD.