This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: Steven Bosscher <stevenb dot gcc at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Paolo Bonzini <bonzini at gnu dot org>, Dominique Dhumieres <dominiq at lps dot ens dot fr>, hubicka at ucw dot cz, Richard Henderson <rth at redhat dot com>, Jeff Law <law at redhat dot com>
- Date: Mon, 03 Dec 2012 19:23:51 +0100
- Subject: Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
- References: <20121118231540.726263BABA@mailhost.lps.ens.fr> <2138069.93vGPRMs7s@polaris> <CABu31nMZQmYzOb6bspdjBO0=yAz689VOpWoEDBXqwrXAcFt3rA@mail.gmail.com>
> >> 2. gcse.c: gcse_emit_move_after added notes, but none of them was very
> >> useful as far as I could tell, and almost all of them turned
> >> self-referencing after CPROP. So I propose we just never add notes in
> >> this case.
> >
> > gcse_emit_move_after also preserves existing notes. Are they
> > problematic?
> Yes, they tend to be invalid after PRE because the registers used in
> the PRE'd expression usually are not live anymore (making the note
> invalid). Sometimes CPROP "re-validates" the notes, but it doesn't
> seem wise to me to rely on that.
So the compiler doesn't bootstrap with the gcse.c patch you posted earlier in
the thread? Still this seems too bold to me, the note datum could be a
constant and should be preserved in this case.
> >> 3. cprop.c: It seems to me that the purpose here is to propagate
> >> constants. If a reg could not be propagated, then the REG_EQUAL note
> >> will not help much either. Propagating constants via REG_EQUAL notes
> >> still helps folding comparisons sometimes, so I'm proposing we only
> >> propagate those. As a bonus: less garbage RTL because a
> >> cprop_constant_p can be shared.
> >
> > That seems a bit radical to me, especially in try_replace_reg which is
> > used for copy propagation as well. In cprop_jump, why is attaching a
> > note to the jump problematic?
>
> Most of the time a note from copy-propagation was not valid because
> the copy-prop'd reg was not live at the point of the note.
This one I think we should drop for now, or just avoid the self-referential
case. There is a comment explicitly saying that the REG_EQUAL note added by
try_replace_reg are part of the algorithm.
> Not really. It uses single_set in a few places, including
> delete_trivially_dead_insns and cse_extended_basic_block.
>
> > so it seems like we're back to the earlier
> > trick of using df_note_add_problem to clean up pre-existing REG_EQ*
> > notes.
> Again: Not really. I also bootstrapped&tested without the cse.c change.
The cse.c hunk is OK then.
> I plan (and promise ;-) to add a REG_EQ* note verifier for GCC 4.9.
Thanks (no need to promise though :-), that will be helpful. In the meantime,
I don't think that we should aim for perfection in 4.8, these REG_EQ* notes
and their quirks have been with us for a long time...
--
Eric Botcazou