This is the mail archive of the
mailing list for the GCC project.
Re: [patch] combine ICE fix
- From: Cesar Philippidis <cesar at codesourcery dot com>
- To: Jeff Law <law at redhat dot com>, Jakub Jelinek <jakub at redhat dot com>
- Cc: <vmakarov at redhat dot com>, <wilson at tuliptree dot org>, <rdsandiford at googlemail dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 16 Oct 2013 08:34:50 -0700
- Subject: Re: [patch] combine ICE fix
- Authentication-results: sourceware.org; auth=none
- References: <5256B923 dot 5020206 at codesourcery dot com> <20131010162505 dot GK30970 at tucnak dot zalov dot cz> <525D94A2 dot 8000907 at redhat dot com>
On 10/15/13 12:16 PM, Jeff Law wrote:
> On 10/10/13 10:25, Jakub Jelinek wrote:
>> On Thu, Oct 10, 2013 at 07:26:43AM -0700, Cesar Philippidis wrote:
>>> This patch addresses an ICE when building qemu for a Mips target in
>>> Yocto. Both gcc-trunk, gcc-4.8 and all of the targets are potentially
>>> affected. The problem occurs because the instruction combine phase uses
>>> two data structures to keep track of registers, reg_stat and
>>> regstat_n_sets_and_refs, but they potentially end up out of sync; when
>>> combine inserts a new register into reg_stat it does not update
>>> regstat_n_sets_and_refs. Failing to update the latter results in an
>>> occasional segmentation fault.
>>> Is this OK for trunk and gcc-4.8? If so, please check it in. I tested it
>>> on Mips and x86-64 and no regressions showed up.
>> That looks broken. You leave everything from the last size till the
>> one uninitialized, so it would only work if combine_split_insns
>> always increments max_reg_num () by at most one.
> I don't think that assumption is safe. Consider a parallel with a bunch
> of (clobber (match_scratch)) expressions.
I address that in the patch posted here
<http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00802.html>. Is that still
>> Furthermore, there are macros which should be used to access
>> the fields, and, if the vector is ever going to be resized, supposedly
>> it should be vec.h vector rather than just array.
>> Or perhaps take into account:
>> /* If a pass need to change these values in some magical way or the
>> pass needs to have accurate values for these and is not using
>> incremental df scanning, then it should use REG_N_SETS and
>> REG_N_USES. If the pass is doing incremental scanning then it
>> should be getting the info from DF_REG_DEF_COUNT and
>> DF_REG_USE_COUNT. */
>> and not use REG_N_SETS etc. but instead the df stuff.
> Which begs the question, how exactly is combine utilizing the
> regstat_n_* structures and is that use even valid for combine?
I'll take a look at that.