Re: IA-64 ICE on integer divide due to trap_if and cfgrtl
On Tue, Apr 15, 2008 at 6:27 AM, Ian Lance Taylor [EMAIL PROTECTED] wrote: Jim Wilson [EMAIL PROTECTED] writes: It seems odd that cfgrtl allows a conditional trap inside a basic block, but not an unconditional trap. The way things are now, it means we need to fix up the basic blocks after running combine or any other pass that might be able to simplify a conditional trap into an unconditional trap. I can work around this in the IA64 port. For instance I could use different patterns for conditional and unconditional traps so that one can't be converted to the other. Or I could try to hide the conditional trap inside some pattern that doesn't get expanded until after reload. None of these solutions seems quite right. But changing the basic block tree during/after combine doesn't seem quite right either. The other solution would be to fix cfgbuild to treat all trap instructions are control flow insns, instead of just the unconditional ones. I'm not sure why it was written this way though, so I don't know if this will cause other problems. I see that sibling and noreturn calls are handled the same way as trap instructions, implying that they are broken too. I think the current cfgbuild behaviour is just to avoid putting a barrier in the middle of a basic block. A conditional trap instruction is not necessarily a control flow instruction for the compiler--it's similar to a function call which may or may not return. An unconditional trap is similar to a function call which is known to not return. I guess the difference is that combine can't turn a function call which may or may not return into a function call which will not return. Since traps are uncommon, and since you can't do a lot of optimization around them anyhow, it's probably OK to make them always return true from control_flow_insn_p. At least it's worth trying to see if anything breaks. A similar issue exists on the tree level when you try to fix PR35468 the right way. Consider char *const line = /dev/ptyXX; line[8] = 1; ... the assignment to line[8] is a conditional trap which ccp transforms into /dev/ptyXX[8] = 1; which is invalid GIMPLE and we'd like to fold into a un-conditional trap (__builtin_trap(), as done elsewhere). Of course now you need to start splitting basic blocks in fold_stmt or teach its callers that it may need to cleanup themselves... both is kind of ugly, but of course we don't want every store to end a basic block either... Richard.
IA-64 ICE on integer divide due to trap_if and cfgrtl
This testcase extracted from libgcc2.c
int
sub (int i)
{
if (i == 0)
return 1 / i;
return i + 2;
}
compiled with -minline-int-divide-min-latency for IA-64 generates an
ICE.
tmp2.c:8: error: flow control insn inside a basic block
(insn 18 17 19 3 tmp2.c:5 (trap_if (const_int 1 [0x1])
(const_int 1 [0x1])) 352 {*trap} (nil))
tmp2.c:8: internal compiler error: in rtl_verify_flow_info_1, at
cfgrtl.c:1920
The problem is that IA-64 ABI specifies that integer divides trap, so we
must emit a conditional trap instruction. cse simplifies the compare.
combine substitutes the compare into the conditional trap changing it to
an unconditional trap. The next pass then fails a consistency check in
cfgrtl.
It seems odd that cfgrtl allows a conditional trap inside a basic block,
but not an unconditional trap. The way things are now, it means we need
to fix up the basic blocks after running combine or any other pass that
might be able to simplify a conditional trap into an unconditional trap.
I can work around this in the IA64 port. For instance I could use
different patterns for conditional and unconditional traps so that one
can't be converted to the other. Or I could try to hide the conditional
trap inside some pattern that doesn't get expanded until after reload.
None of these solutions seems quite right.
But changing the basic block tree during/after combine doesn't seem
quite right either.
The other solution would be to fix cfgbuild to treat all trap
instructions are control flow insns, instead of just the unconditional
ones. I'm not sure why it was written this way though, so I don't know
if this will cause other problems. I see that sibling and noreturn
calls are handled the same way as trap instructions, implying that they
are broken too.
I'm looking for suggestions here as what I should do to fix this.
Jim
Re: IA-64 ICE on integer divide due to trap_if and cfgrtl
Jim Wilson [EMAIL PROTECTED] writes: It seems odd that cfgrtl allows a conditional trap inside a basic block, but not an unconditional trap. The way things are now, it means we need to fix up the basic blocks after running combine or any other pass that might be able to simplify a conditional trap into an unconditional trap. I can work around this in the IA64 port. For instance I could use different patterns for conditional and unconditional traps so that one can't be converted to the other. Or I could try to hide the conditional trap inside some pattern that doesn't get expanded until after reload. None of these solutions seems quite right. But changing the basic block tree during/after combine doesn't seem quite right either. The other solution would be to fix cfgbuild to treat all trap instructions are control flow insns, instead of just the unconditional ones. I'm not sure why it was written this way though, so I don't know if this will cause other problems. I see that sibling and noreturn calls are handled the same way as trap instructions, implying that they are broken too. I think the current cfgbuild behaviour is just to avoid putting a barrier in the middle of a basic block. A conditional trap instruction is not necessarily a control flow instruction for the compiler--it's similar to a function call which may or may not return. An unconditional trap is similar to a function call which is known to not return. I guess the difference is that combine can't turn a function call which may or may not return into a function call which will not return. Since traps are uncommon, and since you can't do a lot of optimization around them anyhow, it's probably OK to make them always return true from control_flow_insn_p. At least it's worth trying to see if anything breaks. Ian
