[patch] combine ICE fix
Jeff Law
law@redhat.com
Tue Oct 15 20:13:00 GMT 2013
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 current
> 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.
> 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?
Jeff
More information about the Gcc-patches
mailing list