On Wed, Aug 28, 2013 at 10:06 PM, Jeff Law <l...@redhat.com> wrote:
> On 08/28/2013 03:50 AM, Mikael Pettersson wrote:
>>
>> This patch fixes an ICE that occurs in #ifdef HAVE_cc0 code. The ICE
>> breaks both Java and Ada bootstrap on m68k-linux. There is also a
>> tiny C++ test case in the BZ entry.
>>
>> The ICE is triggered by the PR middle-end/50780 changes in r180192.
>> As explained in <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49847#c16>,
>> the effect of those changes is that an expression in EH context is
>> broken up so that the cc0 setter and user are put in separate BBs, which
>> normally isn't allowed. As fold_rtx sees the cc0 user, it calls
>> equiv_constant on prev_insn_cc0, but that is NULL due to the BB boundary,
>> resulting in the ICE.
>>
>> This patch checks if prev_insn_cc0 is NULL, and if so doesn't call
>> equiv_constant but sets const_arg to zero, which avoids the ICE and
>> makes fold_rtx leave the insn unchanged.
>>
>> Bootstrapped and regtested on m68k-linux, no regressions. This patch
>> has been in 4.6-based production compilers on m68k-linux since early 2012,
>> and in a 4.7-based compiler since early 2013.
>>
>> Ok for trunk and 4.8?
>>
>> [If approved I'll need help to commit it as I don't have commit rights.]
>>
>> gcc/
>>
>> 2013-08-28 Mikael Pettersson <mi...@it.uu.se>
>>
>> PR rtl-optimization/49847
>> * cse.c (fold_rtx) <second case CC0>: If prev_insn_cc0 is NULL
>> don't call equiv_constant on it.
>
> I can't help but feel something different needs to be done here. It's
> always been the case that those two insns are expected to be contiguous and
> there's going to be code all over the place that makes that assumption and
> that model is what every GCC developer has expected for the last 20+ years.
>
> What precisely about Richi's patch causes the setter and user to end up in
> different blocks? I'm guessing we now consider the comparison itself as
> potentially trapping and thus we end the block immediately? The consumer
> then appears in the next block?
Btw, the handling for COND_EXPRs was simply adjusted to be the same
as for if () conditions.
>From looking at the testcase,
int f (float g)
{
try { return g >= 0; }
catch (...) {}
}
can't you simply avoid expanding throwing stuff to a set-reg-based-on-compare
instruction sequence?
Or why does the issue not trigger on
try { if (g >= 0) return 1; else 0; } catch (...) {}
?
Richard.
> I wonder if the comparison could be duplicated and marked as non-throwing at
> RTL generation time. Alternately (and probably better) would be to convert
> the remaining stragglers and drop the old cc0 support entirely.
>
> jeff
>
>
