This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC] Fix PR rtl-optimization/33732
Rask Ingemann Lambertsen wrote:
> On Wed, Nov 07, 2007 at 09:34:41PM -0500, Kenneth Zadeck wrote:
>
>> John David Anglin wrote:
>>
>>> Breaks ada build in stage2. Turning off ada, I see
>>>
>>> /test/gnu/gcc/objdir/./prev-gcc/xgcc -B/test/gnu/gcc/objdir/./prev-gcc/ -B/opt/g
>>> nu/gcc/gcc-4.3.0/hppa2.0w-hp-hpux11.11/bin/ -c -g -O2 -DIN_GCC -W -Wall -Wwr
>>> ite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmi
>>> ssing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros
>>> -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I/opt/gnu/gcc/gcc-4.3.0/include -I/opt/gnu/gcc/gcc-4.3.0/include -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber ../../gcc/gcc/local-alloc.c -o local-alloc.o
>>> ../../gcc/gcc/local-alloc.c: In function 'find_free_reg':
>>> ../../gcc/gcc/local-alloc.c:2350: internal compiler error: in df_reg_chain_unlink, at df-scan.c:795
>>>
>>>
>> The kinds of errors that i saw were not being able to get a register
>> very late in reload as well as inconsistencies between the number of
>> words being reserved for the stack and the number of slots actually
>> being used by reload.
>>
>> This bug is new, but not at all surprising.
>>
>
> To me it _is_ surpricing to see an ICE just because of one more complete
> rescan of all insns.
>
>
Rask,
I want to reiterate that none of the dataflow maintainers want to pursue
this particular path to fix this bug.
All of us who worked on the dataflow branch spent a lot of time looking
at the bugs that came up and the performance tradeoffs and we made an
informed and deliberate decision to do what we did. Lazyness or
stupidity were not the issues.
The current stack of ra/reload passes have a fragile ordering in which
they build and update their internal datastructures. Choosing a place
where you arbitrarily update the dataflow is going to disturb that flow
of information. We have been there.
I am not saying that it cannot be done, but given that we are late into
stage 3 AND a great deal of this code is due to be replaced in 4.4,
major surgery is not the correct solution.
> It looks like a simple DF bug of not keeping the DF_HARD_REG_LIVE flag
> up-to-date when the register changes in a df_ref. Reload changes a regno
> from a hard reg back to a pseudo reg, but DF forgets to update the flag.
> Does this look right?
>
> Index: gcc/df-scan.c
> ===================================================================
> --- gcc/df-scan.c (revision 129849)
> +++ gcc/df-scan.c (working copy)
> @@ -1839,6 +1839,33 @@ df_insn_change_bb (rtx insn)
> fprintf (dump_file, " to %d\n", new_bb->index);
> }
>
> +/* Update the DF_HARD_REG_LIVE flag of THIS_REF. */
> +
> +static void
> +df_update_hard_reg_live_flag (struct df_ref *this_ref)
> +{
> + unsigned int regno = DF_REF_REGNO (this_ref);
> +
> + /* We need to clear this bit because fwprop, and in the future
> + possibly other optimizations sometimes create new refs using ond
> + refs as the model. */
> + DF_REF_FLAGS_CLEAR (this_ref, DF_HARD_REG_LIVE);
> +
> + /* See if this ref needs to have DF_HARD_REG_LIVE bit set. */
> + if (HARD_REGISTER_NUM_P (regno)
> + && (!DF_REF_IS_ARTIFICIAL (this_ref)))
> + {
> + if (DF_REF_TYPE (this_ref) == DF_REF_REG_DEF)
> + {
> + if (!DF_REF_FLAGS_IS_SET (this_ref, DF_REF_MAY_CLOBBER))
> + DF_REF_FLAGS_SET (this_ref, DF_HARD_REG_LIVE);
> + }
> + else if (!(TEST_HARD_REG_BIT (elim_reg_set, regno)
> + && (regno == FRAME_POINTER_REGNUM
> + || regno == ARG_POINTER_REGNUM)))
> + DF_REF_FLAGS_SET (this_ref, DF_HARD_REG_LIVE);
> + }
> +}
>
> /* Helper function for df_ref_change_reg_with_loc. */
>
> @@ -1859,6 +1886,7 @@ df_ref_change_reg_with_loc_1 (struct df_
>
> DF_REF_REGNO (the_ref) = new_regno;
> DF_REF_REG (the_ref) = regno_reg_rtx[new_regno];
> + df_update_hard_reg_live_flag (the_ref);
>
> /* Pull the_ref out of the old regno chain. */
> if (prev_ref)
> @@ -2584,26 +2612,7 @@ df_ref_create_structure (struct df_colle
> DF_REF_NEXT_REG (this_ref) = NULL;
> DF_REF_PREV_REG (this_ref) = NULL;
> DF_REF_ORDER (this_ref) = df->ref_order++;
> -
> - /* We need to clear this bit because fwprop, and in the future
> - possibly other optimizations sometimes create new refs using ond
> - refs as the model. */
> - DF_REF_FLAGS_CLEAR (this_ref, DF_HARD_REG_LIVE);
> -
> - /* See if this ref needs to have DF_HARD_REG_LIVE bit set. */
> - if ((regno < FIRST_PSEUDO_REGISTER)
> - && (!DF_REF_IS_ARTIFICIAL (this_ref)))
> - {
> - if (DF_REF_TYPE (this_ref) == DF_REF_REG_DEF)
> - {
> - if (!DF_REF_FLAGS_IS_SET (this_ref, DF_REF_MAY_CLOBBER))
> - DF_REF_FLAGS_SET (this_ref, DF_HARD_REG_LIVE);
> - }
> - else if (!(TEST_HARD_REG_BIT (elim_reg_set, regno)
> - && (regno == FRAME_POINTER_REGNUM
> - || regno == ARG_POINTER_REGNUM)))
> - DF_REF_FLAGS_SET (this_ref, DF_HARD_REG_LIVE);
> - }
> + df_update_hard_reg_live_flag (this_ref);
>
> if (collection_rec)
> {
>
>