[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #36 from Jeffrey A. Law law at gcc dot gnu.org --- Author: law Date: Thu Feb 27 19:02:18 2014 New Revision: 208203 URL: http://gcc.gnu.org/viewcvs?rev=208203root=gccview=rev Log: PR rtl-optimization/49847 * cse.c (fold_rtx) Handle case where cc0 setter and cc0 user are in different blocks. * doc/tm.texi (Condition Code Status): Update documention for relative locations of cc0-setter and cc0-user. PR rtl-optimization/49847 * g++.dg/pr49847.C: New test. Added: trunk/gcc/testsuite/g++.dg/pr49847.C Modified: trunk/gcc/ChangeLog trunk/gcc/cse.c trunk/gcc/doc/tm.texi trunk/gcc/testsuite/ChangeLog
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #35 from rguenther at suse dot de rguenther at suse dot de --- On Tue, 25 Feb 2014, law at redhat dot com wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #34 from Jeffrey A. Law law at redhat dot com --- OK. Then I suggest two immediate things to do. 1. Fix the documentation for cc0 targets to indicate that the setter/user no longer have to be consecutive, particularly in the presence of flag_trapping_math. Sounds good. I'd document that they still should be consecutive instructions (if interpreting 'consecutive' as following a fallthru path). And say the cc0 setter may be the source of EH edges. 2. Fault in fixes. While a review of every bit of HAVE_cc0 code is warranted, I'm not terribly inclined as HAVE_cc0 targets simply aren't that important anymore. I suppose fixing things as they arise is good enough. But yes, HAVE_cc0 should be phased out - but I suppose it's unlikely to get existing ports to convert ...
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #34 from Jeffrey A. Law law at redhat dot com --- OK. Then I suggest two immediate things to do. 1. Fix the documentation for cc0 targets to indicate that the setter/user no longer have to be consecutive, particularly in the presence of flag_trapping_math. 2. Fault in fixes. While a review of every bit of HAVE_cc0 code is warranted, I'm not terribly inclined as HAVE_cc0 targets simply aren't that important anymore.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847
--- Comment #33 from rguenther at suse dot de rguenther at suse dot de ---
On Sun, 16 Feb 2014, law at redhat dot com wrote:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847
--- Comment #32 from Jeffrey A. Law law at redhat dot com ---
The problem we're seeing is with the cc0-setter and cc0-user in different
blocks, they're separated by a NOTE_BASIC_BLOCK.
That causes CSE to blow up because it expects that the cc0-setter and
cc0-user are always consecutive. While we're just seeing the failure in
CSE right now, I'm sure there's a ton of places that assume the
setter/user are inseparable as that has been the documented form for ~20
years.
From rtl.texi:
The instruction setting the
condition code must be adjacent to the instruction using the condition
code; only @code{note} insns may separate them.
We either need to relax that and audit all the HAVE_cc0 code to ensure it
doesn't make that assumption, or we need to somehow restore the property that
the setter and user are inseparable.
I think relaxing this constraint and allowing the cc0-setter and
cc0-user be separated by a fallthru-edge should be allowed
(and make sure that bb-reorder later doesn't separate the BBs)
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 Jackie Rosen jackie.rosen at hushmail dot com changed: What|Removed |Added CC||jackie.rosen at hushmail dot com --- Comment #31 from Jackie Rosen jackie.rosen at hushmail dot com --- *** Bug 260998 has been marked as a duplicate of this bug. *** Seen from the domain http://volichat.com Page where seen: http://volichat.com/adult-chat-rooms Marked for reference. Resolved as fixed @bugzilla.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847
--- Comment #32 from Jeffrey A. Law law at redhat dot com ---
The problem we're seeing is with the cc0-setter and cc0-user in different
blocks, they're separated by a NOTE_BASIC_BLOCK.
That causes CSE to blow up because it expects that the cc0-setter and cc0-user
are always consecutive. While we're just seeing the failure in CSE right now,
I'm sure there's a ton of places that assume the setter/user are inseparable as
that has been the documented form for ~20 years.
From rtl.texi:
The instruction setting the
condition code must be adjacent to the instruction using the condition
code; only @code{note} insns may separate them.
We either need to relax that and audit all the HAVE_cc0 code to ensure it
doesn't make that assumption, or we need to somehow restore the property that
the setter and user are inseparable.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added CC|richard.guenther at gmail dot com |rguenth at gcc dot gnu.org, ||stevenb.gcc at gmail dot com --- Comment #27 from Richard Biener rguenth at gcc dot gnu.org --- Yeah, not sure how non-cc0 targets avoid this situation. I wonder if there is a suitable pre-reload pass to do such reloading (I'm not sure teaching reload to do that is the best idea ...)
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 Steven Bosscher steven at gcc dot gnu.org changed: What|Removed |Added CC||steven at gcc dot gnu.org --- Comment #28 from Steven Bosscher steven at gcc dot gnu.org --- (In reply to Jeffrey A. Law from comment #24) One could argue that the CFG building code could be tweaked so that a cc0-setter is never considered the end of a block. The CFG is built too early (on gimple) to know what may end up a cc0 setter. It'd take more than a few CFG fixup hacks after expanding to RTL to get the cc0-setter and cc0-consumer back together in the same basic block. How did this work (i.e. what did the CFG look like) before tree-ssa, e.g. in gcc 3.4?
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #29 from Andreas Schwab sch...@linux-m68k.org --- In gcc = 4.6 I'm unable to tell the compiler to consider the fp compare trapping even with -ftrapping-math.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #30 from rguenther at suse dot de rguenther at suse dot de --- On February 15, 2014 3:22:54 PM GMT+01:00, sch...@linux-m68k.org gcc-bugzi...@gcc.gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #29 from Andreas Schwab sch...@linux-m68k.org --- In gcc = 4.6 I'm unable to tell the compiler to consider the fp compare trapping even with -ftrapping-math. And before the rev. Cited we failed to create eh edges / info. And if we'd have eh edges out of the gimple_cond then we'd generate wrong unwind info. Spilling cc0 by rematerializing the compare seems to be the only way out. Unless the target can avoid iceing. Cc0 should be live on the fallthru edge - so what _is_ exactly the problem we are facing?
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #26 from Jeffrey A. Law law at redhat dot com --- I'd pondered the emit-the-condition twice, one marked as NOTHROW and it may ultimately be the best short term solution. I just don't have enough background on the original change which resulted in this problem to evaluate if other solutions might be viable. FWIW, at one time we did (or considered) a vaguely similar trick to deal with reloads of float point condition codes. IIRC, the target had multiple FP condition code registers, so we wanted to use pseudos and allocate them like any other register. Of course that leads to the potential problem that you can run out and may need to spill them, but the only way to set the hard registers was by doing comparisons, so reloading them after spilling was, umm, nontrivial. Not sure what ever happened with that. FWIW, this potentially affects the m68k, vax, v850, pdp11, h8300, cris, avr and cr16. Of those, I think only the m68k has an FPU, so the impact may be very very small right now.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847
--- Comment #25 from Richard Biener rguenth at gcc dot gnu.org ---
(In reply to Jeffrey A. Law from comment #24)
This is a mess.
As noted in the other comments, we're considering a cc0-setter as a
potentially trapping insn. As a result the cc0-setter and cc0-consumer end
up in different blocks.
That's bad on so many levels and fixing it by hacking up fold_rtx like
this just papers over the fundamental problem (though I must admit from a
pragmatic standpoint, it's pretty effective).
One could argue that the CFG building code could be tweaked so that a
cc0-setter is never considered the end of a block. The downside of that is
we're lying to the compiler about the true nature of the CFG. But that
little white lie may be acceptable. I haven't looked into how ugly that
would be to implement.
Well, it re-exposes the original problem of not properly handling EH
with -fnon-call-exception and trapping FP comparisons? I don't recall
all the issues with the original case I installed the change (at least
we do consider GIMPLE_CONDs as possibly trapping, just we don't allow
a possibly throwing condition in a GIMPLE_COND).
One fix for backends where cc0 setter and consumer
may not be separated is to duplicate the comparison like
bb
...
g = 0.0; // stmt ending BB with EH edges
bb
if (g = 0.0) // redundant compare, but with NOTHROW set
...
Or to revert the original change and think of a better fix.
But certainly you can't rely on the IL being
if (g = 0.0)
instead of (what gimplification forces now)
bool tem = g = 0.0;
bb
if (tem != 0)
because with -fnon-call-exceptions writing that literally in C++ and
compiling with -O0 will yield exactly the same issue as you hit it
now (separated cc0 setter / consumer).
So reverting wouldn't be a real fix. Testcase:
int foo (double x)
{
try {
bool cond = x = 0.0;
if (cond)
return 1;
return 0;
}
catch (...)
{
return -1;
}
}
Where we shouldn't ICE when reverting the original fix and which at -O0
produces exactly the same issue you face now.
Best it into a runtime testcase that properly catches a trapping compare.
Richard.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 Jeffrey A. Law law at redhat dot com changed: What|Removed |Added CC||law at redhat dot com, ||richard.guenther at gmail dot com --- Comment #24 from Jeffrey A. Law law at redhat dot com --- This is a mess. As noted in the other comments, we're considering a cc0-setter as a potentially trapping insn. As a result the cc0-setter and cc0-consumer end up in different blocks. That's bad on so many levels and fixing it by hacking up fold_rtx like this just papers over the fundamental problem (though I must admit from a pragmatic standpoint, it's pretty effective). One could argue that the CFG building code could be tweaked so that a cc0-setter is never considered the end of a block. The downside of that is we're lying to the compiler about the true nature of the CFG. But that little white lie may be acceptable. I haven't looked into how ugly that would be to implement. One could also argue that this is an inherent flaw in cc0 targets and that any unconverted backend should be converted to deprecated. There are certainly better mechanisms for dealing with condition codes than the old cc0 nonsense. But I have neither the time nor the interest in converting the m68k backend. If we went this route it's a good bet the m68k backend would be deprecated. In the end I'm at a loss about the best direction to take. Adding Richi on cc for his thoughts.
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 Thorsten Glaser tg at mirbsd dot org changed: What|Removed |Added CC||tg at mirbsd dot org --- Comment #23 from Thorsten Glaser tg at mirbsd dot org --- Can this please be committed, it definitely fixes gcc-4.8 on Debian/m68k (applied it locally for now and asked the maintainer to include it in the source package).
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 --- Comment #22 from Andreas Schwab sch...@linux-m68k.org --- *** Bug 57929 has been marked as a duplicate of this bug. ***
[Bug rtl-optimization/49847] [4.7/4.8/4.9 Regression] NULL deref in fold_rtx (prev_insn_cc0 == NULL)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Target Milestone|4.7.3 |4.7.4 --- Comment #21 from Richard Biener rguenth at gcc dot gnu.org 2013-04-11 07:59:14 UTC --- GCC 4.7.3 is being released, adjusting target milestone.
